diff --git a/.github/workflows/nix-build.yml b/.github/workflows/nix-build.yml index 3bb79037e..94c5b2bdd 100644 --- a/.github/workflows/nix-build.yml +++ b/.github/workflows/nix-build.yml @@ -216,98 +216,6 @@ jobs: echo "--- openscreen --help ---" run_cli $SANDBOX $CHROME_FLAGS --help - # The real acceptance test, and now the first one to run. Nothing above - # touches the compositor addon, which is what actually renders output: - # record a couple of seconds, export it, and look at what came out. - # - # ORDERING EXPERIMENT -- this block used to sit after the sources loop, - # and moving it is the whole point of the change that put it here. - # Across four runs, record succeeded 1 time in 10 while sources managed - # 13 in 20, and when record does work it returns in 17ms, exactly like - # sources: same mechanism, different frequency. The one difference the - # two had was position. run_cli spawns a fresh `xvfb-run -a` per - # invocation, so record was always invocations 6-8 of the step, after - # five Xvfb servers had been started and torn down, while sources ran at - # 4-8. If display churn is what drives the failures, record should now - # improve. If it stays at roughly 1 in 10 from this position, position - # was never the cause and something in the record path itself is, which - # is a different search. - # - # The other half of that prediction -- sources getting worse -- is NOT - # readable from its aggregate rate, and an earlier draft of this comment - # claimed it was. The workload ahead of sources is not fixed: the sandbox - # probe costs one invocation or two, and the loop below breaks on first - # success, so it costs between two and six. Sources therefore starts at a - # different position on different runs, and a rate averaged over runs - # confounds the two. Hence the run_cli counter: every attempt now prints - # the invocation it is, so the rates can be stratified by position across - # several runs. One run does not settle this; the counter is what makes - # several of them add up. - # - # Read the next runs as that A/B, not as a verdict on the package. Put it - # back if the rates do not move. - # - # Up to three goes, because screen capture on this host is unreliable in - # its own right. One success is enough for the question being asked here. - echo "--- record then export (first run_cli here is #$((RUN_CLI_N + 1))) ---" - EXPORTED="" - # Tracked apart from EXPORTED so the verdict can name the stage that - # actually failed. Every run so far has died in record without export - # ever executing, while the annotation said "the export path does not - # work" -- an accusation aimed at the one component the run never - # reached, and the compositor addon is precisely what this step exists - # to vouch for. - RECORDED=0 - for i in 1 2 3; do - echo "=== export attempt $i/3 (run_cli #$((RUN_CLI_N + 1))) ===" - rm -f /tmp/demo.openscreen /tmp/demo.mp4 - RC=0 - CLI_TIMEOUT=120 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS record --duration 2 --project /tmp/demo.openscreen >"/tmp/rec.$i.out" 2>&1 || RC=$? - # Outside the failure branch for the same reason as above: a record that - # works is exactly the measurement missing from the comparison, since - # this path has never yet produced one. - grep -a "get-sources\]" "/tmp/rec.$i.out" || true - if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.openscreen ]; then - echo "record failed (rc=$RC); last lines:" - tail -5 "/tmp/rec.$i.out" || true - continue - fi - RECORDED=1 - echo "recorded. project:" - head -c 200 /tmp/demo.openscreen; echo - - RC=0 - CLI_TIMEOUT=180 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS export /tmp/demo.openscreen -o /tmp/demo.mp4 >"/tmp/exp.$i.out" 2>&1 || RC=$? - if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.mp4 ]; then - echo "export failed (rc=$RC); last lines:" - tail -15 "/tmp/exp.$i.out" || true - continue - fi - EXPORTED=/tmp/demo.mp4 - break - done - - EXPORT_OK=0 - if [ -z "$EXPORTED" ] && [ "$RECORDED" -eq 0 ]; then - echo "::error::No attempt got past record, so export never ran and the compositor addon is unproven. This is a capture failure on this host, not an export failure." - elif [ -z "$EXPORTED" ]; then - echo "::error::record produced a project but no attempt produced an MP4. The compositor addon is packaged and the export path does not work." - else - SIZE=$(wc -c < "$EXPORTED") - # An MP4 opens with a 4-byte length then 'ftyp'. A zero-length or - # truncated file would otherwise pass a mere existence check. - MAGIC=$(dd if="$EXPORTED" bs=1 skip=4 count=4 2>/dev/null || true) - echo "exported $SIZE bytes, magic at offset 4: $MAGIC" - if [ "$MAGIC" != "ftyp" ]; then - echo "::error::output is not an MP4 (no ftyp box)" - elif [ "$SIZE" -lt 10000 ]; then - echo "::error::MP4 is only $SIZE bytes, too small to hold two seconds of video" - else - echo "Export works: $SIZE bytes of MP4." - EXPORT_OK=1 - fi - fi - # `sources` enumerates displays, windows and microphones, so it # exercises the capture stack rather than proving that a usage string # can be printed. @@ -399,7 +307,7 @@ jobs: fi # Noted, not acted on. Enumeration is flaky here and the export - # assertion -- which now runs above rather than below -- is the more + # assertion below is the more # important question; a flaky attempt costs an annotation rather than # the answer. Verdict at the end, once both have had their say. if [ "$HUNG" -gt 0 ]; then @@ -409,6 +317,90 @@ jobs: echo "::warning::openscreen sources failed on $FAILED of $ATTEMPTS attempts. The capture path is not reliable on this host." fi + # The real acceptance test. Everything above proves the package starts + # and can list a screen; none of it touches the compositor addon, which + # is what actually renders output. Record a couple of seconds, export it, + # and look at what came out. + # + # This block was briefly moved ahead of the sources loop and moved back, + # so that it is not tried a third time. The theory was that position + # explained why record seemed to fail far more often than sources -- + # run_cli spawns a fresh `xvfb-run -a` each time, so record was always + # invocations 6-8, after five Xvfb servers had come and gone. The + # experiment could not answer it: by the time it ran, record had started + # succeeding from its old position anyway, so there was no contrast left + # to measure. From position 4 it succeeded, which proves nothing it was + # not already doing from position 9. + # + # What the runs did establish is that the premise was wrong. Enumeration + # here is bimodal -- 12-31ms when it answers, no return at all when it + # does not, with nothing in between across every measurement so far -- + # and the failures cluster by run and by window within a run rather than + # by command. The apparent record-versus-sources gap was that clustering + # seen through a denominator, not a property of either path. Reopen this + # with the run_cli labels, on a run that actually fails, before assuming + # otherwise. + # + # Up to three goes, because screen capture on this host is unreliable in + # its own right. One success is enough for the question being asked here. + echo "--- record then export (first run_cli here is #$((RUN_CLI_N + 1))) ---" + EXPORTED="" + # Tracked apart from EXPORTED so the verdict can name the stage that + # actually failed. For three runs every attempt died in record without + # export ever executing, while the annotation said "the export path does + # not work" -- an accusation aimed at the one component the run never + # reached, and the compositor addon is precisely what this step exists + # to vouch for. + RECORDED=0 + for i in 1 2 3; do + echo "=== export attempt $i/3 (run_cli #$((RUN_CLI_N + 1))) ===" + rm -f /tmp/demo.openscreen /tmp/demo.mp4 + RC=0 + CLI_TIMEOUT=120 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS record --duration 2 --project /tmp/demo.openscreen >"/tmp/rec.$i.out" 2>&1 || RC=$? + # Outside the failure branch for the same reason as above: a record that + # works is exactly the measurement missing from the comparison, since + # this path has never yet produced one. + grep -a "get-sources\]" "/tmp/rec.$i.out" || true + if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.openscreen ]; then + echo "record failed (rc=$RC); last lines:" + tail -5 "/tmp/rec.$i.out" || true + continue + fi + RECORDED=1 + echo "recorded. project:" + head -c 200 /tmp/demo.openscreen; echo + + RC=0 + CLI_TIMEOUT=180 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS export /tmp/demo.openscreen -o /tmp/demo.mp4 >"/tmp/exp.$i.out" 2>&1 || RC=$? + if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.mp4 ]; then + echo "export failed (rc=$RC); last lines:" + tail -15 "/tmp/exp.$i.out" || true + continue + fi + EXPORTED=/tmp/demo.mp4 + break + done + + EXPORT_OK=0 + if [ -z "$EXPORTED" ] && [ "$RECORDED" -eq 0 ]; then + echo "::error::No attempt got past record, so export never ran and the compositor addon is unproven. This is a capture failure on this host, not an export failure." + elif [ -z "$EXPORTED" ]; then + echo "::error::record produced a project but no attempt produced an MP4. The compositor addon is packaged and the export path does not work." + else + SIZE=$(wc -c < "$EXPORTED") + # An MP4 opens with a 4-byte length then 'ftyp'. A zero-length or + # truncated file would otherwise pass a mere existence check. + MAGIC=$(dd if="$EXPORTED" bs=1 skip=4 count=4 2>/dev/null || true) + echo "exported $SIZE bytes, magic at offset 4: $MAGIC" + if [ "$MAGIC" != "ftyp" ]; then + echo "::error::output is not an MP4 (no ftyp box)" + elif [ "$SIZE" -lt 10000 ]; then + echo "::error::MP4 is only $SIZE bytes, too small to hold two seconds of video" + else + echo "Export works: $SIZE bytes of MP4." + EXPORT_OK=1 + fi + fi # One verdict, after both questions have been asked. Enumeration being # flaky must not hide whether export works, which is the whole point of