-
Notifications
You must be signed in to change notification settings - Fork 18
Use ruff linting; F811 will prevent duplicate test function names #56
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
==========================================
- Coverage 98.53% 98.53% -0.01%
==========================================
Files 29 29
Lines 4909 4908 -1
Branches 1621 1621
==========================================
- Hits 4837 4836 -1
Misses 62 62
Partials 10 10 ☔ View full report in Codecov by Sentry. |
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.
Overall I like this, but there's a few questions I have.
I'd still prefer to keep measuring coverage for tests though - it provides that extra guarantee that the test code is actually being run as expected.
d170c73
to
4c925a3
Compare
Fair enough, though there will likely be some false positives (ie coverage saying that we get reduced coverage) for some time if we go with using xfail. Which I think is a useful placeholder for tests that need fixing once they are possible. |
4c925a3
to
808f8cb
Compare
django_template = engines["django"].from_string(template) # noqa | ||
rust_template = engines["rusty"].from_string(template) # noqa |
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.
I missed it when reviewing before, but I think the reason ruff
is complaining is because django_template
and rust_template
are unused.
Both test_addslashes01
and test_addslashes02
refer to a self.engine
, but this doesn't exist because these aren't methods on a unittest test class.
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.
See #57
Co-authored-by: Lily Foote <[email protected]>
Here we use linting to check that we don't get any duplicate function names in tests. Specifically F811 covers this.
We get some other linting for free, and have the possibility to expand on linting rules if we want.
So in that sense, perhaps using coverage for python test_*.py becomes redundant?