-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Test] Fix path tests on windows #5803
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
[Test] Fix path tests on windows #5803
Conversation
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
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
This pull request fixes Windows path tests by replacing the hard-coded drive letter "D:" with the system drive value fetched dynamically from the environment.
- Replaces hard-coded "D:" with os.environ.get("SystemDrive", "C:") in test assertions.
- Improves documentation of the destination method in the library module.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
test/test_library.py | Updates to Windows path handling in tests and declaration of instance variables. |
beets/library.py | Updates the docstring to clarify behavior when the relative_to_libdir parameter is true. |
Hmm... this |
Root-caused: |
Thank you for the quick fix and find! Funny that the drive was hardcoded, I guess not too many people run the tests on a local windows machine. Could you add a changelog entry? Other than that, should be ready to merge. |
Happy to help! Changelog pushed. |
Description
Fixes #5802.
Today, tests fail on most Windows machines because we hard-code
D:
as the root drive, but most machines useC:
. This change uses the same normalization function in the test assertion to ensure the drives match.To Do
Documentation.What changed?
D:
.Item::destination()
method to document therelative_to_libdir
param.How tested?