This is an automated email from the git hooks/post-receive script.
git pushed a commit to branch wl/browser-all
in repository enlightenment.
View the commit online.
commit ef132c3e2b00c35be95219bed4f4f186590859e7
Author: Cedric BAIL <[email protected]>
AuthorDate: Mon Aug 17 19:06:44 2026 -0600
tests - stop the tidy-up from failing a test that passed
run-nested.sh reported a failure on runs where the client had already printed
"ok" and exited zero. Measured on brave: 3 runs in 5. Not the compositor, not
the browser, and not the slow machine it looked like.
cleanup() saves the client's status on entry and exits with it at the end.
Between those two, under set -e, are two commands that can fail for reasons
that are nobody's fault:
* rm -rf "$RUNDIR", which races E's helpers still writing into HOME as they
wind down. The retry loop around it exists for exactly that race, and the
comment above it says the tidy-up must never decide the exit status - but
a bare rm is a top-level command, so losing the race killed the shell
instead of going round the loop.
* kill in procs_in_rundir, which races the process it just found in /proc.
Both now end in '|| :'. The X_PID kill was already safe, because its test
short-circuits before the kill can be the last command of the list, but it is
the same shape and is now spelt out rather than left to that.
The failure rate tracks how many processes are still winding down, which is
why only the browser tier appeared to have it - a trivial client never lost
the race in 5 runs. It is also why every attempt to trace it made it vanish:
sh -x and a pair of echoes both perturb the timing enough to hide it. What
found it was bisecting cleanup() with markers until one run reached the last
marker and never reached the exit.
The leftover /tmp/e-wl-test.* directories - 45 of them here - are the same
bug seen from the other side: the rm that failed never ran again either.
After: 6 runs in 6 agree with the client's own verdict.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01FtoiXoSKUmZb6Aix6U3GZS
---
src/tests/wayland/run-nested.sh | 36 +++++++++++++++++++++++++++++++++---
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/src/tests/wayland/run-nested.sh b/src/tests/wayland/run-nested.sh
index 0011341c1..2e55bd632 100755
--- a/src/tests/wayland/run-nested.sh
+++ b/src/tests/wayland/run-nested.sh
@@ -193,7 +193,21 @@ procs_in_rundir() {
grep -qxZ "HOME=$RUNDIR" "$env" 2>/dev/null || continue
pid=${env#/proc/}
pid=${pid%/environ}
- [ -n "${1:-}" ] && kill "-$1" "$pid" 2>/dev/null
+ # '|| :' is load-bearing, and it cost a lot of runs to find out.
+ #
+ # This kill races the process it just found: between the grep above and
+ # here, a winding-down helper can exit on its own. kill then fails,
+ # and because it is the *last* command of an && list, set -e applies to
+ # it - so the shell tears down mid-trap and run-nested.sh exits 1 with
+ # the cleanup half done. The client had already passed; its exit status
+ # was saved at the top of cleanup() and never reached the exit.
+ #
+ # From outside that is a test that printed "ok" and was reported as a
+ # failure, more often the more processes there are to reap, which is
+ # why it looked like slow-machine flakiness and why only the browser
+ # tier seemed to have it. The leftover /tmp/e-wl-test.* directories are
+ # the same bug: the rm never ran either.
+ [ -n "${1:-}" ] && { kill "-$1" "$pid" 2>/dev/null || :; }
n=$((n + 1))
done
echo "$n"
@@ -234,7 +248,10 @@ cleanup() {
sleep 0.1
done
[ "$(procs_in_rundir KILL)" = "0" ] || sleep 0.5
- [ -n "$X_PID" ] && kill "$X_PID" 2>/dev/null
+ # Safe only because the test short-circuits when there is no Xvfb, which
+ # keeps the kill from being the last command of the list. Spelt out rather
+ # than left to that: it is the same shape as the bug above.
+ [ -n "$X_PID" ] && { kill "$X_PID" 2>/dev/null || :; }
if [ "${E_TEST_KEEP:-0}" = "1" ]; then
echo "run-nested.sh: kept $RUNDIR (compositor log: $E_LOG)" >&2
else
@@ -243,9 +260,22 @@ cleanup() {
# them and fail on a directory being repopulated while it is emptied.
# Retry, and never let the tidy-up decide the exit status: that turns
# a passing test into a failing one for no reason.
+ #
+ # The '|| :' is what makes the sentence above true, and without it this
+ # loop did the exact opposite of what it says. rm returning non-zero is
+ # a top-level command under set -e, so losing the race did not retry -
+ # it killed the shell three statements before the exit that would have
+ # reported the client's status, which was zero. The test had already
+ # printed "ok" and was reported as a failure anyway.
+ #
+ # Measured at 3 runs in 5 with a browser and 0 in 5 with a trivial
+ # client: the more helpers there are still winding down, the likelier
+ # rm is to trip. That is why it read as slow-machine flakiness, why
+ # only the browser tier seemed to suffer it, and why adding tracing to
+ # find it made it go away.
i=0
while [ $i -lt 20 ]; do
- rm -rf "$RUNDIR" 2>/dev/null
+ rm -rf "$RUNDIR" 2>/dev/null || :
[ -d "$RUNDIR" ] || break
i=$((i + 1))
sleep 0.1
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
