Skip to content

test: make all gnu test pass and fix access issue - #14966

Merged
cakebaker merged 1 commit into
uutils:mainfrom
anastygnome:test-util
Oct 3, 2026
Merged

cakebaker merged 1 commit into
uutils:mainfrom
anastygnome:test-util

Conversation

@anastygnome

@anastygnome anastygnome commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

The implementation of test parsing was not all encompassing and missed a lot of cases from GNU. Notably test would process short expressions in a special way, which was absent in uutils. One test went against coreutils, probably because by default test uses a shell builtin.

This PR fixes all of the issues, at the expense of a comprehensive rewrite of the parser (sorry!). But it is at least a sane base to work upon.

closes #13197
closes #9147
closes #8960

@anastygnome
anastygnome force-pushed the test-util branch 2 times, most recently from 3e9defe to ec367ff Compare September 29, 2026 19:29
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 155.68 MB (+2.41 MB, +1.58%)

Significant per-binary changes:
  [        1.12 MB ->    2.32 MB  (+1.20 MB, +107.32%)
  test     1.12 MB ->    2.32 MB  (+1.20 MB, +107.32%)

@anastygnome anastygnome changed the title test: adress test failures by evaluating expressions directly and separating special cases test: rework expression parsing for GNU compatibility Sep 29, 2026
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/test/test is no longer failing!

@anastygnome
anastygnome force-pushed the test-util branch 2 times, most recently from 559b9a1 to f004f3d Compare September 29, 2026 20:45
@anastygnome
anastygnome marked this pull request as ready for review September 29, 2026 20:45
@anastygnome
anastygnome force-pushed the test-util branch 3 times, most recently from 9271b01 to 8d6deae Compare September 30, 2026 05:41
@anastygnome

Copy link
Copy Markdown
Contributor Author

@sylvestre looking good to me!

@anastygnome anastygnome changed the title test: rework expression parsing for GNU compatibility test: make all gnu test pass Sep 30, 2026
@anastygnome
anastygnome force-pushed the test-util branch 2 times, most recently from 7317cff to 7a0bbc4 Compare September 30, 2026 11:05
@anastygnome

anastygnome commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor Author

@cakebaker can you review?

@anastygnome
anastygnome force-pushed the test-util branch 3 times, most recently from 4beb8cf to 9b41762 Compare October 1, 2026 08:34
@anastygnome
anastygnome force-pushed the test-util branch 8 times, most recently from e1fe84f to 7b71279 Compare October 1, 2026 09:15
@anastygnome anastygnome changed the title test: make all gnu test pass test: make all gnu test pass and fix access issue Oct 1, 2026
@anastygnome

Copy link
Copy Markdown
Contributor Author

@xtqqczze what do you think of this?

Comment thread src/uu/test/src/parser.rs Outdated
Comment thread src/uu/test/src/test.rs Outdated
Comment thread src/uu/test/src/faccessat.rs
Comment thread src/uu/test/src/faccessat.rs
@xtqqczze

This comment was marked as resolved.

@anastygnome
anastygnome force-pushed the test-util branch 3 times, most recently from 2e94284 to 840b514 Compare October 2, 2026 16:39
@anastygnome

anastygnome commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

@cakebaker @xtqqczze I've rebased this. should be good to merge now :)

@xtqqczze xtqqczze left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs review from @sylvestre

`test` has special rules for expressions with one through four arguments.
Keeping a postfix `Symbol` stack makes those cases harder to follow and clones
`OsString`s while looking ahead.

Evaluate directly from the argument slice instead. Keep the short forms
explicit, then use the usual `!`, `-a`, `-o` precedence for longer expressions.
This also fills in GNU-compatible `-l` integer operands, locale ordering for
`<` and `>`, and the `-t` overflow case.

This matches GNU logic, and makes the test pass.

Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>
@cakebaker
cakebaker merged commit 34ee474 into uutils:main Oct 3, 2026
98 checks passed
@cakebaker

Copy link
Copy Markdown
Contributor

Congrats! The gnu test tests/test/test is no longer failing!

Kudos, good work, thanks!

@xtqqczze

xtqqczze commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

This broke the Redox targets:

$ RUSTC_BOOTSTRAP=1 cargo check -q -Zbuild-std --all-targets --features feat_os_unix_redox --target i586-unknown-redox --target x86_64-unknown-redox
error[E0432]: unresolved imports `rustix::fs::AtFlags`, `rustix::fs::accessat`
  --> src/uu/test/src/faccessat.rs:20:22
   |
20 |     use rustix::fs::{AtFlags, CWD, accessat};
   |                      ^^^^^^^       ^^^^^^^^ no `accessat` in `fs`
   |                      |
   |                      no `AtFlags` in `fs`
   |
help: a similar name exists in the module
   |
20 -     use rustix::fs::{AtFlags, CWD, accessat};
20 +     use rustix::fs::{OFlags, CWD, accessat};
   |
help: a similar name exists in the module
   |
20 -     use rustix::fs::{AtFlags, CWD, accessat};
20 +     use rustix::fs::{AtFlags, CWD, access};
   |

Unfortunately, the PR to add fs support for Redox in Rustix (bytecodealliance/rustix#1638) has been awaiting review for quite some time. Given the lack of progress, we may need to consider maintaining a fork of Rustix to unblock Redox support.

@anastygnome

Copy link
Copy Markdown
Contributor Author

@xtqqczze I wrote a separate uucore PR recently #15294 that would fix this (or at least help)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants