Skip to content

[iOS]Ensures all expectations are followed by a verify #547

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Arthur-Milchior
Copy link

The error message if it is not the case state that the expect can be replaced by stub if we actually don’t care about verifying. Chrome code ended up with dozens and dozens of tests that failed to verify because the author and the reviewer forgot to add this last check. I believe that having a way to require all verify to be called would improve the usability of this class in tests.

I understand that you may not be able to merge it as-is, at upgrading would break the test flow of many of your users. Instead, I suspect that you’ll want a way to enable this feature progressively, at user request. I’m willing to work with you to ensure that we can merge this feature if you accept it.

For our usage in chromium, I expect the simpler way to deal with this feature is to have some #DEFINE macro. If a build-time constant is defined, we enable the feature, other wise it’s disabled.

It would also be possible to have a static boolean that default to false, and that the test should switch to true to enable the safety feature.

The error message if it is not the case state that the expect can be replaced by stub if we actually don’t care about verifying.
Chrome code ended up with dozens and dozens of tests that failed to verify because the author and the reviewer forgot to add this last check. I believe that having a way to require all verify to be called would improve the usability of this class in tests.

I understand that you may not be able to merge it as-is, at upgrading would break the test flow of many of your users. Instead, I suspect that you’ll want a way to enable this feature progressively, at user request. I’m willing to work with you to ensure that we can merge this feature if you accept it.

For our usage in chromium, I expect the simpler way to deal with this feature is to have some #DEFINE macro. If a build-time constant is defined, we enable the feature, other wise it’s disabled.

It would also be possible to have a static boolean that default to false, and that the test should switch to true to enable the safety feature.
@LowAmmo
Copy link

LowAmmo commented Jun 16, 2025

Maybe I'm too much of a "purist"...but this is a great idea, and I would actually say providing a temporary way to disable this versus providing a way to optionally enable it...

Then way there's an easy way for consumers to put off making lots of changes...but an immediate feedback to there being "issues". (I also fully support any idea that highlights unit tests possibly not actually testing anything... 😄 )

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

Successfully merging this pull request may close these issues.

2 participants