test: add version test module#560
Open
medubelko wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated unit-test module around the project’s __version__ attribute and a “major.minor” extraction helper intended to mirror (future) docs toolbar version formatting, as a step toward addressing issue #532.
Changes:
- Added
tests/unit/starbase/test_version.pywith checks forstarcraft.__version__and a helper that extractsMAJOR.MINORfrom version strings. - Removed the
test_versionassertion fromtests/unit/starbase/test_init.py(moved into the new module).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| tests/unit/starbase/test_version.py | New unit tests for __version__ and major/minor parsing logic. |
| tests/unit/starbase/test_init.py | Removes version assertion now covered by the new test module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
medubelko
commented
Jun 13, 2026
| """Fixture method that duplicates version trimming in docs/conf.py.""" | ||
|
|
||
| def _trim(version): | ||
| expression = re.compile(r"\d+\.\d+") |
Contributor
Author
There was a problem hiding this comment.
Is calling re excessive here? I use it as a crutch, but I feel like it might be overkill.
lengau
reviewed
Jun 15, 2026
Contributor
Author
|
On second look, this might belong in the existing version integration test. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The version attribute inside a craft (
craft.__version__) is generated by Setuptools, so it's outside of the scope of the modules. The docs, however, rely on it for the version in the toolbar.This PR adds a standalone test module for the version string, and checks against common values. It mocks a function that I plan to add to
conf.py, but isn't implemented yet.This is a first step toward fixing #532.
Concerns
There's nothing to guarantee
conf.pywill have this function. We're creating a value in the docs, then mocking the logic in an unrelated test, so there's no coupling.I think it would be more ideal if each craft could provide something like
craft.__version_major_minor__, so the logic would reside in the app itself, not the docs. Is there a simple place to put that?make lint && make test.