Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (6)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (7)Review documentation changes against `docs/documentation.md` and the linked repository's `standards/documentation.md`.⚙️ CodeRabbit configuration file Files:
Review changes in `make/` against `docs/contributing.md` and the linked repository's `standards/makefiles.md`.⚙️ CodeRabbit configuration file Files:
For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved⚙️ CodeRabbit configuration file Files:
Topic-specific documentation must live in the matching file under [`docs/`](.).📄 CodeRabbit inference engine (docs/documentation.md) Files:
When applicable, read the repository documentation in this order: `docs/e2e_tests.md` — end-to-end test setup and execution Put topic-specific documentation under `docs/` instead of expanding `README.md`.📄 CodeRabbit inference engine (AGENTS.md) Files:
When applicable, read the repository documentation in this order: `docs/testing.md` — repository-specific testing strategy and command mapping Put topic-specific documentation under `docs/` instead of expanding `README.md`.📄 CodeRabbit inference engine (AGENTS.md) Files:
When repository behavior changes, update the narrowest relevant document under `docs/`📄 CodeRabbit inference engine (docs/contributing.md) Files:
🔇 Additional comments (6)
📝 WalkthroughWalkthroughThe PR updates the E2E Make target and documentation. It separates test paths from pytest options, disables E2E coverage output, and increases three workflow job timeouts from 90 to 180 minutes. ChangesE2E execution
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The E2E execution and timeout changes introduce no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
jentyk
force-pushed
the
bugfix/MPT-25021/e2e-suite-on-main-never-completes-every-push-run-hits-90-minute-job-timeout
branch
from
September 16, 2026 13:23
d6eda08 to
e28a3a6
Compare
jentyk
changed the title
jentyk
marked this pull request as draft
September 16, 2026 14:13
jentyk
marked this pull request as ready for review
September 17, 2026 08:10
ruben-sebrango
approved these changes
Sep 17, 2026
`make e2e` dropped the `tests/e2e` path whenever `args` carried pytest options, so the CI step fell back to `testpaths = "tests"` and re-ran the 2310 unit tests, with coverage, after the e2e suite. The target now always passes the suite path (`path` overrides it) and runs with `--no-cov`, so the unit `coverage.xml` that SonarCloud reads is left intact. The e2e suite has exceeded the 90-minute job timeout on every `main` push and cron run since 2026-06-25 because of TEST-environment latency on the term-variants and assets endpoints, so no launch has finished since. Report Portal puts the full suite at 103 to 120 minutes today, with the first half swinging by 17 minutes between launches. Raise the timeout on the push, cron and release workflows to 180 minutes so the suite can complete again with headroom while the latency is escalated. Refs: MPT-25021 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jentyk
force-pushed
the
bugfix/MPT-25021/e2e-suite-on-main-never-completes-every-push-run-hits-90-minute-job-timeout
branch
from
September 17, 2026 08:37
e28a3a6 to
38eaa72
Compare
|
jentyk
deleted the
bugfix/MPT-25021/e2e-suite-on-main-never-completes-every-push-run-hits-90-minute-job-timeout
branch
September 17, 2026 08:43
jentyk
added a commit
that referenced
this pull request
Sep 17, 2026
Pushes to main and release/** run an E2E step that takes hours, but the workflow had no concurrency guard. Every merge started another run while the previous one was still going, so several E2E suites executed at once against the shared TEST environment. Put every E2E entry point in one concurrency group with cancel-in-progress, so only one suite is ever in flight. The group name is the same literal in push-release-branch.yml and cron-main-e2e.yml because the constraint is the TEST environment rather than the branch: a merge to main, a push to a release branch and the twice-weekly scheduled run all contend for the same resource. Cancelling a run that is already hours in is safe here. Over the last 61 active days the final push to main had at least 180 minutes of clear runway on 59 of them, usually overnight, so each working day still ends with one completed suite on that day's tip, and the runs that get cancelled are on commits that were superseded anyway. Release branches see three pushes in five months. Move the SonarCloud scan ahead of the E2E step. Until #427 raised the job timeout, every push run on main reached it while still inside E2E, so the scan that followed was reported as skipped and main was never scanned - only pull requests were. Running the scan first makes it independent of how long E2E takes. Refs MPT-25022. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jentyk
added a commit
that referenced
this pull request
Sep 17, 2026
Pushes to main and release/** run an E2E step that takes hours, but the workflow had no concurrency guard. Every merge started another run while the previous one was still going, so several E2E suites executed at once against the shared TEST environment. Split push-release-branch.yml into a validation job and an E2E job, and put the concurrency group on the E2E job alone. The group name is the same literal there and in cron-main-e2e.yml, because the contended resource is the TEST environment rather than the branch: a merge to main, a push to a release branch and the twice-weekly scheduled run all want the same deployment, so one E2E cancels another wherever it came from. Scoping the group to the E2E job is what keeps make check-all and the SonarCloud scan out of it. Held at workflow level, the shared group let a scheduled or dispatched E2E run cancel a push run mid-validation, and the cancelling workflow repeats neither - main would have lost the lint, unit-test and scan signal it is the only place to get. The validation job carries no group at all, so nothing cancels it; E2E still waits on it through needs, the same ordering the single job had. Cancelling a running suite is safe here. Over the last 61 active days the final push to main had at least 180 minutes of clear runway on 59 of them, usually overnight, so each working day still ends with one completed suite on that day's tip, and the runs that get cancelled are on commits that were superseded anyway. Release branches see three pushes in five months. The SonarCloud scan also moves ahead of E2E within the validation job. Until #427 raised the job timeout, every push run on main reached it while still inside E2E, so the scan that followed was reported as skipped and main was never scanned - only pull requests were. release.yml can still run the suite outside this group; isolating it needs its own job there so serialization never covers the irreversible publish steps, tracked in MPT-25395. Refs MPT-25022. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jentyk
added a commit
that referenced
this pull request
Sep 18, 2026
Pushes to main and release/** run an E2E step that takes hours, but the workflow had no concurrency guard. Every merge started another run while the previous one was still going, so several E2E suites executed at once against the shared TEST environment. Split push-release-branch.yml into a validation job and an E2E job, and put the concurrency group on the E2E job alone. The group name is the same literal there and in cron-main-e2e.yml, because the contended resource is the TEST environment rather than the branch: a merge to main, a push to a release branch and the twice-weekly scheduled run all want the same deployment, so one E2E cancels another wherever it came from. Scoping the group to the E2E job is what keeps make check-all and the SonarCloud scan out of it. Held at workflow level, the shared group let a scheduled or dispatched E2E run cancel a push run mid-validation, and the cancelling workflow repeats neither - main would have lost the lint, unit-test and scan signal it is the only place to get. The validation job carries no group at all, so nothing cancels it; E2E still waits on it through needs, the same ordering the single job had. Cancelling a running suite is safe here. Over the last 61 active days the final push to main had at least 180 minutes of clear runway on 59 of them, usually overnight, so each working day still ends with one completed suite on that day's tip, and the runs that get cancelled are on commits that were superseded anyway. Release branches see three pushes in five months. The SonarCloud scan also moves ahead of E2E within the validation job. Until #427 raised the job timeout, every push run on main reached it while still inside E2E, so the scan that followed was reported as skipped and main was never scanned - only pull requests were. release.yml can still run the suite outside this group; isolating it needs its own job there so serialization never covers the irreversible publish steps, tracked in MPT-25395. Refs MPT-25022. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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.
🤖 AI-generated PR — Please review carefully.
What
Two CI fixes for MPT-25021, where the E2E step on
mainhas hit the 90-minute job timeout on every push and cron run since 2026-06-25.make e2ealways passes the suite path. The target used$(if $(args),$(args), tests/e2e), so whenever CI passed Report Portal options throughargsthetests/e2epath was dropped and pytest fell back totestpaths = "tests". The E2E step therefore re-ran the 2310 unit tests, with coverage, after the e2e suite (June launches show 3613 items for that reason). The target now runstests/e2e(orpath=...for a subset) plusargs, and adds--no-covso an e2e run does not overwrite the unitcoverage.xmlthat SonarCloud reads.push-release-branch.yml,cron-main-e2e.ymlandrelease.yml.docs/e2e_tests.mdanddocs/testing.mddocument the newpath/argssplit.Why 180
The whole suite was run locally against TEST on 2026-09-16 in CI order (
-p no:randomly, no Report Portal): 1h52m wall for 1324 tests, 1187 passed, 33 skipped, 104 setup errors.The heavy areas today:
catalog/product32 min (the 16 term-variant tests alone are 30 min, up to 159 s each),program/program19.5 min (120 tests, ~1 s each in June, ~10 s now),accounts/accounts_users13 min (unchanged),commerce/asset7 min andcommerce/subscription6.5 min (both roughly doubled). The 20 streaming tests, which no CI launch had ever reached, take 75 s in total.150 would have left about 4 minutes on the slowest observed day. 180 leaves ~34 minutes there and ~60 on a day like today, while staying bounded; a per-test
pytest-timeoutis the right follow-up for genuine hangs and is not part of this PR.Found on the way: 102 helpdesk tests (chats, cases, queues) now fail at setup with
400 longDescription: Property is required, so the TEST API has started requiring that field on case creation. That is the block of 107 failures visible in Report Portal launch #333 and needs its own ticket.Why not more
The latency itself is on the TEST environment (
api.s1.show) and is being escalated separately. The concurrency guard and the Sonar-before-E2E move belong to MPT-25022 (#420), which touches the same workflow file and will need a small rebase after this merges.Testing
make -n e2e ...dry runs confirm the rendered pytest command for the CI invocation,path=tests/e2e/catalog, and the bare target.pytest tests/e2e --collect-only --no-covcollects 1324 tests and writes nocoverage.xml.ruff format --check,ruff check,flake8,mypy,uv lock --checkand the 2858 unit tests pass locally viauv run(Dockermake check-allcould not build here: the Docker disk is full). Workflow YAML parses; pre-commitcheck-yamlpassed on commit.mainafter merge finishing an E2E launch in Report Portal.🤖 Generated with Claude Code
Closes MPT-25021
make e2eto usetests/e2eby default.path=....args.--no-covduring E2E runs.