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

Assertion count seems to be wrong #173

Closed
s9w opened this issue May 20, 2024 · 6 comments
Closed

Assertion count seems to be wrong #173

s9w opened this issue May 20, 2024 · 6 comments

Comments

@s9w
Copy link

s9w commented May 20, 2024

Snitch seems to count test cases towards assertion count. A singular empty test case counts as 1 test case and 1 assertion. Note: I'm on 1.2.3 since that's the last version that shipped with the header-only version.

@cschreib
Copy link
Member

Indeed, this is because each test case automatically checks that no uncaught exception is thrown in the test body. Disabling exceptions will remove this additional check.

@tocic
Copy link
Member

tocic commented Jun 1, 2024

@cschreib should we also address this issue? There are no assets other than the source code in 1.2.4 and 1.2.5 for some reason.

I'm on 1.2.3 since that's the last version that shipped with the header-only version.

@s9w meanwhile, you can generate it yourself using the SNITCH_HEADER_ONLY CMake option (example).

@s9w
Copy link
Author

s9w commented Jun 1, 2024

I don't use/like cmake, hence my reliance on the header-only version. But thanks for the suggestion @tocic

@cschreib
Copy link
Member

cschreib commented Jun 1, 2024

Yes we should make sure all releases have assets. I must have forgotten to upload them the last couple times. GitHub actions already generate them, but the upload to the release entry is still manual. I haven't yet figured out how to make it automated (edit: issue opened #175).

@s9w are you OK otherwise with the resolution of this specific ticket being a won't fix?

@s9w
Copy link
Author

s9w commented Jun 1, 2024

@cschreib I disagree with the way that this is counted - at least in this opinionated/non-customizable way - but can accept it as it's not crucial to me. I think that this is explicitly counting an implicit assumption. We might as well count that the function didn't terminate and so on. But again: all good, I don't care much for the number. We can close this

@cschreib
Copy link
Member

cschreib commented Jun 9, 2024

I think that this is explicitly counting an implicit assumption. We might as well count that the function didn't terminate and so on

That's the thing actually, it appears like an implicit check for the user, but it is definitely explicit for the snitch library: there's a try/catch specifically added around the execution of each test case to catch this. If you run the tests with full verbosity, you will see that there's a "no exception caught" being reported when the test returns successfully. If we were able to check for "doesn't terminate" in the same way (#35), we would report it too, because this (like for an exception being thrown, or the test entering an infinite loop) effectively results in the end of the test case not being reached. I hope that clarifies the reasoning a bit more! I'll close the issue, but feel free to respond.

Now that being said, I agree that "empty test case reports one successful assertion" is a surprising situation. I read about this the other day and discovered this is what people call a "rotten green test" (i.e., it reports "green" = pass, but doesn't actually test what it should). There's a report on the topic, and a PR for GoogleTest to attempt to catch and report this. It got me thinking about your issue, and that perhaps we should also do something about it. I'll open a separate ticket for that (#176).

@cschreib cschreib closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants