Conversation
|
👋 kalverra, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
✅ No conflicts with other open PRs targeting |
There was a problem hiding this comment.
Pull request overview
This PR adjusts integration test CI configuration to reduce the test burden on pull requests by removing flaky/heavy tests from the PR trigger, and adds regression test execution on tag pushes.
Changes:
- Removes
PR Integration CCIP Teststrigger from all in-memory integration tests in.github/integration-in-memory-tests.yml, keeping onlyNightly Integration CCIP Tests. - Disables CRE regression tests on PRs and adds a new tag-based condition to run regression on tag pushes in
.github/workflows/integration-tests.yml. - Removes
-parallel=Nflags from a specific subset of test commands in.github/e2e-tests.yml.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/integration-in-memory-tests.yml |
Removes PR Integration CCIP Tests trigger from all ~50 in-memory test entries, leaving only the nightly trigger |
.github/workflows/integration-tests.yml |
Sets CRE regression to false for PRs; adds a new # -- Tags -- block to enable regression on tag pushes |
.github/e2e-tests.yml |
Removes -parallel=N flag from selected test commands (OCR, Automation, Keeper, VRF, LogPoller, CCIP, etc.) |
| SHOULD_RUN_REGRESSION="true" | ||
| SHOULD_RUN_REGRESSION="false" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Should this label and all associated logic be removed/replaced with one that allows people to run it if they add a specific tag?
There was a problem hiding this comment.
@kalverra this needs to be modified to reverse current logic. Currently, we skip tests if label is present. Now we should run them if a label is present (we need to add a new one).
| test_env_type: in-memory | ||
| runs_on: ubuntu-latest | ||
| triggers: | ||
| - PR Integration CCIP Tests |
There was a problem hiding this comment.
How can people opt-in to these tests if they want to run them on their PR?
There was a problem hiding this comment.
You can still call it using workflow dispatch.
There was a problem hiding this comment.
There was a problem hiding this comment.
Even if that wasn't if: github.event_name == 'pull_request' these still wouldn't trigger because this: https://github.com/smartcontractkit/chainlink/blob/develop/.github/workflows/integration-in-memory-tests.yml#L90 is what dictates what runs for that trigger.
So removing it from "triggers" here means it will only run when a workflow passes the nightly trigger.
There was a problem hiding this comment.
I think we want to enable opt-in via a label and via workflow_dispatch. I've forgotten how complex we made this thing :/
| @@ -369,6 +363,7 @@ jobs: | |||
| SHOULD_RUN_REGRESSION="true" | |||
| fi | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
The env variable SKIP_E2E_TESTS_REGRESSION_LABEL_FOUND is declared in the step's env block but is never used in the script body after this PR removed the pull request condition that previously referenced it. This is now dead code that should be removed from the env block to keep the step clean and avoid confusion about its purpose.
erikburt
added
the
run-e2e-tests
Integration In Memory Testsfrom running on PRs as they were extremely flaky/just broken.CRE Regressiontests from running on PRs as they're quite heavy.