HYPERFLEET-605 - test: reorganize E2E testing cases#20
Conversation
WalkthroughThis PR deletes test-design/testcases/adapter-framework/adapter-framework.md and adds three new test design documents under test-design/testcases: adapter.md (two adapter framework test scenarios), cluster.md (three CLM cluster lifecycle test scenarios), and nodepool.md (two nodepool lifecycle test scenarios). All changes are documentation-only; no code, public APIs, or functional logic were modified. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
89e6957 to
45367c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@test-design/testcases/adapter.md`:
- Around line 127-138: The jq filter in the example selects .conditions[] |
select(.type=="Applied") but the example JSON shows "type": "Available", which
is inconsistent; update the example comment so the condition type matches the
filter (change the example's "type" from "Available" to "Applied") or
alternatively change the jq filter to select(.type=="Available")—apply the
change near the jq invocation and the example JSON so both use the same
condition type (referencing the jq filter expression and the example
.conditions[].type field).
- Around line 100-107: Replace the incorrect step title "Step 1: Test template
rendering errors" with a title that reflects this test case's purpose (e.g.,
"Step 1: Test API endpoint failures") in the test-design/testcases/adapter.md
file so the heading matches the described actions and expected results; update
only the heading text for this step and verify any internal references or
subsequent steps that mention this title (search for the exact string "Test
template rendering errors" and replace accordingly).
In `@test-design/testcases/cluster.md`:
- Around line 244-247: Step 3 uses inconsistent adapter names; update the names
to match the ones used in Step 2 by replacing "namespace-creator", "workload",
and "another workload" with the consistent identifiers "namespace-adapter",
"job-adapter", and "job-dependency-adapter" so the test case references the same
adapter names across the document (ensure the sequence remains:
namespace-adapter completes first, then job-adapter, then
job-dependency-adapter).
🧹 Nitpick comments (2)
test-design/testcases/adapter.md (1)
173-195: Clarify the timeout scenario in the example.The description on line 174 states "Configure AdapterConfig with non-existed conditions that can't meet the precondition," but the example checks for
clusterPhasevalues["NotReady", "Ready"]which are common cluster states. This creates confusion about how the timeout is triggered.Consider either:
- Using clearly non-existent phase values (e.g.,
["NonExistentPhase"]) to make the timeout scenario explicit, or- Adding a clarifying comment explaining why these specific phases will not be reached in the test scenario.
test-design/testcases/cluster.md (1)
147-152: Clarify which Kubernetes resources should be verified.The description on line 108 specifically mentions "namespace and job," but Step 2's action and expected result refer generically to "resource" (singular). For clarity, specify which resource types should be verified (e.g., namespace, job, or both).
💡 Suggested clarification
#### Step 2: Verify Kubernetes Resources Management **Action:** -- Verify resource created +- Verify resources created (namespace and job) **Expected Result:** -- Resource is created successfully with templated values rendered +- Resources are created successfully with templated values rendered (namespace and job)
…nup of nodepool and fix inconsistent issue
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test-design/testcases/adapter.md`:
- Line 61: The Step 3 heading currently reads "Wait for timeout and Verify
Timeout Handling" but belongs to the first test case about API endpoint
failures; update the Step 3 title in test-design/testcases/adapter.md to a
correct description for endpoint failure verification (e.g., "Wait for error and
Verify API Endpoint Failure Handling" or "Verify API Endpoint Failure
Handling"), ensuring the heading text that appears under the first test case
(Step 3) is replaced so it no longer references timeouts and matches the
failure-focused scenario.
🧹 Nitpick comments (3)
test-design/testcases/adapter.md (1)
42-42: Clarify the redundant wording.The phrase "Configure AdapterConfig with invalid AdapterConfig" is redundant and unclear. Consider rephrasing to specify what makes the configuration invalid.
📝 Suggested rewording
-- Configure AdapterConfig with invalid AdapterConfig (invalid K8s resource object) +- Configure AdapterConfig containing an invalid Kubernetes resource object - Deploy the test adaptertest-design/testcases/nodepool.md (2)
6-6: Consider shortening the lengthy test title.The title "Nodepools Resource Type - K8s Resource Check Aligned with Preinstalled NodePool Related Adapters Specified" is quite long (over 100 characters) and somewhat difficult to parse. A more concise title would improve readability in the table of contents and when referencing this test.
💡 Suggested alternatives
Consider one of these shorter alternatives:
Option 1:
-2. [Nodepools Resource Type - K8s Resource Check Aligned with Preinstalled NodePool Related Adapters Specified](`#test-title-nodepools-resource-type---k8s-resource-check-aligned-with-preinstalled-nodepool-related-adapters-specified`) +2. [Nodepools Resource Type - K8s Resources Created by Preinstalled Adapters](`#test-title-nodepools-resource-type---k8s-resources-created-by-preinstalled-adapters`)Option 2:
-2. [Nodepools Resource Type - K8s Resource Check Aligned with Preinstalled NodePool Related Adapters Specified](`#test-title-nodepools-resource-type---k8s-resource-check-aligned-with-preinstalled-nodepool-related-adapters-specified`) +2. [Nodepools Resource Type - Adapter-Managed K8s Resources Validation](`#test-title-nodepools-resource-type---adapter-managed-k8s-resources-validation`)Then update the corresponding heading at line 106.
Also applies to: 106-106
93-94: Inconsistent placeholder syntax in kubectl commands.The kubectl delete commands use mixed placeholder styles:
{cluster_id}with curly braces but<nodepool-resources>with angle brackets. Earlier in the document (line 61), placeholders consistently use curly braces{nodepool_id}. Consider standardizing to one style for clarity.🔧 Proposed fix for consistency
-kubectl delete -n {cluster_id} <nodepool-resources> +kubectl delete -n {cluster_id} {nodepool-resources}Apply this change at both lines 93 and 164.
Also applies to: 164-165
| **Expected Result:** | ||
| - API returns successful response | ||
|
|
||
| #### Step 3: Wait for timeout and Verify Timeout Handling |
There was a problem hiding this comment.
Step 3 title mentions "timeout" in the wrong test case.
This step title references "Wait for timeout and Verify Timeout Handling," but this is the first test case focused on API endpoint failures, not timeouts. The timeout-focused test case is the second one (starting at line 83). This appears to be a copy-paste error from the second test case.
📝 Proposed fix
-#### Step 3: Wait for timeout and Verify Timeout Handling
+#### Step 3: Verify failure status reporting
**Action:**
-- Wait for some minutes
+- Wait for adapter to process the invalid resource
- Verify adapter status🤖 Prompt for AI Agents
In `@test-design/testcases/adapter.md` at line 61, The Step 3 heading currently
reads "Wait for timeout and Verify Timeout Handling" but belongs to the first
test case about API endpoint failures; update the Step 3 title in
test-design/testcases/adapter.md to a correct description for endpoint failure
verification (e.g., "Wait for error and Verify API Endpoint Failure Handling" or
"Verify API Endpoint Failure Handling"), ensuring the heading text that appears
under the first test case (Step 3) is replaced so it no longer references
timeouts and matches the failure-focused scenario.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xueli181114 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 |
9449b0d
into
openshift-hyperfleet:main
|
|
||
| 1. [Clusters Resource Type - Workflow Validation](#test-title-clusters-resource-type---workflow-validation) | ||
| 2. [Clusters Resource Type - K8s Resources Check Aligned with Preinstalled Clusters Related Adapters Specified](#test-title-clusters-resource-type---k8s-resources-check-aligned-with-preinstalled-clusters-related-adapters-specified) | ||
| 3. [Clusters Resource Type - Adapter Dependency Relationships Workflow Validation for Preinstalled Clusters Related Dependent Adapters](#test-title-clusters-resource-type---adapter-dependency-relationships-workflow-validation-for-preinstalled-clusters-related-dependent-adapters) | ||
| --- | ||
|
|
There was a problem hiding this comment.
Can you add one more case to verify a scenario : two and above cluster provision cluster with at least one of the adapters are in progress. This can verify observed generation id.
Summary
Summary by CodeRabbit