-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
Refactor unit tests to use more convenient doctest assertion macros #3393
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still tests failing in the CI. Can you please update from the latest develop
branch?
As far as I can tell, my fork is up to date with regards to the Also, it seems that the only CI builds that did not succeed are the |
The latest develop branch runs all CI tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Thanks a lot! |
Thank you, too. |
This PR refactors unit tests to use more convenient doctest assertion macros, as discussed in issue #3365.
The bulk of the change was done automatically, using a regexp replace, with some manual adjustments.
In the process, I found several unit tests that rely on the side effects of running two assertion macros and got broken by running a single one. Those tests are left untouched, to keep them passing. I think I will try to address those is a separate PR.