-
-
Notifications
You must be signed in to change notification settings - Fork 323
test: use match in pytest.raises #1846
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1846 +/- ##
=======================================
Coverage 97.99% 97.99%
=======================================
Files 60 60
Lines 2691 2691
=======================================
Hits 2637 2637
Misses 54 54 ☔ View full report in Codecov by Sentry. |
787fbad to
9daaf1b
Compare
9daaf1b to
37d09f7
Compare
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.
Pull request overview
Updates the test suite to use pytest.raises(..., match=...) for asserting exception messages, reducing post-raises string assertions and making error-message checks more idiomatic.
Changes:
- Replaced
excinfo-based string assertions withpytest.raises(..., match=...)across multiple tests. - Added
re.escape(...)in some places to safely match literal strings (e.g., filenames/paths) in regex-basedmatch. - Minor cleanup in tests (e.g., removing unused captured stderr variable, using
check=Trueinsubprocess.run).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_git.py | Uses match= for GitCommandError assertion. |
| tests/test_factory.py | Uses match= for factory failure exception message. |
| tests/test_cz_customize.py | Uses match= for missing customize config error message. |
| tests/test_conf.py | Adds re and uses re.escape in match= for invalid config filename checks. |
| tests/test_cli.py | Converts several raises assertions to match=, plus small cleanups. |
| tests/test_changelog.py | Uses match= for changelog-related exceptions. |
| tests/test_bump_update_version_in_files.py | Adds re and converts consistency error assertions to match=. |
| tests/commands/test_commit_command.py | Adds re and converts several commit-command exception assertions to match=. |
| tests/commands/test_check_command.py | Converts various check-command exception assertions to match= (including multi-message case). |
| tests/commands/test_changelog_command.py | Uses match= for missing revision range case. |
| tests/commands/test_bump_command.py | Converts bump-command exception assertions to match= in multiple scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with pytest.raises( | ||
| CurrentVersionNotFoundError, | ||
| match=re.escape( | ||
| f"Current version {old_version} is not found in {inconsistent_python_version_file}." | ||
| ), | ||
| ): |
Copilot
AI
Feb 8, 2026
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.
The new match= only checks the first sentence of CurrentVersionNotFoundError and no longer asserts that the follow-up hint about inconsistent version_files is present. This reduces coverage of the error message content; consider extending the regex to include the second line (or keeping an additional assertion) so regressions in that hint are caught.
| with pytest.raises( | ||
| CurrentVersionNotFoundError, | ||
| match=re.escape( | ||
| f"Current version {old_version} is not found in {inconsistent_python_version_file}" | ||
| ), | ||
| ): |
Copilot
AI
Feb 8, 2026
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.
Same as above: this match= no longer validates that the error message includes the explanatory hint about inconsistent version_files. Consider matching the full message (including the second line) to keep the test meaningful.
| with pytest.raises( | ||
| NoCommitizenFoundException, | ||
| match="The committer has not been found in the system.", | ||
| ): |
Copilot
AI
Feb 8, 2026
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.
pytest.raises(..., match=...) treats match as a regex. This pattern contains a trailing . which will match any character, making the test less strict than intended. Use re.escape(...) (and import re) if you want a literal match.
| with pytest.raises( | ||
| MissingCzCustomizeConfigError, match=MissingCzCustomizeConfigError.message | ||
| ): |
Copilot
AI
Feb 8, 2026
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.
match= is a regex; MissingCzCustomizeConfigError.message ends with . so the dot will be treated as a wildcard. Consider wrapping the message with re.escape(...) (and importing re) to ensure the test matches the literal text.
|
|
||
| with pytest.raises(CurrentVersionNotFoundError) as excinfo: | ||
| with pytest.raises( | ||
| CurrentVersionNotFoundError, match="Current version 0.1.0 is not found in" |
Copilot
AI
Feb 8, 2026
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.
match is interpreted as a regex; the version 0.1.0 contains . which will match any character. If the intent is to assert the literal version string, use re.escape(...) around the expected text.
| CurrentVersionNotFoundError, match="Current version 0.1.0 is not found in" | |
| CurrentVersionNotFoundError, | |
| match=re.escape("Current version 0.1.0 is not found in"), |
| with pytest.raises(NotAllowed) as excinfo: | ||
| with pytest.raises( | ||
| NotAllowed, | ||
| match="--major-version-zero is meaningless for current version 1.0.0", |
Copilot
AI
Feb 8, 2026
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.
match is a regex; 1.0.0 contains . which will match any character. If the intent is to verify the literal version in the message, wrap the expected string with re.escape(...).
| match="--major-version-zero is meaningless for current version 1.0.0", | |
| match=re.escape( | |
| "--major-version-zero is meaningless for current version 1.0.0" | |
| ), |
| @@ -44,15 +44,13 @@ def test_invalid_command(util: UtilFixture, capsys, file_regression, arg): | |||
|
|
|||
|
|
|||
| def test_cz_config_file_without_correct_file_path(util: UtilFixture, capsys): | |||
Copilot
AI
Feb 8, 2026
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.
capsys is no longer used in this test after switching to match=. Consider removing it from the function signature to avoid an unused fixture parameter.
| def test_cz_config_file_without_correct_file_path(util: UtilFixture, capsys): | |
| def test_cz_config_file_without_correct_file_path(util: UtilFixture): |
Description
Use
matchparameter inpytest.raisesand remove verbose string assertions.Was generative AI tooling used to co-author this PR?
Used
cursoragent mode to assist this task.