-
Notifications
You must be signed in to change notification settings - Fork 453
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
Increase test coverage of the python_format
checker
#1176
Conversation
@@ -65,9 +65,6 @@ def _validate_format(format: str, alternative: str) -> None: | |||
arguments are not interchangeable as `alternative` may contain less | |||
placeholders if `format` uses named placeholders. | |||
|
|||
The behavior of this function is undefined if the string does not use |
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 think this is wrong - python_format
happily calls this method when format
does not contain any format placeholders (for example when msgid
contains them but msgid_plural
does not, python_format
will still call _validate_format(msgid_plural, msgstr)
).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1176 +/- ##
==========================================
+ Coverage 91.37% 91.54% +0.17%
==========================================
Files 27 27
Lines 4672 4674 +2
==========================================
+ Hits 4269 4279 +10
+ Misses 403 395 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ 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.
LGTM! Every little (sensible) coverage increase is for the better.
There a few bugs that I found in the
python_format
checker while working on the linter that I'd like to fix (e.g. one is that it does not validatemsgstr[N]
forN >= 2
).Though before I start fixing anything, I want to make sure we have the checker fully covered with tests as it would be pretty easy to accidentally introduce other bugs.
cc @akx