Skip to content
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

Make cache directory path Windows friendly #71

Merged

Conversation

andrewesweet
Copy link
Contributor

@andrewesweet andrewesweet commented Apr 20, 2023

The existing logic for building the cache directory path assumes *nix style paths when attempting to make a unique cache dir for each test fixture and step. Paths with a Windows drive prefix on the front ("C:\...") muck this up. Pathlib's '/' operator silently covers this up (under Python 3.9 on Windows, at least).

This change reasonably preserves the uniqueness logic whilst being Window friendly by taking a hash of the path to the test fixture's configuration directory tfdir and using that in the cache directory path construction.

The existing logic for building the cache directory path assumes *nix style paths when attempting to make a unique cache dir for each test fixture and step. Paths with a Windows drive prefix on the front ("C:\...") muck this up. Pathlib's '/' operator silently covers this up (under Python 3.9 on Windows, at least).

This change preserves the uniqueness logic whilst being Window friendly by taking a hash of the path to the test fixture's configuration directory `tfdir` and using that in the cache directory path construction.
@andrewesweet
Copy link
Contributor Author

I don't see how the change I introduces caused the Linting status check to fail.

@ludoo
Copy link
Collaborator

ludoo commented Apr 20, 2023

Thanks for this! Can you check the linting errors? Linter configuration is in the ci workflow files in the .github/workflows folder.

@ludoo
Copy link
Collaborator

ludoo commented Apr 20, 2023

I don't see how the change I introduces caused the Linting status check to fail.

Might be my fault even though I don't think I pushed to master outside a PR recently. Simplest solution is to fix it, I can do that if you prefer. :)

@andrewesweet
Copy link
Contributor Author

I don't see how the change I introduces caused the Linting status check to fail.

Might be my fault even though I don't think I pushed to master outside a PR recently. Simplest solution is to fix it, I can do that if you prefer. :)

Done

@ludoo ludoo merged commit a447abf into GoogleCloudPlatform:master Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants