Skip to content

Conversation

@ppinchuk
Copy link
Collaborator

@ppinchuk ppinchuk commented Nov 9, 2025

Pull Request Overview

Introduce unit tests to verify the presence and semantic correctness of the __version__ attribute in the compass module. These tests ensure compliance with the expected versioning format.

Notes

Just a few quick tests. Not super important but might help us in the long run to catch tricky edge case issues, especially when running tests on some remote mahcines

@ppinchuk ppinchuk requested a review from castelao as a code owner November 9, 2025 00:29
Copilot AI review requested due to automatic review settings November 9, 2025 00:29
@ppinchuk ppinchuk self-assigned this Nov 9, 2025
@ppinchuk ppinchuk added enhancement Update to logic or general code improvements p-low Priority: low topic-python-general Issues/pull requests related to python labels Nov 9, 2025
@ppinchuk ppinchuk added this to the Finishing touches for OSS milestone Nov 9, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds unit tests to verify the compass package exposes a properly formatted __version__ attribute. The tests ensure the version string exists, is non-empty, and follows semantic versioning with optional development version suffixes (e.g., 0.11.3.dev8+gHASH).

  • Validates the presence and type of the __version__ attribute
  • Checks that the version string matches a semantic versioning pattern with optional dev suffixes
  • Guards against placeholder version values like "9999"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@castelao castelao left a comment

Choose a reason for hiding this comment

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

Is the idea here to capture a bad pipeline that would result in a bad setup, such as a docker container built with the wrong version?

@ppinchuk
Copy link
Collaborator Author

Is the idea here to capture a bad pipeline that would result in a bad setup, such as a docker container built with the wrong version?

Yes that was what I was thinking!

@ppinchuk ppinchuk requested a review from castelao November 15, 2025 22:03
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.55%. Comparing base (73d6592) to head (abaac6b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #334      +/-   ##
==========================================
+ Coverage   36.50%   36.55%   +0.04%     
==========================================
  Files          45       45              
  Lines        4240     4240              
  Branches      380      380              
==========================================
+ Hits         1548     1550       +2     
+ Misses       2670     2669       -1     
+ Partials       22       21       -1     
Flag Coverage Δ
unittests 36.55% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ppinchuk
Copy link
Collaborator Author

I guess the downside to this test is that all our actions now have to run with a fetch-depth: 0 flag. I don't think that's too big of a deal, but I'm also not sure attached to this test if you think this would be a pain @castelao

@castelao
Copy link
Member

castelao commented Nov 17, 2025

I guess the downside to this test is that all our actions now have to run with a fetch-depth: 0 flag. I don't think that's too big of a deal, but I'm also not sure attached to this test if you think this would be a pain @castelao

I agree, I don't think it is a big deal to require fetch-depth: 0. Currently, the most important place to verify/check the version is when building the container, which we don't verify that today. We should attach the version of the container to the version of the library, so we can still play with the intermediate dev release but also be clear on what are we deploying in production.

I don't see a clear benefit with this test here, but if you want to add it, I support you.

@ppinchuk
Copy link
Collaborator Author

Let's add it and see if it gets annoying. If it does, happy to remove it

@ppinchuk ppinchuk merged commit ea07247 into main Nov 18, 2025
24 of 25 checks passed
@ppinchuk ppinchuk deleted the pp/version_tests branch November 18, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Update to logic or general code improvements p-low Priority: low topic-python-general Issues/pull requests related to python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants