Skip to content

Commit 9f6256c

Browse files
committed
ci(nix): require every sources attempt to report a display
With enumeration split by type, the sources smoke test no longer has a known flake. Exit 0 with displays: [] now counts as a failure, and the verdict requires all attempts. Refs #462.
1 parent 968f82a commit 9f6256c

1 file changed

Lines changed: 20 additions & 17 deletions

File tree

‎.github/workflows/nix-build.yml‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,14 @@ jobs:
426426
# xvfb-run folds the command's stderr into stdout, so the stderr
427427
# capture is empty and every milestone lands in the other file.
428428
grep -a milestone "/tmp/sources.$i.out" || echo " (none at all)"
429+
elif [ "$RC" -eq 0 ] && ! python3 -c "import json,sys; sys.exit(0 if json.load(open(sys.argv[1]))['displays'] else 1)" "/tmp/sources.$i.json"; then
430+
# Exit 0 is not an answer on its own. Electron 43 settles the
431+
# combined screen+window call after its 3s ready timer with the
432+
# screen dropped, so `displays: []` is #462's other face, and
433+
# Xvfb always has one screen to report.
434+
FAILED=$((FAILED + 1))
435+
echo "returned no display (#462). Last milestone:"
436+
grep -a milestone "/tmp/sources.$i.out" | tail -1 || echo " (none)"
429437
elif [ "$RC" -eq 0 ]; then
430438
OK=$((OK + 1))
431439
LAST_OK="/tmp/sources.$i.json"
@@ -460,10 +468,8 @@ jobs:
460468
PY
461469
fi
462470
463-
# Noted, not acted on. Enumeration is flaky here and the export
464-
# assertion below is the more
465-
# important question; a flaky attempt costs an annotation rather than
466-
# the answer. Verdict at the end, once both have had their say.
471+
# Annotated here, decided at the end: export runs either way, so a
472+
# failed enumeration cannot hide whether the addon works.
467473
if [ "$HUNG" -gt 0 ]; then
468474
echo "::warning::openscreen sources had to be killed on $HUNG of $ATTEMPTS attempts. Check the milestones above for how far it got before the outer bound fired."
469475
fi
@@ -590,21 +596,18 @@ jobs:
590596
# flaky must not hide whether export works, which is the whole point of
591597
# having packaged the compositor addon.
592598
#
593-
# The gate is "did enumeration ever answer" and "does export work", not
594-
# "did all five attempts pass". Requiring FAILED -eq 0 made the job red
595-
# by construction: the standing numbers on this runner are 1/5, 3/5 and
596-
# 4/5 ok, so a run where export is perfect and four enumerations succeed
597-
# still failed. A post-merge check that is red by design is one that gets
598-
# muted, which costs more than the flakiness it was reporting. The
599-
# per-attempt warnings above keep that flakiness visible without letting
600-
# it decide the build; tighten this to $ATTEMPTS once the capture failure
601-
# is understood and fixed.
599+
# Every enumeration must answer with a display, and export must work.
600+
# The gate used to be "did enumeration ever answer", because the call
601+
# failed on 1 to 4 attempts in 5 for a reason nobody had found. #462
602+
# found it: asking for screens and windows in one getSources call is
603+
# what fails under X11, and the get-sources handler now asks one type
604+
# at a time (under Xvfb: 0 failures in 32 runs, against 7 in 16
605+
# combined). A flake left after that is a regression, not runner luck.
602606
#
603-
# Export, on the other hand, is a hard gate on every trigger again. It no
604-
# longer depends on a capability this host does not have, so there is
605-
# nothing left to excuse: if it fails now, the package is broken.
607+
# Export is a hard gate on every trigger. It does not depend on a
608+
# capability this host lacks, so if it fails, the package is broken.
606609
echo "=== verdict: enumeration $OK/$ATTEMPTS ok, export $EXPORT_OK, $RUN_CLI_N run_cli invocations ==="
607-
if [ "$EXPORT_OK" -ne 1 ] || [ "$OK" -eq 0 ]; then
610+
if [ "$EXPORT_OK" -ne 1 ] || [ "$OK" -ne "$ATTEMPTS" ]; then
608611
exit 1
609612
fi
610613

0 commit comments

Comments
 (0)