Prevent one case's comparison bug from crashing the whole matrix run - #114
Merged
Conversation
The macOS publish workflow has been failing to produce a report at all
for the last two runs: a single case's summary comparison raised an
unhandled KeyError ('audio') deep in _compare_audio_summaries(), which
propagated straight out of asyncio.gather() in run_matrix() and killed
run_all.py before the report/Pages-publish steps ever ran. This is why
recently-added scenarios (e.g. server-initiated-legacy-unencrypted from
#112) appeared to be "missing" from the published site -- the whole
report generation had silently stopped succeeding, not just that one
scenario's row.
- runner.py: wrap _compare_summaries() in run_case() so any exception
it raises is turned into a failed case with a descriptive reason
instead of crashing the entire matrix run. A bug in one scenario's
comparison logic should now only fail that one case.
- _compare_audio_summaries(): guard against a missing/malformed
"audio" section in either summary with an explicit, readable failure
reason instead of a raw KeyError, since that's the concrete bug that
triggered this.
Verified locally: python -m compileall src scripts passes, and a full
aiosendspin -> aiosendspin matrix run (including the new
server-initiated-legacy-unencrypted and
server-initiated-protocol-baseline-v1 scenarios) still produces the
same pass/fail results as before, plus report generation still
succeeds end-to-end.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
The actual bug behind "legacy-unencrypted not showing up on the site"
The last two publish workflow runs (main push after #112, and the manual workflow_dispatch after #113) both failed to produce a report at all:
This exception propagates straight out of
asyncio.gather()inrun_matrix()with no handling, sorun_all.pyexits non-zero before the report/Pages-publish steps ever run. The workflow'sCheck site was builtstep then just skips publishing (index.html not found) rather than failing loudly, which made it look like only the newserver-initiated-legacy-unencryptedscenario was missing -- really, the whole site has been stuck on a stale snapshot from before #109/#112 merged, since neither of the two most recent runs produced any report at all.Fix
run_case()in a try/except so a bug in any single scenario's comparison logic fails only that one case with a descriptive reason, instead of crashing the entire matrix._compare_audio_summaries()to fail with a clear message when either summary is missing itsaudiosection, rather than raisingKeyError.Verification
python -m compileall src scriptspasses.aiosendspin -> aiosendspinfull matrix run produces the same pass/fail results as before (including the new legacy-unencrypted and protocol-baseline scenarios), andconformance.cli reportsucceeds end-to-end.Follow-up
Once merged, we should re-trigger (or wait for the next scheduled) publish run to confirm the live site picks back up and shows the scenarios that have been invisible since #109/#112.