build-gnu: fix factor tests being re-added by automake during make check#10907
Conversation
|
GNU testsuite comparison: |
|
Just stop caching GNU coreutils build? |
|
Might be a good option, |
|
Actuall, cache is working only on forks and 2nd push. So caches are not useful. |
|
I think theres an issue here, when gnu is being pulled from the cache now it is rebuilding when the tests are being run and causing the gnu tests to run on the gnu binaries |
|
I'm OK to drop GitHub cache from *.yml. Author is me. |
|
Is my understanding correct that even before this PR it was still faster to run the workflow without the caching because the rebuild issue? For example: https://github.com/uutils/coreutils/actions/runs/22039759086/job/63678632753 this one with no cache and https://github.com/uutils/coreutils/actions/runs/22039759086/job/63678632753 this one with the cache. It seems that when the cache is hit because it rebuilds the test files it takes longer to run the tests so even if we remove the cache it would still be faster to keep this PR? |
|
i don't know |
This isn't the most elegant of fixes, but the reason that we see varying integration test numbers is that if we don't re-use the gnu build from the cache then before we run the tests we will regenerate the test files again which includes all of the files that we patched out and all of the factor tests. This removes all of the patches to the Makefile and also causes the test suite to take double the amount of time to run.
The three changes are: changing the factor total test count to 40 from 37 since this was update upstream, removing all of these tests from the tests/local.mk so that even if in the future we regress and rebuild the tests again it will not have those factor tests, then lastly touching Makefile.in Makefile after all of the modifications to the build files so that when we run make-check it wont rebuild the test files again.
I made a branch to stress test this to validate that it fixes the bug and it appears that it does, but at the same time the conditions don't match the main CI exactly so I'm not 100% confident that this fixes it completely.