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

tests: Make CHECK enforce 1 instead of just "!= 0" #1481

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

real-or-random
Copy link
Contributor

@real-or-random real-or-random commented Jan 11, 2024

This ensures that we don't omit the "== 1" in tests accidentally,
and thus also strengthens existing tests in which it has been omitted.

We want to check "== 1" in particular for the return values of API
functions, but it also makes sense in the case of internal functions.

If you really want to check "!= 0", you can still write it explicitly.


The second commit drops all the redundant "== 1"s. I tried to be careful to drop it only where it makes sense. Now, git grep "== 1" shows only instances where the 1 is supposed to be an actual integer and not a boolean. Still, this touches many lines, so I can drop this commit if you think it's too intrusive.

This ensures that we don't omit the "== 1" in tests accidentally,
and thus also strengthens existing tests in which it has been omitted.

We want to check "== 1" in particular for the return values of API
functions, but it also makes sense in the case of internal functions.

If you really want to check "!= 0", you can still write it explicitly.
Copy link
Contributor

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

concept ACK

@jonasnick

This comment was marked as resolved.

@theStack
Copy link
Contributor

Concept ACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants