Skip to content

fix(ci): run the compile, chart and e2e jobs on merge - #2963

Open
mesutoezdil wants to merge 1 commit into
Project-HAMi:masterfrom
mesutoezdil:fix/ci-run-jobs-on-merge
Open

fix(ci): run the compile, chart and e2e jobs on merge#2963
mesutoezdil wants to merge 1 commit into
Project-HAMi:masterfrom
mesutoezdil:fix/ci-run-jobs-on-merge

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@hami-robot

hami-robot Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mesutoezdil
Once this PR has been reviewed and has the lgtm label, please assign shouren for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 09e21918-13eb-4f6b-b145-2e222b9e4186

📥 Commits

Reviewing files that changed from the base of the PR and between 5560887 and f4a329b.

📒 Files selected for processing (1)
  • .github/workflows/ci.yaml

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The CI workflow now runs build, chart packaging, and E2E jobs on master. The MIG smoke test also runs on master when ENABLE_MIG_E2E is true.

Changes

Master branch CI execution

Layer / File(s) Summary
Expand CI job conditions
.github/workflows/ci.yaml
The build, package_chart, and e2e_test jobs run when E2E execution is enabled or the workflow runs on master. The MIG smoke test also requires ENABLE_MIG_E2E.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 50298

This change enables the existing compile, chart, and end-to-end checks to run on merges to master while preserving pull-request behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: spencercjh

Poem

A rabbit checks the branch,
Master hops into the flow,
Build and charts run bright,
E2E tests join the trail,
MIG waits for its switch.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the CI change: compile, chart, and E2E jobs now run on merges into master.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mesutoezdil

Copy link
Copy Markdown
Contributor Author

/kind bug

@hami-robot hami-robot Bot added the kind/bug Something isn't working label Sep 2, 2026
@mesutoezdil
mesutoezdil force-pushed the fix/ci-run-jobs-on-merge branch from 5560887 to f4a329b Compare September 2, 2026 20:47
The comments say these jobs execute when hami code merges, but they gate
only on get_info.outputs.e2e_run. That value comes from
git diff $(github.event.pull_request.base.sha) $(github.sha), and on a
push the pull_request context is empty, so the diff compares against
nothing and yields no files. The run for 95530c6 on master logs an empty
PR commit and reports Compile, package_chart, e2e_test and e2e_mig_test
as skipped.

The test job already carries the master escape hatch. Give the remaining
four the same one so the whole set agrees. Compile and package_chart run
on ubuntu-22.04, so they are safe anywhere, but the two e2e jobs need the
self-hosted nvidia runners, which only the upstream repository has. Their
escape hatch is scoped with github.repository so a fork pushing to its
own master does not queue a job that can never be picked up. The workflow
is meant to run on forks, as its own header comment says.

Pull requests are unaffected: github.ref is refs/pull/N/merge there, so a
docs only change still skips everything it skipped before.

Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
@mesutoezdil
mesutoezdil force-pushed the fix/ci-run-jobs-on-merge branch from f4a329b to 5029836 Compare September 3, 2026 12:10
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unittests 70.85% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

kind/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant