Cluster listing test case refactor: move into an existing test#4816
Cluster listing test case refactor: move into an existing test#4816openshift-merge-bot[bot] merged 3 commits intomainfrom
Conversation
move the cluster listing test case into cluster sharing RG test case to better handle resources and avoid the potentially flaky timeout:0 https://redhat.atlassian.net/browse/ARO-25831
There was a problem hiding this comment.
Pull request overview
Refactors E2E coverage for resource-group-level cluster listing by removing the standalone “list clusters without node pools” test and incorporating equivalent listing validation into the existing “clusters sharing resource group” scenario, with corresponding suite fixture updates.
Changes:
- Deleted
cluster_list_no_infra.goand removed its test from generated suite-listing fixtures. - Added RG-level cluster listing assertions to
clusters_sharing_resgroup.go(expects exactly the two created clusters). - Adjusted
simple_negative_cases.goto accumulate errors more defensively around nodepool version update and to reduce error-level logging for expected failures.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/testdata/zz_fixture_TestMainListSuitesForEachSuite_stage_parallelstage_parallel.txt | Removes the deleted listing test from suite output fixture. |
| test/testdata/zz_fixture_TestMainListSuitesForEachSuite_rp_api_compat_all_parallelrp_api_compat_all_parallel.txt | Removes the deleted listing test from suite output fixture. |
| test/testdata/zz_fixture_TestMainListSuitesForEachSuite_rp_api_compat_all_parallel_01rp_api_compat_all_parallel_development.txt | Removes the deleted listing test from suite output fixture. |
| test/testdata/zz_fixture_TestMainListSuitesForEachSuite_prod_parallelprod_parallel.txt | Removes the deleted listing test from suite output fixture. |
| test/testdata/zz_fixture_TestMainListSuitesForEachSuite_integration_parallelintegration_parallel.txt | Removes the deleted listing test from suite output fixture. |
| test/testdata/zz_fixture_TestMainListSuitesForEachSuite_dev_cd_check_paralleldev_cd_check_parallel.txt | Removes the deleted listing test from suite output fixture. |
| test/e2e/simple_negative_cases.go | Makes nodepool version-update negative case more defensive; changes expected-error logging behavior. |
| test/e2e/clusters_sharing_resgroup.go | Adds RG-level listing check for the two created clusters in the shared customer RG. |
| test/e2e/cluster_list_no_infra.go | Deletes the standalone RG cluster listing test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mbukatov, mgahagan73, mvacula02 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ARO-25831
What
Moves the cluster listing test case into clusters_sharing_resgroup.go test case. The coverage is slightly changed - instead of listing for 1 cluster in separate resource group twice, we now list for 2 clusters within the same resource group.
Why
Currently, the test is creating (seeding) 2 clusters without waiting (0 timeout). This can cause unexpected behavior. Waiting for the clusters, on the other hand, is a wasteful use of resources for such a simple task.
Test clusters_sharing_resgroup.go is an ideal candidate for implementation of the cluster listing test case as it already stands up 2 clusters and expects 3rd cluster to fail. Listing (for only 2 expected) clusters adds another layer of validation as a byproduct.
Testing
This is a refactor of a positive E2E test.