test: pin the contents kept when a symlinked .env is replaced - #691
Open
Gares95 wants to merge 1 commit into
Open
test: pin the contents kept when a symlinked .env is replaced#691Gares95 wants to merge 1 commit into
Gares95 wants to merge 1 commit into
Conversation
The six existing symlink tests all pass against an implementation that keeps the contents the path resolved to and against one that discards them, so neither behaviour is currently pinned. test_set_key_symlink_to_existing_file uses a target holding the same key that is then set, and asserts with a substring, so any prior contents are overwritten by the set and invisible either way. test_unset_key_symlink_to_existing_file has a target holding only the key that is then unset, so the result is "" whether the previous contents were read or not. Adds two tests whose targets hold a key the operation does not touch, so the kept contents are asserted exactly. No existing test is modified and no behaviour changes.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
The six existing symlink tests pass unchanged against two implementations that
behave differently, so neither behaviour is currently pinned.
When
.envis a symlink andfollow_symlinksis left at its default, the linkis replaced by a regular file (already asserted by
test_set_key_symlink_to_existing_fileandtest_unset_key_symlink_to_existing_file), and that file keeps the contents thepath resolved to beforehand. That second half is not covered, because of how the
two tests are constructed:
test_set_key_symlink_to_existing_filewrites a target holdinga=x, thensets the same key
a. Whatever was read is overwritten by the set, so theprior contents cannot show up in the result. The assertion is also
"a='y'" in symlink.read_text(), which tolerates any extra content.test_unset_key_symlink_to_existing_filewrites a target holding onlya=x,then unsets
a. The result is""whether the previous contents were read ornot.
I checked this by running the existing suite against a build that discards the
previous contents instead of keeping them: all six symlink tests still pass, and
a symlinked
.envwhose target held three keys came back holding only the newlyset one.
This adds two tests whose targets hold a key the operation does not touch, so the
kept contents are asserted exactly:
test_set_key_symlink_keeps_target_contents: target holdsb=x, seta=y,result is
b=x\na='y'\n.test_unset_key_symlink_keeps_target_contents: target holdsa=xandb=y,unset
a, result isb=y\n.Both expected values are the current output on
main, not new behaviour. Noexisting test is modified and nothing in
src/changes, so this is purelyadditional coverage.
They also mirror contracts the suite already pins for a regular file, so the
expectations are not new:
test_set_keycoversa=b\nplus settingcgivinga=b\nc='d'\n, andtest_unset_with_valuecoversa=b\nc=dminusagivingc=d. These two are the symlink equivalents.For context, I opened and then withdrew #690, which changed this read path and
would have dropped those contents. The suite stayed green throughout, which is
what prompted me to look at the coverage rather than the behaviour.