fix: Storage plugin bug on Windows #2487
Conversation
This commit also makes vscode test plugin work, as test discovery doesn't find all the "test_*" files which are not supposed to be run by pytest directly
| @@ -1,3 +1,5 @@ | |||
| * text=auto eol=lf | |||
There was a problem hiding this comment.
This ensures files are checked-out with linux file endings on Windows.That is usually not the recommendation for windows. But this is the best option given how many input-output files in the tests system that are manipulated by posix tools.
| @@ -0,0 +1,2 @@ | |||
| [pytest] | |||
| python_files = tests/tests.py tests/test_expand.py tests/test_io.py tests/test_schema.py tests/test_linting.py tests/test_executor_test_suite.py No newline at end of file | |||
There was a problem hiding this comment.
The test suite contains plenty of files which doesn't follow the standard pytest conventions. This tripped the vs-code test plugin which tried to inspect non-test files.
melund
changed the title
melund
force-pushed
the
master
branch
2 times, most recently
from
404f7ed to
4f72db4
Compare
melund
force-pushed
the
master
branch
2 times, most recently
from
e4810eb to
6c999ba
Compare
melund
changed the title
|
@johanneskoester. Test pass and it is ready for review. |
johanneskoester
left a comment
johanneskoester
left a comment
There was a problem hiding this comment.
Thanks a lot! Sorry, the review was done since last week, but I forgot to hit the submit button. One thing below.
|
Seems like touching directories on windows does not work anymore with the changes here. |
|
Kudos, SonarCloud Quality Gate passed!
|
I will try to roll back so the check doesn't happen inside IOFile. The test were able to pass before I did that. |
| local_path_str = str(storage_object.local_path()) | ||
| if ON_WINDOWS: | ||
| local_path_str = local_path_str.replace("\\", "/") | ||
| file_with_wildcards_applied = IOFile(local_path_str, rule=self.rule) |
There was a problem hiding this comment.
The Path object has a method as_posix() which does exactly what you do here. This also applies to other cases below. Maybe we should just always use this method?
|
I have now implemented a simpler version of this in #2519. Thanks a lot for this work here, which pointed me to the right places for a simplified fix! |
2 participants
Description
This PR attempts to fix the storage bug plugin test failures on Windows.
Fixes: #2473
@johanneskoester, I am not sure this is how you want to fix the bug, but it should give you an idea what the problem is.
Currently, on Windows all files in rules etc. are converted to forward slashes to avoid the 'backslash' mesh. However, the new storage plugin uses 'pathlib' for some paths internally. Converting pathlib.Path objects to strings gives paths with 'backslash'es on Windows. So I have added code to handle those cases.
I have also improved the CI test setup on windows. The setup time for conda environment creation is down from 12 min to 2.5 min on Windows. Much more is possible, but I have split that work into a separate PR (#2489)
The PR also does a few other things for local testing on Windows:
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).