Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit test guide and catch example #138

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

KipHamiltons
Copy link
Member

@KipHamiltons KipHamiltons commented Jul 13, 2021

Very cool and epic guides. One covers unit testing, another gives a little detail of a catch example.

TODO:

  • Add stuff about catch BDD tests to catch guide, and try to bring focus onto that style

Preview

@KipHamiltons KipHamiltons self-assigned this Jul 13, 2021
@KipHamiltons KipHamiltons added the documentation Improvements or additions to documentation label Jul 13, 2021
Copy link
Member

@ysims ysims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to link to the catch guide in the how to write tests guide? I'd even maybe go as far as to say, combine the pages (see troubleshooting page)? Otherwise I'd also suggest that 'Catch Getting Started' should be under 'Main Codebase'.
If I wanted to write tests and knew nothing, 'Catch Getting Started' wouldn't stand out to me as a page that would hold the answers I seek.

src/book/03-guides/04-general/04-writing-tests.mdx Outdated Show resolved Hide resolved
src/book/03-guides/04-general/04-writing-tests.mdx Outdated Show resolved Hide resolved
src/book/03-guides/04-general/04-writing-tests.mdx Outdated Show resolved Hide resolved
src/book/03-guides/02-tools/05-catch-guide.mdx Outdated Show resolved Hide resolved
src/book/03-guides/02-tools/05-catch-guide.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@Crafty15 Crafty15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overflow thing is still a bit hazy to me. Most reference material i've read mentions handling min - 1 and max + 1 (values across boundaries). I suppose we have cases where we don't want to do this. If so, maybe we could state our reasoning.

@JosephusPaye
Copy link
Member

JosephusPaye commented Jul 14, 2021

As to the question of whether or not you should test overflow/underflow/similar boundary conditions:

If the piece of code you're testing could get under/overflowed input as a part of its normal operation, then you want to test that. If it doesn't, then you don't have to test it. Put another way, if you're testing a function that has explicit knowledge of boundary or exceptional conditions (it uses a conditional, try/catch, checks that its inputs are not out of bounds, etc), you want to test on and around those boundaries and conditions.

Essentially, you want your tests to cover as many code paths as possible, to verify that everything it attempts to do, it does correctly. Most of the time the boundary conditions you'll be testing will be based on some specific numeric value in the code being tested. For example, if you're testing a function that takes an input x (or an input that influences x) and has if (x <= 100) { ... } somewhere in it, you want to test x = 99, x = 100, and x = 101, at a minimum.

@CMurtagh-LGTM
Copy link
Contributor

CMurtagh-LGTM commented Jul 14, 2021

As to the question of whether or not you should test overflow/underflow/similar boundary conditions:

If the piece of code you're testing could get under/overflowed input as a part of its normal operation, then you want to test that. If it doesn't, then you don't have to test it. Put another way, if you're testing a function that has explicit knowledge of boundary or exceptional conditions (it uses a conditional, try/catch, checks that its inputs are not out of bounds, etc), you want to test on and around those boundaries and conditions.

Essentially, you want your tests to cover as many code paths as possible, to verify that everything it attempts to do, it does correctly. Most of the time the boundary conditions you'll be testing will be based on some specific numeric value in the code being tested. For example, if you're testing a function that takes an input x (or an input that influences x) and has if (x <= 100) { ... } somewhere in it, you want to test x = 99, x = 100, and x = 101, at a minimum.

Bounds checking is completely different to integer overflow. Why are you linking my comment for integer overflow then talking about bounds checking???

Integer overflow is if you have 0111 as a 4 bit integer with two's complement, so this would represent 7. Then if we add one we get 1000, which with two's complement is -8. The c++ standard lists this as undefined behaviour

@Crafty15 @JosephusPaye

@JosephusPaye
Copy link
Member

JosephusPaye commented Jul 14, 2021

I was responding to the thread your comment started. It would have been more accurate to link to Liam's comment in that thread.

That said, I wasn't talking about bounds checking specifically, but the general testing of boundary conditions (of which arithmetic integer over/underflows are a subset). In your example, 7 is by definition a boundary value of a 4-bit int with two's complement.

Copy link
Collaborator

@Bidski Bidski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial review for now

src/book/03-guides/02-tools/05-catch-guide.mdx Outdated Show resolved Hide resolved
src/book/03-guides/02-tools/05-catch-guide.mdx Outdated Show resolved Hide resolved
@KipHamiltons KipHamiltons marked this pull request as draft August 14, 2021 07:57
@Crafty15 Crafty15 linked an issue Feb 19, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add section about pairing via bluetooth
6 participants