Clean up skipped tests and document those still needed for win32 #5812
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR marks and documents tests that fail on Windows (and WSL on NTFS) so they are gracefully skipped, adds helper functions to detect WSL/NTFS environments, and updates the changelog accordingly.
- Added
@unittest.skipIfandpytest.mark.skipifdecorators to skip Windows-specific tests. - Introduced
is_wsl,is_path_on_ntfs, andis_wsl_and_ntfshelpers to support skipping on WSL/NTFS. - Updated
docs/changelog.rstto record the new test skips.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_ui.py | Skips UI tests on Windows with FIXME comments |
| test/test_release.py | Skips RST-to-MD conversion test on Windows |
| test/test_query.py | Removed some Windows skips to re-enable tests on Windows |
| test/plugins/test_player.py | Uses pytest.mark.skipif to skip socket-based player tests on Windows |
| test/plugins/test_play.py | Skips relative-path play test on Windows with new comment |
| test/plugins/test_permissions.py | Imports is_wsl_and_ntfs to skip permission tests on WSL/NTFS |
| test/plugins/test_convert.py | Skips convert plugin tests on Windows with pytest decorator |
| docs/changelog.rst | Notes skipped Windows and WSL/NTFS tests |
| beets/test/helper.py | Adds WSL/NTFS detection utilities |
Comments suppressed due to low confidence (1)
test/plugins/test_play.py:80
- [nitpick] This comment is vague. Clarify exactly why the test must be skipped on Windows (e.g., explain how drive-root semantics differ).
# more joys of Windows not having a root start unless you only have 1 drive
| print(f"Error reading /proc/mounts: {e}") | ||
| return False | ||
|
|
||
| return fs_type.lower() == "9p" |
There was a problem hiding this comment.
The NTFS detection is comparing the filesystem type to "9p", but WSL mounts typically use "drvfs" (or a similar fstype). Update this check to detect the actual NTFS mount type.
| return fs_type.lower() == "9p" | |
| return fs_type.lower() == "drvfs" |
Copilot uses AI. Check for mistakes.
| @@ -911,6 +911,9 @@ def test_paths_section_respected(self): | |||
| assert key == "x" | |||
| assert template.original == "y" | |||
|
|
|||
| # FIXME: fails on windows as folder is created in current folder rather than | |||
| # the tmp dir for testing. | |||
| @unittest.skipIf(sys.platform == "win32", "win32") | |||
There was a problem hiding this comment.
[nitpick] This decorator is repeated on multiple methods. Consider using a module-level pytestmark = pytest.mark.skipif(...) or a class-level decorator to DRY up the Windows skip logic.
| @unittest.skipIf(sys.platform == "win32", "win32") |
Copilot uses AI. Check for mistakes.
| @@ -911,6 +911,9 @@ def test_paths_section_respected(self): | |||
| assert key == "x" | |||
| assert template.original == "y" | |||
|
|
|||
| # FIXME: fails on windows as folder is created in current folder rather than | |||
There was a problem hiding this comment.
[nitpick] Capitalize "Windows" in the comment for consistency with platform naming.
| # FIXME: fails on windows as folder is created in current folder rather than | |
| # FIXME: fails on Windows as folder is created in current folder rather than |
Copilot uses AI. Check for mistakes.
| @@ -102,6 +103,8 @@ def md_changelog(): | |||
| - Fixed something.""" # noqa: E501 | |||
|
|
|||
|
|
|||
| # Fixme windows | |||
| @unittest.skipIf(sys.platform == "win32", "win32") | |||
There was a problem hiding this comment.
[nitpick] Tests in this suite use pytest; switch to @pytest.mark.skipif(sys.platform == 'win32', reason='Windows') for consistency with the rest of the test suite.
| @unittest.skipIf(sys.platform == "win32", "win32") | |
| @pytest.mark.skipif(sys.platform == "win32", reason="Windows") |
Copilot uses AI. Check for mistakes.
Description
Marked tests needing not to be tested on windows.
Documented why tests need to be skipped IF the code is not logical.
Most tests are more to how windows manages the filesystem compared to *nix & MacOS
The test_player tests, I have completely skipped as I think the failures are lack of sockets being easily used for the tests.
To Do
A few tests still create files in the local folder rather than in TEMP. I'll add those as updates, but looking for feedback on changes?
docs/to describe it.)docs/changelog.rstto the bottom of one of the lists near the top of the document.)