Skip to content

groups: fixes to pass GNU's Testsuite#2446

Merged
sylvestre merged 3 commits intouutils:masterfrom
jhscheer:groups_gnu_testsuite
Jun 23, 2021
Merged

groups: fixes to pass GNU's Testsuite#2446
sylvestre merged 3 commits intouutils:masterfrom
jhscheer:groups_gnu_testsuite

Conversation

@jhscheer
Copy link
Contributor

This passes the relevant tests for groups from GNU's Testsuite that currently fail on master branch.

============================================================================
Testsuite summary for GNU coreutils 8.32.162-4eda
============================================================================
PASS: tests/misc/groups-dash.sh
PASS: tests/misc/groups-process-all.sh
  • add new feature to support multiple users (introduced in coreutils 8.31)
  • sync help text with GNU's groups manpage
  • add tests for everything

jhscheer added 2 commits June 21, 2021 13:19
* add support for multiple users
* sync help text with GNU's `groups` manpage
@jhscheer jhscheer force-pushed the groups_gnu_testsuite branch from 56d832d to 8813cbe Compare June 21, 2021 22:26
Copy link
Contributor

@rivy rivy left a comment

Choose a reason for hiding this comment

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

Some nits.

And thanks for tackling this issue!

@jhscheer
Copy link
Contributor Author

@rivy Thanks for your feedback and doing the review.

@jhscheer jhscheer force-pushed the groups_gnu_testsuite branch from 8813cbe to 2736ef8 Compare June 22, 2021 07:41
@sylvestre
Copy link
Contributor

Clippy isn't happy


error[E0425]: cannot find value `VERSION_MULTIPLE_USERS` in this scope
  --> /home/runner/work/coreutils/coreutils/tests/by-util/test_groups.rs:81:66
   |
12 | const VERSION_MIN_MULTIPLE_USERS: &str = "8.31"; // this feature was introduced in GNU's coreutils 8.31
   | ------------------------------------------------ similarly named constant `VERSION_MIN_MULTIPLE_USERS` defined here
...
81 |     let version_check_string = check_coreutil_version(util_name, VERSION_MULTIPLE_USERS);
   |                                                                  ^^^^^^^^^^^^^^^^^^^^^^
   |
help: a constant with a similar name exists
   |
81 |     let version_check_string = check_coreutil_version(util_name, VERSION_MIN_MULTIPLE_USERS);
   |                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: consider importing this constant
   |
1  | use crate::test_id::VERSION_MULTIPLE_USERS;
   |

@jhscheer jhscheer force-pushed the groups_gnu_testsuite branch from 2736ef8 to 981c0ce Compare June 22, 2021 08:04
@sylvestre
Copy link
Contributor

Cool
Move from
GNU tests summary = TOTAL: 601 / PASS: 169 / FAIL: 321 / ERROR: 25
to
GNU tests summary = TOTAL: 601 / PASS: 171 / FAIL: 319 / ERROR: 25

@jhscheer
Copy link
Contributor Author

@sylvestre #2361 also brought two new PASSES, but those were way harder to achieve.

@jhscheer jhscheer force-pushed the groups_gnu_testsuite branch from 981c0ce to 8eb27fa Compare June 23, 2021 10:13
jhscheer added a commit to jhscheer/coreutils that referenced this pull request Jun 23, 2021
* change of identifier names and spelling according to the suggestions in the review of uutils#2446
@jhscheer jhscheer force-pushed the groups_gnu_testsuite branch from 8eb27fa to 11f36ea Compare June 23, 2021 14:50
@rivy
Copy link
Contributor

rivy commented Jun 23, 2021

If you don't mind, rebase this off of #2452 when it's merged and the spelling errors will be fixed.

Thanks for the work!

@sylvestre sylvestre merged commit 3b2d0d1 into uutils:master Jun 23, 2021
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.

3 participants