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: switch asserts to require #1418

Closed
ValarDragon opened this issue Jun 27, 2018 · 1 comment
Closed

tests: switch asserts to require #1418

ValarDragon opened this issue Jun 27, 2018 · 1 comment
Assignees

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented Jun 27, 2018

Is there a reason we use asserts in most tests instead of require? I find it much harder to debug the output of assert statements (since it takes longer to isolate the actual error), and I think it just increases time to debug the core issue. I propose that we change all of the assert statements in test cases to requires. Should be do-able with a couple of sed commands.

(Assert still executes the remaining tests, whereas require halts after that error)

/cc @ebuchman , @rigelrozanski , @cwgoes

@rigelrozanski
Copy link
Contributor

yeah totally let's replace the majority of asserts with requires - asserts can still be useful in limited scenarios - for instance, you want to check a series of parameters from the output simultaneously - if you used require there it could be difficult to adequately debug.

@ValarDragon ValarDragon self-assigned this Jun 29, 2018
ValarDragon added a commit that referenced this issue Jun 29, 2018
Switch most assert statements to require, to ease debugging.
Closes #1418
cwgoes pushed a commit that referenced this issue Jun 30, 2018
* meta: Switch the majority of asserts to require

Switch most assert statements to require, to ease debugging.
Closes #1418

* Fix imports
adrianbrink pushed a commit that referenced this issue Jul 2, 2018
* meta: Switch the majority of asserts to require

Switch most assert statements to require, to ease debugging.
Closes #1418

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

No branches or pull requests

2 participants