-
Notifications
You must be signed in to change notification settings - Fork 0
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
test(rules): rstest framework #39
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.
This is much better and a perfect fit for this use-case!
I proposed to remove constant arguments with the objective of improving readability. Let me know if you think this is not a good idea.
Thank you @cmdoret and very good suggestion! For 2 out of 3 rules I left a simple assert and removed the argument that were constant across cases 👍 |
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.
🙌
Proposed Changes
Replaced previous rule tests with new tests following the
rstest
framework. Now we have 1 test function per rule, with each function testing different scenarios.Example of previous code structure:
Example of ✨ new ✨ code structure:
Addresses #25
Types of Changes
functionality to not work as expected).
other choices apply).
Checklist
CONTRIBUTING
guidelines.
works.