Retry a suite that recorded no test case, not one that left no directory - #35
Merged
Merged
Conversation
The retry added in #33 never fired on the failure it was written for. It asked whether the results directory was absent; a run whose APK install never happened still leaves that directory behind, empty. So the run on the merge commit reported `Starting 0 tests`, took the non-fatal path, and the job went green with the public ECH suite having said nothing at all — which is what the retry existed to prevent. Ask about test cases instead. Checked against all four states: no directory, empty directory, XML with zero cases (the observed failure), and real results — only the last declines to retry. Also drop `wait_for_package_service`. #31 landed a real readiness gate a few commits later, and it is strictly stronger than this one on every axis: it waits on the settings provider and external storage as well as the package service, it proves the path with a live call, and it fails the run instead of shrugging when the device never arrives. Two waits where the weaker one runs second is just a claim that the first didn't work. This does not yet say whether ECH works on API 37. No run has executed those six cases: the first two never installed, and the third failed on the public suffix asset. It says that a run which doesn't execute them will stop looking like one that did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CqiK79k9uoWsn2AzgXpHMA
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
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.
Follow-up to #33, whose retry never fired on the failure it was written for.
The defect
The retry asked whether the results directory was absent. A run whose APK install never happened still leaves that directory behind — empty. So on the merge commit of #33 the Android public suite did this:
Zero tests, no retry attempted, non-fatal path taken, job green. Exactly the outcome the retry existed to prevent, and indistinguishable on the status page from a suite that had nothing to say.
The fix
Ask about test cases rather than about the directory. Checked against all four states:
Only the last declines, which is the intent.
Also: one wait, not two
wait_for_package_serviceis removed. #31 landed a real readiness gate a few commits after #33, and it is strictly stronger on every axis — it waits on the settings provider and external storage as well as the package service, proves the path with a live call, and fails the run instead of shrugging when the device never arrives. Two waits where the weaker one runs second is just a claim that the first didn't work. #31's gate is what should prevent zero-test runs; the retry is the backstop for when it doesn't.What this does not do
It does not say whether ECH works on API 37. No run has yet executed those six public cases — the first two never installed, and the third failed on the
PublicSuffixDatabase.listasset that #33 fixed. This change only ensures that a run which doesn't execute them stops looking like one that did.Generated by Claude Code