fix: paginate list endpoints so history and comments are found - #319
Open
AlveElde wants to merge 4 commits into
Open
fix: paginate list endpoints so history and comments are found#319AlveElde wants to merge 4 commits into
AlveElde wants to merge 4 commits into
Conversation
fetchArtifacts listed a run's artifacts with a single request, so it only ever saw the 30 newest. In a repo with many artifacts per run the CTRF report often falls outside that window, the run then looks like it has no report, and every history-backed report falls back to showing a single run. Page through the full list with per_page 100, and pass the artifact name to the endpoint's name filter so the common case stays one request. An empty name is ignored, since name= would match nothing. The name check in processArtifactsFromRun stays as a fallback for deployments that ignore the filter.
GitHub keeps listing artifacts after their retention has passed, and downloading one answers 410. Since history now reaches runs far enough back to be expired, that exception escaped processArtifactsFromRun and discarded every report already collected for the run, so metrics.ts dropped the whole run. Skip artifacts marked expired, and handle download and unzip failures per artifact so an unreadable one costs only itself.
listComments made a single request, so it returned only the oldest 30 comments. findExistingMarkedComment then failed to find the reporter's own comment on any pull request with more discussion than that, and handleComment posted a new comment on every run instead of updating the existing one. The isLatest check compared against the wrong comment for the same reason.
All attempts of a workflow run share a run id, so a re-run left two same-name artifacts on the run. Both were downloaded and pushed, which made metrics.ts count one run as two entries of previous-results-max and double-counted its results in the flaky and fail rate insights. The old 30 artifact window usually hid the extra one. Take the highest artifact id, which is the latest upload, and fall back to an earlier attempt only when the newer one cannot be read.
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.
Fix #314
per_page: 100, and pass the artifact name to the endpoint'snamefilter so the common case stays a single request. The name check inprocessArtifactsFromRunremains as a fallback for deployments that ignore the filter.findExistingMarkedCommentcould not find the reporter's own comment on a busy pull request, so the action posted a new comment on every run instead of updating, andisLatestcompared against the wrong comment.previous-results-maxand double-counting the run in the flaky and fail rate insights.Side note
sourcemap: trueintsup.config.tsis worth revisiting separately. The committeddist/index.js.mapis 16 MB, and across 103 rebuilt versions its blobs account for 31 MiB of the repository's 55 MiB pack, so roughly 300 KB of permanent history per dist-touching commit. Nothing consumes it: the action runsnode dist/index.jswith no--enable-source-mapsanywhere inaction.ymlor the workflows.