Polish gnu tests #1766

jaggededgedjustice · 2021-03-07T13:56:44Z

This Makes some improvements to the gnu tests:

Apply timeout to each test so we do not need to remove any test that hangs
Use system utility in some tests that had failures due to utilities that weren't the focus of the test
Copy hashsum to create the *sum binaries so they can be tested
Find any GNU binary that has not been built and copy falseto it so that the relevant tests will fail
remove unbuffer from the test run as it was causing hangs that even the top timeout didn't kill and it didn't offer noticeable benefit

Move to a timeout applied to each script and re add the tests that are hanging so they show as failing

This causes the make process to hang for some reason, and it itsn't providing any real advantage so it's taken out

This reverts commit bbce179.

This reverts commit 8dae8b7.

sylvestre · 2021-03-07T14:01:07Z

.github/workflows/GNU.yml

@@ -83,7 +104,7 @@ jobs:
        GNULIB_DIR="${PWD}/gnulib"
        pushd gnu

-        unbuffer timeout -sKILL 3600 make -j "$(nproc)" check SUBDIRS=. RUN_EXPENSIVE_TESTS=yes RUN_VERY_EXPENSIVE_TESTS=yes VERBOSE=no || :
+        timeout -sKILL 4h make -j "$(nproc)" check SUBDIRS=. RUN_EXPENSIVE_TESTS=yes RUN_VERY_EXPENSIVE_TESTS=yes VERBOSE=no || : # Kill after 4 hours in case something gets stuck in make


4h ? :)
1h seems enough, no ?

not really, run time for the 'Run GNU tests' stage was 1 hour 9 minutes on the last commit before I opened the PR (adding some shorter timeouts on a few more tests known to hang could speed things up a bit). I'm not worried about making this as sort as possible, with the timeout being applied per-test this was just left as a last resort in case something in make gets jammed.
It turns out there's a built in 6 hour limit on github actions, so we could also take this out and rely on that if something does get break.

Maybe 2 hours then. Also, expect can now be removed from the apt-get install line above, since you are removing the unbuffer command.

jaggededgedjustice · 2021-03-07T14:07:42Z

@tdulcet what do you think of these changes?

There are some tests that currently fail due to differences in the exact wording of error messages, which I'm not sure what to do with. They shouldn't be removed because they include tests on behaviour that must be kept, but editing the messages being checked against might be a lot of work to keep up to date as people change the code.

sylvestre · 2021-03-07T15:43:35Z

With this patch, we are at PASS: 121 (used to be PASS: 141)
is that expected?

jaggededgedjustice · 2021-03-07T16:18:52Z

Those are probably the tests that were running on utils that aren't built in this repo, those would have been using the system util before and passing. Let me just review the changes to confirm that is what's happened.

jaggededgedjustice · 2021-03-07T17:19:56Z

I found 2 tests that were failing because they were using utils that aren't built so they were using false. There's also some problem with the stty tests, which now complain they don't have controlling tty, I need to investigate what's up with that.
Otherwise the reduced passes are legitimate.

tdulcet

@tdulcet what do you think of these changes?

Your changes look good. Thanks for doing this! I just have some minor feedback.

tdulcet · 2021-03-08T08:39:03Z

.github/workflows/GNU.yml

+        for binary in $(./build-aux/gen-lists-of-programs.sh --list-progs)
+        do
+          bin_path="${BUILDDIR}/${binary}"
+          test -f "${bin_path}" || cp "${BUILDDIR}/false" "${bin_path}"


Maybe output the names of all binaries that are not currently built for reference. Something like this:

test -f "${bin_path}" || { echo "'${binary}' was not built with uutils, using the 'false' program"; cp "${BUILDDIR}/false" "${bin_path}"; }

tdulcet · 2021-03-08T09:12:05Z

.github/workflows/GNU.yml

        # Change the PATH in the Makefile to test the uutils coreutils instead of the GNU coreutils
        sed -i "s/^[[:blank:]]*PATH=.*/  PATH='${BUILDDIR//\//\\/}\$(PATH_SEPARATOR)'\"\$\$PATH\" \\\/" Makefile
        sed -i 's| tr | /usr/bin/tr |' tests/init.sh
        make
        # Generate the factor tests, so they can be fixed
-        for i in $(seq -w 1 36)
+        for i in $(seq -w 0 36)


This can be done in Bash without seq:

for i in {00..36}

tdulcet · 2021-03-08T10:07:30Z

.github/workflows/GNU.yml

+        sed -i -e "s|stat|$(which stat)|" tests/chgrp/basic.sh tests/cp/existing-perm-dir.sh tests/touch/60-seconds.sh tests/misc/sort-compress-proc.sh tests/touch/60-seconds.sh
+        sed -i -e "s|ls -|$(which ls) -|" tests/chgrp/posix-H.sh tests/chown/deref.sh tests/cp/same-file.sh tests/misc/mknod.sh tests/mv/part-symlink.sh
+        sed -i -e "s|mkdir |$(which mkdir) |" tests/cp/existing-perm-dir.sh tests/rm/empty-inacc.sh
+        sed -i -e "s|timeout |$(which timeout) |" tests/cp/parent-perm-race.sh tests/ls/infloop.sh tests/misc/sort-exit-early.sh tests/misc/sort-NaN-infloop.sh tests/misc/uniq-perf.sh tests/tail-2/inotify-only-regular.sh tests/tail-2/pipe-f2.sh tests/tail-2/retry.sh tests/tail-2/symlink.sh tests/tail-2/wait.sh tests/tail-2/pid.sh tests/dd/stats.sh
+        sed -i -e "s| timeout | $(which timeout) |" tests/tail-2/inotify-rotate.sh # Don't break the function called 'grep_timeout'
+        sed -i -e "s|chmod |$(which chmod) |" tests/du/inacc-dir.sh tests/mkdir/p-3.sh
+        sed -i -e "s|sort |$(which sort) |" tests/ls/hyperlink.sh tests/misc/test-N.sh
+        sed -i -e "s|split |$(which split) |" tests/misc/factor-parallel.sh
+        sed -i -e "s|truncate |$(which truncate) |" tests/split/fail.sh
+
+        #Add specific timeout to tests that currently hang to limit time spent waiting
+        sed -i -e "s|seq \\$|$(which timeout) 0.1 seq \$|" tests/misc/seq-precision.sh tests/misc/seq-long-double.sh
+        sed -i -e "s|cat |$(which timeout) 0.1 cat |" tests/misc/cat-self.sh


The -e flags are not needed here. The which commands are also not needed, since the GNU coreutils should always be in /usr/bin/.

There was some problem with commands not being found which is why I started using which, though that might have been a typo so I'll have another try.

tdulcet · 2021-03-08T10:18:41Z

.github/workflows/GNU.yml

@@ -83,7 +104,7 @@ jobs:
        GNULIB_DIR="${PWD}/gnulib"
        pushd gnu

-        unbuffer timeout -sKILL 3600 make -j "$(nproc)" check SUBDIRS=. RUN_EXPENSIVE_TESTS=yes RUN_VERY_EXPENSIVE_TESTS=yes VERBOSE=no || :
+        timeout -sKILL 4h make -j "$(nproc)" check SUBDIRS=. RUN_EXPENSIVE_TESTS=yes RUN_VERY_EXPENSIVE_TESTS=yes VERBOSE=no || : # Kill after 4 hours in case something gets stuck in make


Maybe 2 hours then. Also, expect can now be removed from the apt-get install line above, since you are removing the unbuffer command.

tdulcet · 2021-03-08T10:29:40Z

.github/workflows/GNU.yml

@@ -83,7 +104,7 @@ jobs:
        GNULIB_DIR="${PWD}/gnulib"


This and the above variable is not used. Same with SKIP and XPASS variables below.

tdulcet · 2021-03-08T10:30:20Z

.github/workflows/GNU.yml

@@ -38,42 +38,63 @@ jobs:
        pushd uutils
        make PROFILE=release
        BUILDDIR="$PWD/target/release/"
+        cp ${BUILDDIR}/install ${BUILDDIR}/ginstall # The GNU tests rename this script before running, to avoid confusion with the make target


${BUILDDIR} should be quoted in case the path contains spaces.

sylvestre · 2021-03-08T19:09:22Z

The log is huge (207m), would it be possible to trim some of the content ?
For example, thousands of line of

2021-03-07T15:17:56.3511831Z +999999999999999943801810948794571024057224129020550531544123892056457216
2021-03-07T15:17:56.3512143Z +999999999999999943801810948794571024057224129020550531544123892056457216
2021-03-07T15:17:56.3512467Z +999999999999999943801810948794571024057224129020550531544123892056457216
..

jaggededgedjustice · 2021-03-08T20:35:35Z

I'm not sure about how to trim that, it could be deleted from the log that gets included in the 'test-report' archive I guess but it'd still get printed in the output. The best fix is to wait for @Arcterus to finish the fix for #1703.

Arcterus · 2021-03-09T11:06:25Z

Yeah, sorry for the delay on that. I should have time this weekend to finish cleaning it up. It’s in a pretty decent state already, so I don’t think it’ll take very long to do so.

chadbrewbaker · 2021-03-13T03:03:52Z

Can we log the GNU tests to flat shell scripts to decouple them from the Perl like the AWK test suite?

https://blog.janestreet.com/testing-with-expectations/

https://www.cs.princeton.edu/courses/archive/spring01/cs333/awktest.html

chadbrewbaker · 2021-03-13T03:10:21Z

The log is huge (207m), would it be possible to trim some of the content ?
For example, thousands of line of

2021-03-07T15:17:56.3511831Z +999999999999999943801810948794571024057224129020550531544123892056457216
2021-03-07T15:17:56.3512143Z +999999999999999943801810948794571024057224129020550531544123892056457216
2021-03-07T15:17:56.3512467Z +999999999999999943801810948794571024057224129020550531544123892056457216
..

Large logs should be compressed by default. Personal preference for zstd because you can give it a dictionary to reduce the size even more.

jaggededgedjustice · 2021-03-13T13:20:18Z

Can we log the GNU tests to flat shell scripts to decouple them from the Perl like the AWK test suite?

https://blog.janestreet.com/testing-with-expectations/

https://www.cs.princeton.edu/courses/archive/spring01/cs333/awktest.html

The idea here is to run the GNU tests so we can compare the current implementation to the GNU implementation, so we want to be taking their tests directly so that this is kept up to date with the current behaviour of the GNU coreutils. This is why the tests are taken from a current checkout of the GNU repo, and means we should be looking to make the fewest changes necessary to make sure they run correctly. Converting tests from Perl to shell scripts would require us to copy the tests into this repo and then try to maintain them ourselves. I do not think that is something that should be done. Further, even if we were to decide to maintain our own fork of the GNU tests I don't see what benefit converting existing tests from Perl to shell would provide.

Large logs should be compressed by default. Personal preference for zstd because you can give it a dictionary to reduce the size even more.

Github will be compressing the archive on their backend (I assume, it'd be the only way to keep storage manageable) so I'm not sure adding our own compression would help. Further it'd mean anyone attempting to access the archive will now need to deal with 2 layers of compression (the zst on individual files plus the zip you download the archive as). The best solution is to fix the bug that is causing seq to hang so there aren't any large logs to worry about.

sylvestre · 2021-03-20T09:44:19Z

@jaggededgedjustice can it be merged?
thanks

jaggededgedjustice · 2021-03-21T15:17:45Z

I think this can be merged now, I'm just going to do a final review of the results after the latest changes.

sylvestre · 2021-03-21T20:28:40Z

@jaggededgedjustice looks good to me :)
please let me know if you want me to land it!

sylvestre · 2021-03-22T15:50:01Z

@jaggededgedjustice can it be merged? It is green
thanks

jaggededgedjustice · 2021-03-22T22:03:38Z

ok, this is good to go.

tdulcet · 2021-03-23T10:16:57Z

The GNU tests are failing with this error:

gawk: cmd. line:1: (FILENAME=- FNR=173) fatal: node.c:415:make_str_node: r->stptr: can't allocate -2147483645 bytes of memory (Cannot allocate memory)

before they finish and output the results, which is why "Extract test info" step is failing and 4352d47 was necessary.

sylvestre · 2021-03-23T10:26:15Z

@tdulcet see #1842 (comment) from @nbraud

jaggededgedjustice · 2021-03-23T10:30:37Z

Issue #1879 is the root cause of the failure in gawk.

jaggededgedjustice · 2021-03-23T18:57:16Z

Though following on from the comment on the other PR, would it be better to have the GNU tests run on a cron rather than every PR? It's mostly an informational resource at the moment and isn't useful for deciding if a PR is good to merge as basically everything is failing.

jaggededgedjustice added 25 commits February 20, 2021 15:07

Re-add fixed test

eb8cdcf

Stop tests failing on utils that aren't the focu of the test

a73c34c

Fix script name

b3dea73

Fix script name

910f6d1

Use which to find system utils

f2013e4

Rename install for testing

cebb58c

Move timeout to per-test script

4e90de4

Move to a timeout applied to each script and re add the tests that are hanging so they show as failing

Typo

89f7494

Use system timeout command

16c8b3c

Add 4 hour global timeout for protection

b92b88a

Stop seq-precision.sh causing jams in make

dc49415

Shorten the timeout on seq-precision

b19afeb

Try removing seq-precision

e89387c

Keep producing logs even if make hangs

4cca2b6

Add timeouts to other tests that hang

1cef9aa

More tight timeouts

03619d8

Remove unbuffer

bbce179

This causes the make process to hang for some reason, and it itsn't providing any real advantage so it's taken out

Create *sum binaries for tests

a395af7

Use system sha1sum in factor tests

2008297

generate all factor scripts

3b93995

fail tests for any binary not built

e42479b

Fix tests still failing for the wrong reason

b098bd5

Revert "Remove unbuffer"

8dae8b7

This reverts commit bbce179.

Merge branch 'master' into polish-gnu-tests

64e0b00

Revert "Revert "Remove unbuffer""

78ec6d1

This reverts commit 8dae8b7.

sylvestre reviewed Mar 7, 2021

View reviewed changes

tdulcet reviewed Mar 8, 2021

View reviewed changes

jaggededgedjustice added 4 commits March 14, 2021 15:32

Use system utils

d8c3d1d

clean up sed commands

7c219fd

clean up workflow script

570e456

Merge branch 'master' into polish-gnu-tests

c804ec3

jaggededgedjustice added 3 commits March 20, 2021 22:15

Remove use of which to set system binaries

0a661a6

Don't fail the job if test-suite.log is missing

4352d47

Fix typo

0f77b54

jaggededgedjustice added 2 commits March 21, 2021 20:49

Fix more problems with utils that aren't being tested

40677bd

Fix yaml syntax

027d5e6

Fix test setup

56da6b4

jaggededgedjustice marked this pull request as ready for review March 22, 2021 19:17

sylvestre merged commit 9ffcfcd into uutils:master Mar 22, 2021

jaggededgedjustice deleted the polish-gnu-tests branch March 23, 2021 19:53

tertsdiepraam mentioned this pull request Aug 22, 2023

build-gnu.sh: /usr/bin/timeout should not be hardcoded to /usr/bin location #5193

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Polish gnu tests #1766

Polish gnu tests #1766

jaggededgedjustice commented Mar 7, 2021

sylvestre Mar 7, 2021

jaggededgedjustice Mar 7, 2021

tdulcet Mar 8, 2021

jaggededgedjustice commented Mar 7, 2021 •

edited

Loading

sylvestre commented Mar 7, 2021

jaggededgedjustice commented Mar 7, 2021

jaggededgedjustice commented Mar 7, 2021 •

edited

Loading

tdulcet left a comment

tdulcet Mar 8, 2021

tdulcet Mar 8, 2021

tdulcet Mar 8, 2021 •

edited

Loading

jaggededgedjustice Mar 8, 2021

tdulcet Mar 8, 2021

tdulcet Mar 8, 2021

tdulcet Mar 8, 2021

sylvestre commented Mar 8, 2021

jaggededgedjustice commented Mar 8, 2021

Arcterus commented Mar 9, 2021

chadbrewbaker commented Mar 13, 2021

chadbrewbaker commented Mar 13, 2021

jaggededgedjustice commented Mar 13, 2021 •

edited

Loading

sylvestre commented Mar 20, 2021

jaggededgedjustice commented Mar 21, 2021

sylvestre commented Mar 21, 2021

sylvestre commented Mar 22, 2021

jaggededgedjustice commented Mar 22, 2021

tdulcet commented Mar 23, 2021

sylvestre commented Mar 23, 2021

jaggededgedjustice commented Mar 23, 2021

jaggededgedjustice commented Mar 23, 2021

Polish gnu tests #1766

Polish gnu tests #1766

Conversation

jaggededgedjustice commented Mar 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaggededgedjustice commented Mar 7, 2021 • edited Loading

sylvestre commented Mar 7, 2021

jaggededgedjustice commented Mar 7, 2021

jaggededgedjustice commented Mar 7, 2021 • edited Loading

tdulcet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tdulcet Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sylvestre commented Mar 8, 2021

jaggededgedjustice commented Mar 8, 2021

Arcterus commented Mar 9, 2021

chadbrewbaker commented Mar 13, 2021

chadbrewbaker commented Mar 13, 2021

jaggededgedjustice commented Mar 13, 2021 • edited Loading

sylvestre commented Mar 20, 2021

jaggededgedjustice commented Mar 21, 2021

sylvestre commented Mar 21, 2021

sylvestre commented Mar 22, 2021

jaggededgedjustice commented Mar 22, 2021

tdulcet commented Mar 23, 2021

sylvestre commented Mar 23, 2021

jaggededgedjustice commented Mar 23, 2021

jaggededgedjustice commented Mar 23, 2021

jaggededgedjustice commented Mar 7, 2021 •

edited

Loading

jaggededgedjustice commented Mar 7, 2021 •

edited

Loading

tdulcet Mar 8, 2021 •

edited

Loading

jaggededgedjustice commented Mar 13, 2021 •

edited

Loading