Skip to content
This repository was archived by the owner on Jan 16, 2026. It is now read-only.

fix(ci): reduce ci usage by only running jobs on merge group#3194

Merged
theochap merged 1 commit intomainfrom
theo/reduce-ci-usage
Dec 15, 2025
Merged

fix(ci): reduce ci usage by only running jobs on merge group#3194
theochap merged 1 commit intomainfrom
theo/reduce-ci-usage

Conversation

@theochap
Copy link
Member

Description

CI usage has been high, this reduces the usage by running expensive jobs only in the mq

@theochap theochap requested a review from BioMark3r as a code owner December 12, 2025 22:37
Copilot AI review requested due to automatic review settings December 12, 2025 22:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to reduce CI usage by restricting expensive test workflows (Proof, Sysgo E2E Tests, and Acceptance Tests) to run only in the merge group, removing both pull_request and push triggers. While the intention to reduce CI costs is understandable, this approach has significant workflow implications that need to be carefully considered.

Key Changes:

  • Removed pull_request trigger from three expensive test workflows
  • Removed push trigger for main branch from the same workflows
  • Workflows now only run in merge_group context

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
.github/workflows/proof.yaml Removed pull_request and push triggers, keeping only merge_group for proof/FPP tests
.github/workflows/node_e2e_sysgo_tests.yaml Removed pull_request and push triggers, keeping only merge_group for Sysgo E2E tests
.github/workflows/acceptance-tests.yaml Removed pull_request and push triggers, keeping only merge_group for acceptance tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -1,9 +1,6 @@
name: Proof
on:
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Removing the pull_request trigger means these critical tests will not run on pull requests at all. This creates a significant workflow issue:

  1. Developers won't see test results during PR review
  2. Test failures will only be discovered in the merge queue, not during development
  3. This delays feedback and can block the merge queue with failing PRs

If the goal is to reduce CI usage, consider alternative approaches:

  • Use path filters to only run tests when relevant files change
  • Use conditional logic based on labels (e.g., only run on PRs labeled "ready-for-review")
  • Use workflow_run to trigger only after other checks pass
  • Keep pull_request but add conditions to skip on draft PRs

Completely removing pull_request triggers from test workflows is likely to cause more problems than it solves.

Suggested change
on:
on:
pull_request:
paths:
- 'tests/**'
- '.github/**'
- 'justfile'
- 'Cargo.*'
- '**/Cargo.*'

Copilot uses AI. Check for mistakes.
@@ -1,9 +1,6 @@
name: Sysgo E2E Tests
on:
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Removing the pull_request trigger means these critical tests will not run on pull requests at all. This creates a significant workflow issue:

  1. Developers won't see test results during PR review
  2. Test failures will only be discovered in the merge queue, not during development
  3. This delays feedback and can block the merge queue with failing PRs

If the goal is to reduce CI usage, consider alternative approaches:

  • Use path filters to only run tests when relevant files change
  • Use conditional logic based on labels (e.g., only run on PRs labeled "ready-for-review")
  • Use workflow_run to trigger only after other checks pass
  • Keep pull_request but add conditions to skip on draft PRs

Completely removing pull_request triggers from test workflows is likely to cause more problems than it solves.

Suggested change
on:
on:
pull_request:
paths:
- 'node/**'
- 'tests/**'
- '.github/workflows/node_e2e_sysgo_tests.yaml'

Copilot uses AI. Check for mistakes.
@@ -1,9 +1,6 @@
name: Acceptance Tests
on:
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Removing the pull_request trigger means these critical tests will not run on pull requests at all. This creates a significant workflow issue:

  1. Developers won't see test results during PR review
  2. Test failures will only be discovered in the merge queue, not during development
  3. This delays feedback and can block the merge queue with failing PRs

If the goal is to reduce CI usage, consider alternative approaches:

  • Use path filters to only run tests when relevant files change
  • Use conditional logic based on labels (e.g., only run on PRs labeled "ready-for-review")
  • Use workflow_run to trigger only after other checks pass
  • Keep pull_request but add conditions to skip on draft PRs

Completely removing pull_request triggers from test workflows is likely to cause more problems than it solves.

Suggested change
on:
on:
pull_request:

Copilot uses AI. Check for mistakes.
on:
push:
branches: [main]
merge_group:
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Removing the push trigger for the main branch means these tests won't run after PRs are merged to main. While merge_group should catch issues before merging, consider these scenarios:

  1. Direct pushes to main (if permitted) won't trigger tests
  2. Post-merge verification won't occur
  3. Branch protection may not have visibility into the health of the main branch

If your workflow relies exclusively on merge queues and direct pushes to main are disabled via branch protection, this may be acceptable. However, it's worth verifying that this aligns with your repository's branch protection rules and merge strategy.

Suggested change
merge_group:
merge_group:
push:
branches:
- main

Copilot uses AI. Check for mistakes.
@@ -1,9 +1,6 @@
name: Sysgo E2E Tests
on:
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Removing the push trigger for the main branch means these tests won't run after PRs are merged to main. While merge_group should catch issues before merging, consider these scenarios:

  1. Direct pushes to main (if permitted) won't trigger tests
  2. Post-merge verification won't occur
  3. Branch protection may not have visibility into the health of the main branch

If your workflow relies exclusively on merge queues and direct pushes to main are disabled via branch protection, this may be acceptable. However, it's worth verifying that this aligns with your repository's branch protection rules and merge strategy.

Suggested change
on:
on:
push:
branches:
- main

Copilot uses AI. Check for mistakes.
@@ -1,9 +1,6 @@
name: Acceptance Tests
on:
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Removing the push trigger for the main branch means these tests won't run after PRs are merged to main. While merge_group should catch issues before merging, consider these scenarios:

  1. Direct pushes to main (if permitted) won't trigger tests
  2. Post-merge verification won't occur
  3. Branch protection may not have visibility into the health of the main branch

If your workflow relies exclusively on merge queues and direct pushes to main are disabled via branch protection, this may be acceptable. However, it's worth verifying that this aligns with your repository's branch protection rules and merge strategy.

Suggested change
on:
on:
push:
branches:
- main

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Dec 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.1%. Comparing base (820ff00) to head (d3e48d3).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

❗ There is a different number of reports uploaded between BASE (820ff00) and HEAD (d3e48d3). Click for more details.

HEAD has 35 uploads less than BASE
Flag BASE (820ff00) HEAD (d3e48d3)
proof 14 0
e2e 20 0
unit 2 1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@theochap theochap enabled auto-merge December 15, 2025 15:05
@theochap
Copy link
Member Author

Force merging that one so that the changes are applied

@theochap theochap disabled auto-merge December 15, 2025 15:56
@theochap theochap merged commit e408b6b into main Dec 15, 2025
37 of 38 checks passed
@theochap theochap deleted the theo/reduce-ci-usage branch December 15, 2025 15:56
theochap added a commit to ethereum-optimism/optimism that referenced this pull request Jan 15, 2026
…ona#3194)

## Description

CI usage has been high, this reduces the usage by running expensive jobs
only in the mq
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants