feat: Move some GitHub public e2e test to GHE#2466
Conversation
🔍 PR Lint Feedback
|
Summary of ChangesHello @chmouel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the testing infrastructure by migrating a significant portion of GitHub-related end-to-end tests to utilize a second controller, likely for GitHub Enterprise environments. This change ensures broader test coverage and validation of GitHub integration functionalities across different setups. Additionally, it optimizes test execution by rebalancing the load of certain test groups to run concurrently. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors a number of GitHub e2e tests to run them against a second controller, which appears to be a GitHub Enterprise instance. The changes primarily involve renaming test functions to reflect their new target and setting the SecondController flag to true in the test configurations. Additionally, the CI workflow script has been updated to rebalance the test execution load. The changes are consistent, straightforward, and appear to be correct. I have no specific comments as the implementation aligns well with the stated goals.
There was a problem hiding this comment.
Pull request overview
This PR migrates several GitHub integration e2e tests to run against a GitHub Enterprise (GHE) second controller instance. The changes ensure broader test coverage by distributing tests across both the primary GitHub controller and a secondary GHE controller. Additionally, the PR rebalances test loads by moving "Others" tests from the github_second_controller execution group to the concurrency group.
Changes:
- Renamed multiple GitHub test functions to include "Second" prefix (e.g.,
TestGithubPacCli→TestGithubSecondPacCli) - Added
SecondController: trueflag to test configurations to enable second controller usage - Updated CI workflow script to route "Others" tests to the concurrency group instead of github_second_controller group
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/github_tkn_pac_cli_test.go | Renamed test to TestGithubSecondPacCli and enabled second controller via Setup() |
| test/github_tag_gitops_test.go | Renamed test to TestGithubSecondGitOpsCommentOnTag and enabled second controller |
| test/github_skip_ci_test.go | Renamed 3 skip CI tests and added SecondController: true flag |
| test/github_push_retest_test.go | Renamed 3 push/retest tests and added SecondController: true flag |
| test/github_pullrequest_test_comment_test.go | Renamed test and added SecondController: true flag |
| test/github_pullrequest_test.go | Renamed 12 pull request tests and added SecondController: true flag |
| test/github_pullrequest_retest_test.go | Renamed retest test and added SecondController: true flag |
| test/github_incoming_test.go | Renamed 10 incoming webhook tests and updated verifyIncomingWebhook calls |
| test/github_config_maxkeepruns_test.go | Renamed MaxKeepRuns test and added SecondController: true flag |
| hack/gh-workflow-ci.sh | Updated test routing: moved "Others" to concurrency group, removed from github_second_controller group |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I haven't splitted second as concurrent job, let's see if sequential help first |
1ee1571 to
018b9ce
Compare
|
analyse for other tests to be moved on GHE |
018b9ce to
1aace82
Compare
The regular expression used to match the unsupported delete tag event log message was too strict. It did not account for potential variations in the message content that could follow the core string. The regex was updated to be more flexible by allowing any characters to follow the initial phrase, ensuring more reliable log matching. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Added the `SecondController: true` flag to numerous GitHub integration tests. This change enabled the use of the second controller in these tests, ensuring broader coverage and verification of the GitHub integration's functionality. The modification spanned various test files, including those for pull requests, incoming webhooks, push events, and skip CI scenarios. The TestOthers were also moved to concurrency to balance the loads. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1aace82 to
3771d07
Compare
Added the
SecondController: trueflag to numerous GitHub integration tests. This change enables the use of the second controller in these tests, ensuring broader coverage and verification of the GitHub integration's functionality. The modification spans various test files, including those for pull requests, incoming webhooks, push events, and skip CI scenarios.And at the same time move the TestOthers to concurrency to balance the loads.
📝 Description of the Change
👨🏻 Linked Jira
🔗 Linked GitHub Issue
Fixes #
🚀 Type of Change
fix:)feat:)feat!:,fix!:)docs:)chore:)refactor:)enhance:)deps:)🧪 Testing Strategy
🤖 AI Assistance
If you have used AI assistance, please provide the following details:
Which LLM was used?
Extent of AI Assistance:
Important
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Gemini gemini@google.com
Co-authored-by: ChatGPT noreply@chatgpt.com
Co-authored-by: Claude noreply@anthropic.com
Co-authored-by: Cursor noreply@cursor.com
Co-authored-by: Copilot Copilot@users.noreply.github.com
**💡You can use the script
./hack/add-llm-coauthor.shto automatically addthese co-author trailers to your commits.
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.