Skip to content

tail: make both test files run again - #207

Merged
JalonSolov merged 1 commit into
vlang:mainfrom
metif12:fix/tail-tests
Oct 5, 2026
Merged

JalonSolov merged 1 commit into
vlang:mainfrom
metif12:fix/tail-tests

Conversation

@metif12

@metif12 metif12 commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

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_test.v:24  error: to propagate a Result, the call must also
                                return a Result type

string_to_i64('1')! == 1 used to unwrap. ! now means propagate, which needs
the 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 zero
runtime. 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_test.v: all five tests, empty against expected

tail() 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
tail() runs, so nothing was ever appended. Measured both obvious spellings 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 closures capture that. tail itself was never wrong here: tail -n 2 test.txt prints exactly what the test expected, byte for byte, and so does GNU.

One more thing, because make test builds with -W and a deprecation is an error
there: os.execute('ps ${args.pid}') is now os.exec(['ps', args.pid]). There is
no shell in it to lose, and -Wimpure-v -W now passes for this directory.

Tests

v test ./src/tail is 2 of 2 and so is v -Wimpure-v -W test ./src/tail.
v fmt -verify src/tail passes.

The whole suite is still 17 of 49 files failing on Windows; this takes it to 16.

@JalonSolov

Copy link
Copy Markdown
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.
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
JalonSolov merged commit 00ddab1 into vlang:main Oct 5, 2026
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.
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