Skip to content

uniq: pass remaining GNU tests#5994

Merged
sylvestre merged 7 commits intouutils:mainfrom
zhitkoff:uniq-obs
Feb 25, 2024
Merged

uniq: pass remaining GNU tests#5994
sylvestre merged 7 commits intouutils:mainfrom
zhitkoff:uniq-obs

Conversation

@zhitkoff
Copy link
Copy Markdown
Contributor

@zhitkoff zhitkoff commented Feb 21, 2024

This PR makes the following changes in order to pass GNU tests/uniq/uniq.pl:

  1. Rework of how obsolete options are handled - following the same approach as was done for split. Unfortunately it is not possible to implement it with just Clap for all required test cases
  2. In support of (1) above - introduces a module posix with 3 constants and a single function to return POSIX version based on _POSIX2_VERSION environment variable. This module would be needed to add similar functionality to sort, tail and touch later on (most likely to finalize their GNU tests compliance as well)
  3. Rework of how input is processed - switching from requiring UTF8 (and failing if input has invalid UTF8 characters) to processing raw u8 bytes sequence. This is required to pass GNU schar test case and in general aligns the behavior with GNU version
  4. Override some Clap errors to match the exact wording of error messages hardcoded into GNU tests
  5. More tests to cover the changes

Should also fix #3509

@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

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

@zhitkoff
Copy link
Copy Markdown
Contributor Author

@sylvestre checking in to see if you have any other comments / suggestions on this PR

//!
use std::env;

pub const OBSOLETE: usize = 199209;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about using an enum here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, initially i tried to implement it as an enum, but discarded in favor of these constants - unfortunately it is inconsistently checked in GNU, so was easier to implement this way for now. Might need to give it another try later on as these will be needed to fully cover GNU tests for at least 2 more utilities

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok, thanks

@sylvestre
Copy link
Copy Markdown
Contributor

Impressive work!

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.

test_uniq::gnu_tests often fails with failed to write to stdin of child: Broken pipe

2 participants