-
-
Notifications
You must be signed in to change notification settings - Fork 185
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
Improve testsuite #1975
base: main
Are you sure you want to change the base?
Improve testsuite #1975
Conversation
Somehow ignoring the snapshots from linting doesn't work in this branch as it does for the one on the fork neighbourhoodie#13 |
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.
So the changes to the tests as written are wrong in many cases.
Many of the tests verify the correctness of the results as opposed to being regression tests handled by snapshotting. For tests that verify result correctness, we hard code the correct answer clearly in the test case. In some cases we even include comments on why the answer is correct. The changes here obfuscate that.
We need to separate these two cases out and only apply snapshotting to true regression tests that only care if the code output did not change and have no constraints on what is in that output.
@beckermr Hi 👋 I work with @hulkoba at @neighbourhoodie and I'm following up while she's on vacation. Your comment about snapshot testing absolutely makes sense. Do you have a list of which tests you think we should use snapshots for, and which should remain as hard-coding the expected response? |
I'd say all of them should remain hard coded. |
Hey all 👋 Since the snapshot testing is still discussed, I was wondering if you are still interested in the removal of If so, I could take those commits from this PR and create a new one. |
Yes, for sure we should move to pytest! |
Hello 👋!
As part of the STF work for Conda, this PR introduces several improvements to our testing framework by transitioning from unittest to pytest, leveraging pytest-mock for mocking, and adopting snapshot testing with syrupy.
Note that the testing section in the readme is updated at #1956
Thank you!
Checklist