google-gemini
Infra: Bug: E2E did not run
We had a pr from a googler, https://github.com/google-gemini/gemini-cli/pull/9114, that had many failing tests including some that we think are required.
We need to understand what combination of things happened to let this in.
Sorry about this! Do you have a link to information about what was failing? I ran npm run test:e2e and npm run preflight in the branch with my change and without my change and I thought they got the same failures that looked really unrelated, but I guess I missed something or wasn't sync'd properly. 😬
@mattKorwel Any specifics on the failing tests? This is also critical for several of my team's workflows... how can I help resolve these?
Seems like all the of tests are skipped: https://github.com/google-gemini/gemini-cli/actions/runs/18042915615 The workflow file is before we added merge_queue_skipper. Our if statements look okay from first glance but suspect that one of the condition checks aren't correct and is causing the whole e2e workflow to be skipped over.
Similar case after merge queue skipper was added: https://github.com/google-gemini/gemini-cli/actions/runs/18188529827
Right now it seems like adding the label does not cause e2e to be triggered
We have instances where the workflow fails at merge queue skipper and causing the whole e2e to be skipped: https://github.com/google-gemini/gemini-cli/actions/runs/18232148734
Fixing this part doesn't fix all the issues but should allow us to cover some of the merges that go in without passing e2e
Update: need approval on the test PR there to test whether or not the updated workflow is working as intended
Standup update: Code changes ready to review, tags WAI, removed all checks on the E2E check job except for when forks have pull_request events
Update: Going to remove the label check and listening to pull_request_target event. We actually have a check for approvers permission to run workflow settings set that makes everything obsolete.
Standup update: Cannot test the pull_request_target event due to security. Github does not trigger pull_request_target events from pull requests from forks. Going to pair with Matt and see if we can test it somehow.
Standup update: Pending review from Matt, code works (I think) but not sure how to test
Same status as yesterday, linking PR: https://github.com/google-gemini/gemini-cli/pull/11028/files
Think this works but cannot test
Context:
- Currently skipped tests are interpreted as successes (Github built-in)
- For forked PRs, everything is skipped because we filter events for only label changes
- Once E2E workflow is ran for a fork with label, users can update their PR and everything is skipped, and interpreted as success
- We already require approval on our workflows, so we should just trigger E2E workflow on all forked PR changes
Changes needed:
- Skipped tests should not count as passing
- Label should be removed
- Forked PRs need to pass E2E tests before submission
Blockers:
- From Github documentation:
This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow.
Therefore, my code changes cannot be tested unless the changes are submitted.
E2e changes have been approved and are waiting to merge. We'll need to test this one after it lands and verify what worked or didn't. Moving to "In progress" while we test.
E2E changes did not work, specifically because the pull_request_target directly bypass protection settings. This is because
Workflows triggered by pull_request_target events are run in the context of the base branch. Since the base branch is considered trusted, workflows triggered by these events will always run, regardless of approval settings. For more information about the pull_request_target event, see [Events that trigger workflows](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target).
From github docs
Update: Chained workflow seems to work but things cannot be tested unless the code is in main. Currently: Trigger workflow (updated) triggers -> E2E workflow (on main)
Update: Added workflow dispatch to chained e2e workflow, getting it submitted so we can test trigger -> chained e2e
Update: Working on associating status back to the original PR. E2E runs after triggering.
Standup update: Got chained workflow running for pushes to main + merge queue but having some tasks skipped + merge queue skipper failing, have a PR out to fix that.
Update: Chained workflow might need a PAT to run since the SECRET_TOKEN from the chained workflow does not have write status permission. We need the write permission to report the status back to the original PR that triggered it.
Remaining work:
- Give gemini cli robot PAT status write permission
- Use Gemini cli robot PAT in chained e2e workflow to write status back to the original PR
- Use the chained e2e workflow in presubmit checks
I've requested commit status write permission but it needs approval from an admin.
This is done!
There is no longer a need for a special label. the E2E (Chained) workflow will be kicked off when a maintainer approves the workflow.