test-cache: Fix XDG tests, make tests more consistent #880
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The tests on Linux were relying on XDG_CACHE_HOME being set to its fallback value of ∼/.cache, which made them fail on systems with custom values for XDG_CACHE_HOME. To account for this, we take the cue from the Windows tests and set XDG_CACHE_HOME to a known, custom, value. This incidentally also tests that our logic can account for custom values of XDG_CACHE_HOME, so this doesn't need its own test. The value of "/tmp/home/.cache" was chosen as a cross between the Windows value of "/tmp/AppData/Local" and the fallback "∼/.cache" Fixes: pypa#814
Several changes to the tests in test_cache.py to make them more consistent with each other - test_get_cache_dir: Follow all other tests in not casting to posix paths in checking paths are as expected, instead checking an equal Path object is roundtripped. Also, run the test simulating all platform's path logic, like the other tests. - test_get_cache_dir_old_pip: Remove logic checking whether _get_cache_dir accepts explicitly-set paths. It is duplicated from test_get_cache_dir, doesn't appear to be exercising any new codepath, and it is unclear why this case needs extra exercise - test_get_pip_cache, test_get_cache_dir_old_pip: follow the lead of test_get_cache_dir_pip_disabled_in_environment and inline the call to _get_cache_dir
|
OK, removed RFC markers from the commit messages. I'd appreciate you running the new |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solving the issues I raised in #814 (comment), this commit set (at least on my machine) fixes the tests on Linux systems with custom XDG variables.
While I was there, I noticed the tests were a little inconsistent, so this PR also contains a couple of commits addressing that. Three of the changes I'm unsure about, so I'd appreciate a second look at them before this is merged. (I've labeled this as a draft until the two RFCs below are resolved, since even if they are accepted their commit messages need to be reworded in light of that)
fix(test_cache): Correct XDG testing logic
The tests on Linux were relying on XDG_CACHE_HOME being set to its fallback value of
∼/.cache, which made them fail on systems with custom values forXDG_CACHE_HOME.To account for this, we take the cue from the Windows tests and set
XDG_CACHE_HOMEto a known, custom, value. This incidentally also tests that our logic can account for custom values ofXDG_CACHE_HOME, so this doesn't need its own test.The value of
/tmp/home/.cachewas chosen as a cross between the Windows value of/tmp/AppData/Localand the fallback∼/.cache(This commit is a MVP for the PR, the others can be ignored if desired)
Fixes: More respectful convention for caching #814
RFC: fix(test_cache): Make tests more consistent
test_get_cache_dir: Follow all other tests in not casting to posix paths in checking paths are as expected, instead checking an equal Path object is roundtripped. NOTE: This edit I'm least confident in, not least since I don't have a Windows machine available to test this on.test_get_cache_dir_old_pip: Remove logic checking whether _get_cache_dir accepts explicitly-set paths. It is duplicated fromtest_get_cache_dir, doesn't appear to be exercising any new codepath, and it is unclear why this case needs extra exercisetest_get_pip_cache,test_get_cache_dir_old_pip: follow the lead oftest_get_cache_dir_pip_disabled_in_environmentand inline the call to_get_cache_dirRFC: fix(test_cache): Cross-OS test_get_cache_dir
In view of my doubts above in re whether I broke
test_get_cache_diron Windows, run the test simulating all platforms. However, I am unsure that my understanding ofpathliband the relevant monkeypatching is correct, or if this commit is even needed.