ci(e2e): raise behaviour job timeout to 45m - #5519
Conversation
URL-sourced harness behaviour scenarios push the suite past the 30m pull_request_target job ceiling. PRs cannot raise this themselves because pull_request_target uses the workflow from main. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Context for reviewersThis lands on main so Evidence from #5498: behaviour job cancelled at exactly ~30m15s ( Required checks are |
PR Summary by QodoCI: increase behaviour E2E timeouts to 45 minutes
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
|
🤖 Finished Review · ✅ Success · Started 9:07 AM UTC · Completed 9:16 AM UTC |
Site previewPreview: https://04b70578-site.fullsend-ai.workers.dev Commit: |
Code Review by Qodo
1. No timeout headroom
|
| (github.event_name != 'pull_request_target' || needs.gate.outputs.authorized == 'true') | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 30 | ||
| timeout-minutes: 45 |
There was a problem hiding this comment.
1. No timeout headroom 🐞 Bug ☼ Reliability
The behaviour job sets timeout-minutes: 45 while make behaviour-test runs `go test -timeout 45m`, so checkout/setup/auth time can consume part of the 45m budget and GitHub may cancel the job before Go’s timeout triggers. This can reduce diagnosability (no Go timeout dump) and may prevent failure-only artifact upload from completing if the job is cancelled near the limit.
Agent Prompt
### Issue description
The behaviour workflow job timeout equals the Go test timeout, but the job includes several non-test steps (changes detection, checkout, setup-go, GCP auth) and a failure-only artifact upload. If the test run is long or hangs, GitHub’s job-level timeout can fire before `go test -timeout` does, which reduces useful timeout diagnostics and can interrupt/skip artifact uploading.
### Issue Context
This PR raises both the job timeout and `go test -timeout` to 45m, but keeping them equal still leaves zero headroom.
### Fix Focus Areas
- .github/workflows/e2e.yml[178-277]
- Makefile[155-160]
### Suggested fix
- Increase the **job** timeout above the **test** timeout (e.g., job `timeout-minutes: 55` while keeping `go test -timeout 45m`).
- Optionally also set a **step-level** timeout for `Run behaviour tests` (e.g., `timeout-minutes: 45`) so the test command is bounded, while the extra job headroom covers setup + artifact upload.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ReviewFindingsHigh
Labels: PR modifies e2e CI workflow and Makefile test targets |
| (github.event_name != 'pull_request_target' || needs.gate.outputs.authorized == 'true') | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 30 | ||
| timeout-minutes: 45 |
There was a problem hiding this comment.
[high] protected-path
This PR modifies .github/workflows/e2e.yml, which is under the .github/ protected path. The PR has no linked issue providing formal authorization for modifying governance/infrastructure files. While the PR description clearly explains the rationale (behaviour tests on #5498 cancelled at ~30m15s with healthy suites at ~24-29m, headroom needed after dispatch-window increases in #5506), human approval is always required for protected-path changes.
Suggested fix: Link a tracking issue authorizing the CI timeout change, or obtain explicit human reviewer approval for modifying .github/ files.
|
Superseded by #5498, which merged the same behaviour |
|
🤖 Finished Retro · ✅ Success · Started 10:18 AM UTC · Completed 10:28 AM UTC |
Retro: PR #5519 — ci(e2e): raise behaviour job timeout to 45mWorkflow: Human-authored, 2-line CI config change (timeout 30→45m in Review quality: The review agent's only finding was governance-related (no linked issue for a protected-path change), not a code quality issue. The Qodo bot caught a real functional concern the review agent missed: the Go test timeout (45m) equals the job timeout (45m), leaving zero headroom for checkout/setup/auth/artifact-upload steps. This is a legitimate CI correctness gap. No new proposals filed — all improvement opportunities are covered by existing open issues:
Agents repo: |
Summary
behaviourjobtimeout-minutesfrom 30 → 45.make behaviour-testGo-timeoutto 45m.Why
pull_request_targetworkflows load YAML from base/main, so a timeout bump inside PR #5498 cannot take effect until it lands on main. Behaviour on #5498 was cancelled at exactly ~30m15s (timeout-minutes: 30on main) while still running triage scenarios. Healthy suites already take ~24–29m; headroom is needed after dispatch-window increases (#5506).Related
Test plan
behaviourjob showstimeout-minutes: 45on main after mergeMade with Cursor