Skip to content
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

Use the semver package to parse the log version in testing #216

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

mcdonnnj
Copy link
Member

🗣 Description

This pull request moves the semver package to a testing requirement and uses it to parse the versions in the test_log_version() test.

💭 Motivation and context

Currently the version comparison relies on explicit string matching. However, I think that this should be more contextually aware. If an underlying Python package is one version and the Docker package (which is in lockstep version-wise) has a build identifier then the versions will not match. However, if a build identifier is the only differentiation between the versions I believe they should be considered equivalent. An example is that 1.0.0 and 1.0.0+build.1 should be seen as equivalent versions instead of a mismatch. Using semver to parse the versions will result in these versions being seen as equivalent. Some example version comparisons:

>>> from semver import parse_version_info
>>> old = parse_version_info("1.0.0")
>>> new = parse_version_info("1.0.0+build.1")
>>> old == new
True
>>> new = parse_version_info("1.0.0-rc.1")
>>> old == new
False

Here we see that only versions with build identifiers are considered equivalent. I think this is a reasonable adjustment of how this logic compares versions when it comes to underlying dependencies.

🧪 Testing

Automated tests pass.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

Instead of comparing the project version and the version read from the
container log directly as strings it makes sense to parse them and
contextually compare them. This is important if you are comparing, for
example, a Python package version that should match the Docker image,
but the Docker image has a build identifier (ex: `1.0.0` and
`1.0.0+build.1`). The version should be considered equivalent in this
scenario if a project is properly following SemVer versioning.
@mcdonnnj mcdonnnj added improvement This issue or pull request will add or improve functionality, maintainability, or ease of use test This issue or pull request adds or otherwise modifies test code labels Feb 15, 2025
@mcdonnnj mcdonnnj requested a review from a team February 15, 2025 22:16
@mcdonnnj mcdonnnj self-assigned this Feb 15, 2025
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. 👍

@mcdonnnj mcdonnnj added this pull request to the merge queue Feb 18, 2025
@mcdonnnj mcdonnnj added the kraken 🐙 This pull request is ready to merge during the next Lineage Kraken release label Feb 18, 2025
Merged via the queue into develop with commit 0a2e987 Feb 18, 2025
17 checks passed
@mcdonnnj mcdonnnj deleted the improvement/use_semver_for_version_comparisons branch February 18, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use kraken 🐙 This pull request is ready to merge during the next Lineage Kraken release test This issue or pull request adds or otherwise modifies test code
Projects
Development

Successfully merging this pull request may close these issues.

4 participants