Skip to content

ci: comment on PRs with tests that got slower instead of failing them - #4048

Merged
rom1504 merged 1 commit into
masterfrom
ci/duration-comment
Sep 4, 2026
Merged

ci: comment on PRs with tests that got slower instead of failing them#4048
rom1504 merged 1 commit into
masterfrom
ci/duration-comment

Conversation

@u9g

@u9g u9g commented Sep 4, 2026

Copy link
Copy Markdown
Member

Follow-up to #4028. Test durations are too noisy for a hard gate: fishing and nether vary by 2-3x between identical runs (see #4045's CI, and master's own run at faa2e33 failed the same check). Keep the comparison, but post the tests that got more than 1.5x slower as a PR comment instead of failing the job.

How it works:

  • compareDurations.js writes the slower tests to a file and never exits non-zero.
  • Each matrix job uploads that file plus the PR number as a slower-<job-index> artifact.
  • New duration-comment.yml runs on workflow_run of CI, downloads the artifacts and creates or updates a single marked comment on the PR. When a later push has no slower tests, the same comment is updated to say so.

Fork PRs get a read-only token, so the comment cannot be posted from the CI run itself; workflow_run.pull_requests is also empty for forks, which is why the PR number travels in the artifact.

Since the new workflow triggers on workflow_run, it only starts running once this is merged to master. The script was smoke-tested locally with fake duration files.

Test durations are too noisy for a hard gate: fishing and nether vary by
2-3x between identical runs, and master itself is failing the 1.5x check.
Keep the comparison, but post the slower tests as a PR comment instead.

Fork PRs run CI with a read-only token, so the matrix jobs upload the
slower tests as an artifact and a workflow_run job posts (or updates) the
comment. The PR number travels in the artifact because
workflow_run.pull_requests is empty for forks.
@rom1504
rom1504 merged commit c07e457 into master Sep 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants