[herd] report timeouts when running tests - #1976
Open
psafont wants to merge 7 commits into
Open
Conversation
psafont
marked this pull request as draft
August 27, 2026 15:21
psafont
marked this pull request as ready for review
August 27, 2026 15:40
psafont
force-pushed
the
dev/pau/test-timeouts-fail
branch
7 times, most recently
from
September 3, 2026 15:26
f81073b to
f5293bc
Compare
Collaborator
Do timeouts imply the corresponding test target is failed (i.e. does it exit with non-zero code)? |
psafont
marked this pull request as draft
September 3, 2026 15:32
psafont
force-pushed
the
dev/pau/test-timeouts-fail
branch
from
September 3, 2026 15:41
3221da6 to
e7e65d7
Compare
Contributor
Author
Yes, herd_catalogue_regression_test is ignoring the error code and exiting with 0 on purpose to keep behaviour for tests.
I think this should be possible by adding an -exit to the test harnesses. I was afraid that such a change could break exiting behaviour, so the switch should be concerted |
psafont
force-pushed
the
dev/pau/test-timeouts-fail
branch
3 times, most recently
from
September 4, 2026 15:23
0c504c7 to
2e1771c
Compare
Depending on the timing of the timeout signal, the exception was captured by a wildcard match, masking the timeout failure and showing an unrelated error. There might be other cases hidden throughout the code, and it might even be raised in the third-party dependencies, making timeout reporting difficult to do in all cases. A different design for reporting timeouts is needed, but making one that doesn't cause churn might be too disruptive. In particular, cooperative, polled timers might be interesting to investigate. Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@arm.com>
These were using "" as a special value to mean None, a fact which was not mentioned in the interface of the functions. Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@arm.com>
Programs running commands are now forced to deal with the errors. With the previous code that used exceptions, they were not always handled, and in some cases they were handled even if the code could not raise them. Now more programs are more aware of the different ways that a herd run can fail, this includes the mismatches between expected files and herd output. This handling of error can also be used the future to build complex control flows and better integrate it with mapply. For the time being, only a single error is being surfaced when running mapply. Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@arm.com>
The value is 128 + SIGVTALRM, following the convention of bash when it's interrupted by a signal, which is 128 + signal. The internal tools are modified as well to report the timeout immediately in a list of tests, while allowing the makefile-based tests to continue. Timeouts for catalog tests are reported for the whole directory, more work needs to be done on mapply to be able to report only the tests that timed out. Example of current output of `make test`: ``` Warning: tests timed out: catalogue/aarch64-mixed/tests/2+2W+posb0b0+posb1b1.litmus, catalogue/aarch64-mixed/tests/CO-MIXED-20cc+H.litmus, catalogue/aarch64-mixed/tests/CoRR+amo.swph0h0-posh0a.w0+w0.litmus, catalogue/aarch64-mixed/tests/CoRR+rmwh0h0-posh0a.w0+w0.litmus, catalogue/aarch64-mixed/tests/CoRW2+posb0b0+b0.litmus, catalogue/aarch64-mixed/tests/CoRW2+posb1b0+h0.litmus, catalogue/aarch64-mixed/tests/CoRW2+posh0h0+h0.litmus, catalogue/aarch64-mixed/tests/LB+dmb.sy+data-wsi-wsi+MIXED+H.litmus, catalogue/aarch64-mixed/tests/MP+HAmo+BAcqAmo.litmus, catalogue/aarch64-mixed/tests/MP+dmb.syb0b1+datab1b1-rfib1h0.litmus, catalogue/aarch64-mixed/tests/MP-Koeln.litmus, catalogue/aarch64-mixed/tests/SmallEndian.litmus herd7 catalogue aarch64-mixed tests: OK ``` Thanks to ShaleXIONG for providing an expensive test to run for the timeouts. Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@arm.com>
Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@arm.com>
Let the system locate the location for the executables, it can still be overriden in special cases. Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@arm.com>
Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@arm.com>
psafont
force-pushed
the
dev/pau/test-timeouts-fail
branch
from
September 4, 2026 15:26
2e1771c to
166422d
Compare
psafont
marked this pull request as ready for review
September 4, 2026 15:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before this series, timeouts in catalog tests in
make testwere reported as missing from the test base, for example:Now they are reported as timeouts:
More work is needed to thread the error codes through mapply to be able to report only the tests that timed out instead of the whole directory.
We might want to change it to be able to somehow report the timeouts, although last time I tried github didn't support warnings, and instead simply returned errors.
Depends on #1979