-
-
Notifications
You must be signed in to change notification settings - Fork 301
test(SemVer): refine tests and remove unnecessary tests which test th… #1598
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: v4-9-2
Are you sure you want to change the base?
test(SemVer): refine tests and remove unnecessary tests which test th… #1598
Conversation
assert isinstance(SemVer2("0.0.1"), VersionProtocol) | ||
|
||
|
||
def test_semver_sortable(): |
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'm not sure why we need to test this. The __lt__
operator is defined in packaging
library and we are not overriding it.
72bb16e
to
4d8f987
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4-9-2 #1598 +/- ##
=========================================
Coverage ? 98.32%
=========================================
Files ? 58
Lines ? 2682
Branches ? 0
=========================================
Hits ? 2637
Misses ? 45
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4d8f987
to
8d3387c
Compare
8d3387c
to
65ebc5b
Compare
65ebc5b
to
56025a2
Compare
tests/utils.py
Outdated
class VersionSchemeTestArgs(NamedTuple): | ||
current_version: str | ||
increment: Increment | None | ||
prerelease: Prerelease | None | ||
prerelease_offset: int | ||
devrelease: int | None | ||
expected_version: str |
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.
Main change
tests/test_version_scheme_semver2.py
Outdated
itertools.chain(tdd_cases, weird_cases, simple_flow, linear_prerelease_cases), | ||
"args", | ||
[ | ||
# test driven development |
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.
# test driven development |
I think we can remove this line. This does not provide much information
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.
yep
removed
tests/test_version_scheme_semver2.py
Outdated
VersionSchemeTestArgs( | ||
current_version="0.1.1", | ||
increment="PATCH", | ||
prerelease=None, | ||
prerelease_offset=0, | ||
devrelease=None, | ||
expected_version="0.1.2", | ||
), |
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 feel like we should make it something like this
"version_args, expected_version"
VersionSchemeTestArgs( | |
current_version="0.1.1", | |
increment="PATCH", | |
prerelease=None, | |
prerelease_offset=0, | |
devrelease=None, | |
expected_version="0.1.2", | |
), | |
( | |
VersionSchemeTestArgs( | |
current_version="0.1.1", | |
increment="PATCH", | |
prerelease=None, | |
prerelease_offset=0, | |
devrelease=None,, | |
), | |
"0.1.2" |
The expected version probably should be viewed as a different argument
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.
sounds good
56025a2
to
2aef016
Compare
…e behavior of library, improve readability with NamedTuple
2aef016
to
410d8fb
Compare
…e behavior of library
Relate #1631
Description
Checklist
Code Changes
poetry all
locally to ensure this change passes linter check and testsDocumentation Changes
poetry doc
locally to ensure the documentation pages renders correctlyExpected Behavior
Steps to Test This Pull Request
Additional Context