Repository navigation
test: stop four fixtures asserting Unix-shaped assumptions - #562
Merged
Merged
Conversation
Four of the twelve known windows failures were fixtures encoding assumptions that only hold on Unix. None is a product defect; all four assert something true in one place and meaningless in the other. test_compensation.star forced its copy to fail by aiming at "/dev/null/impossible/path.txt", which fails on Unix because /dev/null exists and is not a directory. On windows that names an ordinary path, so the copy SUCCEEDED, compensation never ran, and the test quietly created directories at the root of the current drive. It now fails the copy the way the gather test already does -- a destination whose parent is a regular file, which no platform will make a directory of. test_shell_exec.star and test_source.star interpolate t.tmp(...) into an `sh -c` command string. t.tmp returns filepath.Join, so on windows that path is native, and sh consumes its backslashes as escapes: C:\Users\RUNNER~1\... arrives as C:UsersRUNNER~1..., a drive-relative path, and the redirect writes somewhere unintended. Single-quoting the destination fixes both, because POSIX single quotes preserve backslashes and the MSYS runtime forwards a Windows path to the Win32 API unconverted -- path translation was never involved, since it applies to arguments handed to native executables and a redirect is resolved by sh itself. That escaping hazard is not confined to fixtures; pkg/platform concatenates package names into shell strings the same way and runs some of them under sudo. Filed as #561, where the remedy is argv rather than better quoting. TestGatherFailureUnwind_ViaPublicAPI asserted the run error names "boom.txt". The write fails while creating boom's parent, and the platforms name different halves of that failure: windows reports `mkdirat blocker: file exists`. The assertion now looks for "blocker", which both name, and which only the boom item has -- identifying the failing iteration just as precisely. Baseline expected to move 12 -> 8. Refs #373.
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.
Four of the twelve known windows failures were fixtures encoding assumptions that hold only on Unix. None is a
product defect — each asserts something true in one place and meaningless in the other.
test_compensation.star— the failure never happenedIt forced the copy to fail by targeting
/dev/null/impossible/path.txt, which fails on Unix because/dev/nullexists and is not a directory. On Windows that names an ordinary path, so the copy succeeded, compensation
never ran, and the test was quietly creating directories at the root of the current drive.
It now fails the copy the way
gather_api_test.goalready does — a destination whose parent is a regular file,which no platform will make a directory over.
test_shell_exec.starandtest_source.star— sh ate the pathBoth interpolate
t.tmp(...)into ansh -ccommand string.t.tmpreturnsfilepath.Join(...), so on Windowsthat path is native and backslash is sh's escape character:
a drive-relative path, so the redirect wrote somewhere unintended and
expect_filereported it missing.Single-quoting the destination fixes both. POSIX single quotes preserve backslashes literally, and the MSYS
runtime forwards a Windows path to the Win32 API unconverted. Path translation was never involved — it
applies to arguments handed to native executables, and a redirect is resolved by
shitself.The same hazard exists in production:
pkg/platformconcatenates package names into shell strings and runs someunder
sudo. Filed as #561, where the remedy is argv rather than better quoting.TestGatherFailureUnwind_ViaPublicAPI— the error names the parentIt asserted the run error contains
boom.txt. The write fails while creating boom's parent, and the twoplatforms name different halves of that failure — Windows reports
mkdirat blocker: file exists.The assertion now looks for
blocker, which both platforms name and which only the boom item has, so itidentifies the failing iteration just as precisely. Verified against the Unix error before changing it.
Verified
make check— cleanRefs #373.