-
-
Notifications
You must be signed in to change notification settings - Fork 733
Fix test_local_directory
and test_local_directory_make_new_directory
on Windows
#6954
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
Conversation
test_local_directory
and test_local_directory_make_new_directory
on Windows
CI flake: #6896 |
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files + 1 15 suites +1 6h 40m 22s ⏱️ + 1h 1m 55s For more details on these failures, see this check. Results for commit e259c17. ± Comparison against base commit b8d1c7b. |
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.
Any idea what caused this to fail in the first place?
dask/dask#9429 contains some context. It looks like this might be caused by CI upgrading to Python distributed/distributed/worker.py Line 577 in b8d1c7b
|
good catch! |
Did CPython intend this as a breaking change? |
assert w.local_directory.startswith(fn) | ||
assert "dask-worker-space" in w.local_directory | ||
async def test_local_directory(s, tmp_path): | ||
with dask.config.set(temporary_directory=str(tmp_path)): |
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.
Nit?: I prefer to use os.fsdecode
instead of str
for pathlib.Path objects
Good question, nonetheless, Microsoft actively discourages periods at the end of file names:
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file |
async def test_local_directory(s, tmp_path): | ||
with dask.config.set(temporary_directory=str(tmp_path)): | ||
w = await Worker(s.address) | ||
assert w.local_directory.startswith(str(tmp_path)) |
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.
Fun fact: This can be done a little bit nicer by pathlib.Path(w.local_directory)
and then using the pathlib https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.is_relative_to if someone can sort out a release of the pathlib2 backport
Closes #6934
Closes #6935
pre-commit run --all-files