fix: Rename gh Second to GHE and move remove flaky#2480
fix: Rename gh Second to GHE and move remove flaky#2480chmouel merged 1 commit intotektoncd:mainfrom
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 refactors the GitHub End-to-End testing infrastructure to enhance clarity and maintainability. The primary goal is to standardize the naming convention for GitHub Enterprise tests, moving from an ambiguous 'second controller' designation to a more explicit 'GHE'. Additionally, the CI workflow has been optimized by consolidating flaky tests into the main test matrix, which simplifies test management and execution. These changes collectively improve the organization and efficiency of the testing suite. Highlights
Changelog
Ignored Files
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
The pull request successfully renames the GitHub Enterprise (GHE) provider from github_second_controller to github_ghe across the CI scripts and E2E tests. It also consolidates the public GitHub tests into a single github_public target and ensures flaky tests are correctly routed to the flaky job. The changes are consistent and maintain the existing test partitioning logic. Note that TEST_GITHUB_SECOND_EL_URL is now a required environment variable for GHE tests, and the consolidation of public GitHub tests into a single job may impact CI parallelism if the number of tests grows significantly.
956ca56 to
ec972c1
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves the clarity and consistency of GitHub E2E test naming by replacing ambiguous "Second" terminology with the clearer "GHE" (GitHub Enterprise) designation. The changes also consolidate the CI pipeline by moving flaky tests from a separate job into the main test matrix, simplifying the workflow configuration.
Changes:
- Renamed
github_second_controllerprovider togithub_ghethroughout CI configuration, scripts, and test functions - Renamed
SecondControllerstruct field toGHEandonSecondControllerparameter toonGHEin test helper functions - Consolidated flaky tests into the main e2e-tests matrix job, removing the dedicated
e2e-flaky-testsjob - Removed the splitting of public GitHub tests into
github_1andgithub_2, replacing with singlegithub_publicprovider
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/pkg/github/setup.go | Renamed onSecondController parameter to onGHE for clarity |
| test/pkg/github/pr.go | Renamed SecondController field to GHE in PRTest struct |
| test/pkg/github/instrumentation.go | Updated field references from SecondController to GHE |
| test/github_tkn_pac_cli_test.go | Renamed test function from TestGithubSecond* to TestGithubGHE* |
| test/github_tag_gitops_test.go | Renamed test function to use GHE naming convention |
| test/github_skip_ci_test.go | Renamed test functions and updated struct field references to use GHE |
| test/github_push_test.go | Renamed test functions and updated struct initialization |
| test/github_push_retest_test.go | Renamed test functions and updated struct field references |
| test/github_pullrequest_test_comment_test.go | Renamed test functions and updated struct initialization |
| test/github_pullrequest_test.go | Renamed 15 test functions from TestGithubSecond* to TestGithubGHE* |
| test/github_pullrequest_retest_test.go | Renamed test functions and updated comments |
| test/github_pullrequest_privaterepository_test.go | Renamed test function and updated struct initialization |
| test/github_pullrequest_concurrency_test.go | Renamed 4 test functions and updated comments |
| test/github_pullrequest_concurrency_multiplepr_test.go | Renamed test function and updated comments |
| test/github_incoming_test.go | Renamed 9 test functions and updated parameter names |
| test/github_config_maxkeepruns_test.go | Renamed test function and updated struct initialization |
| test/README.md | Updated documentation to reflect new provider categories |
| hack/gh-workflow-ci.sh | Updated test filtering logic and removed github_1/github_2 split |
| .github/workflows/e2e.yaml | Updated matrix providers and removed separate flaky test job |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ec972c1 to
5f24319
Compare
1c6eefb to
28f3d34
Compare
| local -a github_tests=() | ||
| if [[ "${target}" == *"github"* ]] && [[ "${target}" != "github_second_controller" ]]; then | ||
| mapfile -t github_tests < <(echo "${all_tests}" | grep -iP '^TestGithub' 2>/dev/null | grep -ivP 'Concurrency|GithubSecond|Flaky' 2>/dev/null | sort 2>/dev/null) | ||
| if [[ "${target}" == *"github"* ]] && [[ "${target}" != "github_ghe" ]] && [[ "${target}" != "github_second_controller" ]]; then |
There was a problem hiding this comment.
| if [[ "${target}" == *"github"* ]] && [[ "${target}" != "github_ghe" ]] && [[ "${target}" != "github_second_controller" ]]; then | |
| if [[ "${target}" == *"github"* ]] && [[ "${target}" != "github_ghe" ]]; then |
I think its not needed then?
There was a problem hiding this comment.
yeah it's to be able to test CI on this pr since we it refers to the e2e.yaml from main will remove when it's green (and then it will be red until we merge)
|
after the tiny suggestion /lgtm |
ae2c83c to
3a45a66
Compare
Refactor the end-to-end test suite to standardize naming and simplify the CI workflow. Rename "SecondController" references to "GHE" across test files, structs, and helper functions to clearly identify GitHub Enterprise scenarios. Update the GitHub Actions workflow to use descriptive matrix targets (`github_public`, `github_ghe`) and eliminate the separate flaky test job. Modify the CI script to support these new targets while retaining backward compatibility for the previous target names. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
3a45a66 to
ec7e2dc
Compare

Description
Renames the
github_second_controllerE2E provider togithub_ghefor clarityand simplifies the CI matrix by folding the standalone
e2e-flaky-testsjobinto the main
e2e-testsmatrix.github_second_controllerprovider togithub_ghein CI workflow,hack/gh-workflow-ci.sh, andtest/README.mde2e-flaky-testsjob into thee2e-testsmatrix asthe
flakyprovider, removing ~130 lines of duplicated CI configgithub_1/github_2splits into a singlegithub_publicprovider
PRTest.SecondControllerfield toPRTest.GHEand theSetupparameter
onSecondControllertoonGHETestGithubSecond*test functions toTestGithubGHE*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.