Repository navigation
tail: make both test files run again - #207
Merged
Merged
Conversation
This was referenced Oct 4, 2026
Contributor
|
Conflicts. |
metif12
added a commit
to metif12/coreutils
that referenced
this pull request
Oct 5, 2026
`make test` is `v -Wimpure-v -W test .`, and on current V it could not compile 31 of the 49 test files: common/testing/testrig.v:211: error: function `execute` has been deprecated; use os.exec with an argument array; command strings can allow shell injection -W turns a deprecation into an error, so every test file that went through the harness, and every file that called os.execute itself, was failing to build rather than running. That is what the three Linux workflows do, and what this unblocks. os.exec takes an argument array, so a command line has to be split somewhere. Every command in this repository is written as a string by the tests, so this adds the one place that does it: split_args.v whitespace between words, single and double quotes, and nothing else Not a shell, on purpose. A backslash is an ordinary character everywhere in it, because on this platform a backslash is nearly always part of a path and a shell's rule of eating it would take the volume off `C:\Users\me\file.txt`. The shapes it does have to handle are the ones the tests actually write, and each is pinned by a test taken from a real call site: --output-delimiter="|" comm_test.v sh -c "exit 42" timeout_test.v tr -d '\r' sum_test.v C:\Users\me\coreutils\test.txt the reason it is not a shell An unterminated quote is not an error: the rest of the line becomes one word, so a test that made the mistake fails on what the program said rather than on a parse in this file. 124 call sites, 22 files. `v -Wimpure-v -W test .` on Windows: before 31 files could not be built at all after 2 and the suite goes from 25 files failing to 17, with those 17 being real test failures rather than build failures. The two that still do not build are fixed elsewhere: tail's string_to_i64_test.v is vlang#207 and sum's is vlang#215. Nothing that a test asserts. The commands are the same strings, the arguments are the same arguments, and no test was reworded; the difference is only that the split is done in V rather than by a shell. Twenty files under src/ and the three under common/testing/ and no other.
metif12
added a commit
to metif12/coreutils
that referenced
this pull request
Oct 5, 2026
Every test in this file compared an empty list against its expected output, and the failure named a file argument rather than the thing that was wrong: fmt: failed to open file "Now is the time for all good" fmt is handed a closure to write through, and setup() built one that appended to a `&[]string` pointing at a local in setup(). That local is gone by the time fmt runs, so nothing was ever appended. Measured on current V: closure appending to a captured `mut []string` reads back empty closure appending through a `&[]string` reads back empty, and crashes in array__push A pointer to a heap struct does share, so the sink is a `&Sink` with a `mut` field and the two closures capture that. The same trap in tail_test.v is vlang#207. fmt itself was never wrong here; it is handed the file, it reads it, and what came back was being thrown away. src/fmt is 1 of 1 and `v fmt -verify src/fmt` passes.
metif12
force-pushed
the
fix/tail-tests
branch
from
October 5, 2026 18:18
9f0b4e7 to
310b850
Compare
JalonSolov
pushed a commit
that referenced
this pull request
Oct 5, 2026
Every test in this file compared an empty list against its expected output, and the failure named a file argument rather than the thing that was wrong: fmt: failed to open file "Now is the time for all good" fmt is handed a closure to write through, and setup() built one that appended to a `&[]string` pointing at a local in setup(). That local is gone by the time fmt runs, so nothing was ever appended. Measured on current V: closure appending to a captured `mut []string` reads back empty closure appending through a `&[]string` reads back empty, and crashes in array__push A pointer to a heap struct does share, so the sink is a `&Sink` with a `mut` field and the two closures capture that. The same trap in tail_test.v is #207. fmt itself was never wrong here; it is handed the file, it reads it, and what came back was being thrown away. src/fmt is 1 of 1 and `v fmt -verify src/fmt` passes.
JalonSolov
pushed a commit
that referenced
this pull request
Oct 6, 2026
…t V (#218) `make test` is `v -Wimpure-v -W test .`, and on current V it could not compile 31 of the 49 test files: common/testing/testrig.v:211: error: function `execute` has been deprecated; use os.exec with an argument array; command strings can allow shell injection -W turns a deprecation into an error, so every test file that went through the harness, and every file that called os.execute itself, was failing to build rather than running. That is what the three Linux workflows do, and what this unblocks. os.exec takes an argument array, so a command line has to be split somewhere. Every command in this repository is written as a string by the tests, so this adds the one place that does it: split_args.v whitespace between words, single and double quotes, and nothing else Not a shell, on purpose. A backslash is an ordinary character everywhere in it, because on this platform a backslash is nearly always part of a path and a shell's rule of eating it would take the volume off `C:\Users\me\file.txt`. The shapes it does have to handle are the ones the tests actually write, and each is pinned by a test taken from a real call site: --output-delimiter="|" comm_test.v sh -c "exit 42" timeout_test.v tr -d '\r' sum_test.v C:\Users\me\coreutils\test.txt the reason it is not a shell An unterminated quote is not an error: the rest of the line becomes one word, so a test that made the mistake fails on what the program said rather than on a parse in this file. 124 call sites, 22 files. `v -Wimpure-v -W test .` on Windows: before 31 files could not be built at all after 2 and the suite goes from 25 files failing to 17, with those 17 being real test failures rather than build failures. The two that still do not build are fixed elsewhere: tail's string_to_i64_test.v is #207 and sum's is #215. Nothing that a test asserts. The commands are the same strings, the arguments are the same arguments, and no test was reworded; the difference is only that the split is done in V rather than by a shell. Twenty files under src/ and the three under common/testing/ and no other.
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.
tail: make both test files run again
Neither test file in this directory had been running. One did not compile, and
the other compared an empty string against its expected output five times.
string_to_i64('1')! == 1used to unwrap.!now means propagate, which needsthe enclosing function to return a Result, so the file has not compiled since
that change and
v test .has been reporting it as a runtime failure with a zeroruntime. It is the only file in this repository that wrote it that way. Two
helpers, converted() and rejected(), replace the twenty-odd unwraps; the
assertions themselves are unchanged.
tail() is handed a closure to write through, and setup() built one that appended
to a
&[]stringpointing at a local in setup(). That local is gone by the timetail() runs, so nothing was ever appended. Measured both obvious spellings on
current V:
A pointer to a heap struct does share, so the sink is a
&Sinkwith amutfield and the closures capture that. tail itself was never wrong here:
tail -n 2 test.txtprints exactly what the test expected, byte for byte, and so does GNU.One more thing, because
make testbuilds with -W and a deprecation is an errorthere:
os.execute('ps ${args.pid}')is nowos.exec(['ps', args.pid]). There isno shell in it to lose, and -Wimpure-v -W now passes for this directory.
Tests
v test ./src/tailis 2 of 2 and so isv -Wimpure-v -W test ./src/tail.v fmt -verify src/tailpasses.The whole suite is still 17 of 49 files failing on Windows; this takes it to 16.