Skip to content

ci(e2e): raise behaviour job timeout to 45m - #5432

Closed
ifireball wants to merge 1 commit into
mainfrom
ci/behaviour-timeout-45m
Closed

ci(e2e): raise behaviour job timeout to 45m#5432
ifireball wants to merge 1 commit into
mainfrom
ci/behaviour-timeout-45m

Conversation

@ifireball

Copy link
Copy Markdown
Member

Summary

Test plan

Made with Cursor

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>
@ifireball
ifireball requested a review from a team as a code owner July 22, 2026 05:56
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Raise behaviour CI job timeout to 45 minutes

⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Increase GitHub Actions behaviour job timeout from 30m to 45m.
• Align make behaviour-test Go test timeout to 45m to match CI.
• Prevent pull_request_target runs from being killed due to longer behaviour suites.
Diagram

graph TD
  A(["GitHub Actions runner"]) --> B["e2e workflow"] --> C["behaviour job"] --> D["timeout-minutes: 45"] --> E(["make behaviour-test"]) --> F(["go test -tags behaviour -timeout 45m"])
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Shard behaviour suite into parallel jobs
  • ➕ Reduces wall-clock time and avoids simply extending CI ceilings
  • ➕ Improves feedback latency when suites grow further
  • ➖ Requires test partitioning strategy and potentially more CI resources
  • ➖ May expose test ordering/isolations issues
2. Split long-running URL-dispatch scenarios into a separate workflow/job
  • ➕ Keeps default behaviour job fast while still exercising extended scenarios
  • ➕ Lets you tune timeout/resource settings independently
  • ➖ More workflows/jobs to maintain
  • ➖ May reduce coverage on the default CI path if not enforced consistently
3. Optimize test runtime (targeted fixes) instead of increasing timeouts
  • ➕ Addresses root cause and lowers CI cost
  • ➕ Avoids masking performance regressions
  • ➖ Often slower to execute than a timeout adjustment
  • ➖ May not be feasible if new scenarios are inherently long-running

Recommendation: The timeout increase is the right immediate fix because pull_request_target uses the workflow from main, so PRs cannot self-raise the ceiling. Consider sharding or splitting suites if behaviour runtime continues to grow, to avoid steadily increasing CI timeouts and reducing signal latency.

Files changed (2) +2 / -2

Other (2) +2 / -2
e2e.ymlIncrease behaviour job timeout to 45 minutes +1/-1

Increase behaviour job timeout to 45 minutes

• Raises the GitHub Actions job-level 'timeout-minutes' from 30 to 45 for the behaviour suite job, preventing 'pull_request_target' runs from being killed at 30 minutes.

.github/workflows/e2e.yml

MakefileAlign behaviour-test Go test timeout to 45 minutes +1/-1

Align behaviour-test Go test timeout to 45 minutes

• Updates the 'behaviour-test' make target to run 'go test' with '-timeout 45m', matching the increased CI job timeout ceiling.

Makefile

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:58 AM UTC · Completed 6:08 AM UTC
Commit: 46db852 · View workflow run →

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://b35e21c2-site.fullsend-ai.workers.dev

Commit: 46db8527895a1d0a9b8f421ada835a65461be627

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

High

  • [protected-path] .github/workflows/e2e.yml:185 — This PR modifies a file under the protected .github/ path without a linked issue providing formal authorization. The change itself is a straightforward timeout bump (30m → 45m) and the PR body explains the operational need (accommodating longer behaviour test suites per test(#3928): add behaviour scenarios for URL-sourced harness dispatch #5407), but protected-path changes require a linked issue for traceability. Human approval is always required for changes to governance and infrastructure files.
    Remediation: Link an issue authorizing this change, or have a maintainer approve directly.

Labels: PR modifies CI workflow and e2e test configuration

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread .github/workflows/e2e.yml
(github.event_name != 'pull_request_target' || needs.gate.outputs.authorized == 'true')
runs-on: ubuntu-24.04
timeout-minutes: 30
timeout-minutes: 45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] protected-path

This PR modifies a file under the protected .github/ path without a linked issue providing formal authorization. The change itself is a straightforward timeout bump (30m → 45m) and the PR body explains the operational need (accommodating longer behaviour test suites per #5407), but protected-path changes require a linked issue for traceability. Human approval is always required for changes to governance and infrastructure files.

Suggested fix: Link an issue authorizing this change, or have a maintainer approve directly.

@fullsend-ai-review fullsend-ai-review Bot added component/ci CI pipelines and checks component/e2e End-to-end tests labels Jul 22, 2026
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to unblock but we need to reduce the time spent, I'm getting Konflux vibes here about waiting for 2h for a PR.

@ifireball

ifireball commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Closing, I do not really want to breach 30m. Implementing scenario paralelization instead

@ifireball ifireball closed this Jul 22, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 11:08 AM UTC · Completed 11:18 AM UTC
Commit: 46db852 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5432 — ci(e2e): raise behaviour job timeout to 45m

Outcome: Human-authored, 2-line CI config PR was closed voluntarily by the author (ifireball) who chose scenario parallelization over timeout increases. The workflow completed normally.

Timeline

  1. 05:56 UTC — ifireball opens PR fullsend-ai/fullsend#5432: timeout bump 30m→45m in .github/workflows/e2e.yml and Makefile.
  2. 06:08 UTC — Review agent (run 29895236546) issues CHANGES_REQUESTED with one HIGH-severity [protected-path] finding: .github/workflows/e2e.yml modified without a linked issue.
  3. 09:57 UTC — Human reviewer (rh-hemartin) APPROVED with LGTM, noting CI timing concerns but not mentioning the protected-path finding.
  4. 11:05 UTC — Author closes PR voluntarily, opting for scenario parallelization instead.

Assessment

Review quality: The review agent's sole finding — a HIGH-severity protected-path flag on a 2-line timeout bump — was procedurally valid per current rules but disproportionate for this type of mechanical change. The human reviewer approved without acknowledging the protected-path concern, confirming the finding added friction without value. Qodo bot correctly found zero code issues.

Token cost: 4 sub-agents ran for ~11 minutes. Three of four (correctness, style-conventions, security) had nothing actionable on a numeric config change. The review was correctly scoped as "trivial" but still dispatched the full sub-agent suite.

Autonomy readiness: The human override (approving despite CHANGES_REQUESTED) demonstrates that for trivial human-authored CI config changes, the review agent is more conservative than human reviewers. This supports increased autonomy or severity reduction for this class of change.

Evidence for existing issues (no new proposals)

All improvement areas identified are already tracked by open issues with substantial prior evidence:

  • #1551 (downgrade protected-path severity for human-authored PRs) — PR ci(e2e): raise behaviour job timeout to 45m #5432 is a near-exact repeat of the pattern described in this issue: human-authored CI config, HIGH protected-path, CHANGES_REQUESTED ignored by human reviewer.
  • #2200 (cap missing-authorization severity at Low for human-authored PRs with descriptive bodies) — PR ci(e2e): raise behaviour job timeout to 45m #5432 had a clear description explaining the rationale and referencing test(#3928): add behaviour scenarios for URL-sourced harness dispatch #5407, yet missing-authorization was rated HIGH.
  • #5370 (distinguish mechanical version bumps from governance changes in .github/) — a timeout value change is among the most mechanical changes possible to a workflow file.
  • #2614 (refine .github/ protected path to target specific subdirectories) — e2e.yml timeout config is far from the governance-sensitive files the protected-path policy is designed to guard.
  • #5157 (scale review detail to change complexity) — dispatching 4 sub-agents for a 2-line numeric change, where 3 had nothing to review, is proportionate in time but wasteful in coverage.

No new proposals filed — existing issues comprehensively cover the improvement opportunities surfaced by this retro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks component/e2e End-to-end tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants