HYPERFLEET-533 - test: update E2E Test Cases according to the new scope #15
Conversation
WalkthroughThis PR updates test documentation and a template: it edits test-design/templates/testcase-template.md to remove the Title and Feature rows, changes Priority values from [Critical/High/Medium/Low] to [Tier0/Tier1/Tier2], and adds Created and Updated rows. It also adds test-design/testcases/adapter-framework/adapter-framework.md — a new, detailed test document describing multiple Adapter Framework — Customization scenarios (preconditions, steps, expected results). No code or runtime behavior changed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
yingzhanredhat
force-pushed
the
ying-e2e
branch
from
8503b3d to
4e8c82b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@test-design/testcases/cluster/adapter-framework.md`:
- Around line 52-56: The curl examples for creating clusters (the POST to
${API_URL}/api/hyperfleet/v1/clusters that currently uses -H "Content-Type:
application/json") and the related GET examples must include an authentication
header; update each curl invocation to add a placeholder auth header such as
Authorization: Bearer <TOKEN> (or an API key header placeholder) and document
the placeholder so testers replace it with real credentials; apply this change
consistently to the POST example shown and all GET request examples elsewhere in
the file.
- Around line 652-662: Clarify whether adapters 2 and 3 are parallel or
sequential by updating the Expected Result paragraph: state explicitly if
Adapter 2 (workload) and Adapter 3 (another workload) both only depend on
Adapter 1 (namespace-creator) and may run in parallel after Adapter 1 completes,
or state that Adapter 3 depends on Adapter 2 and must run sequentially; update
the bullet list and the preconditions so they reference the adapter identities
(adapter 1: namespace-creator, adapter 2: workload, adapter 3: another workload)
and the NamespaceReady status reported by adapter 1 to remove ambiguity.
- Around line 548-556: Update the “Simulate HyperFleet API unavailability” step
to specify a safe, scoped method: e.g., use a Kubernetes NetworkPolicy applied
to only the test namespace to deny ingress to the hyperfleet-api service, or
scale the hyperfleet-api deployment to 0 replicas within the test namespace via
kubectl scale deployment hyperfleet-api -n <test-namespace> --replicas=0;
document the exact rollback commands (delete the NetworkPolicy or scale back to
original replica count) and include a fallback mock approach (run a local proxy
or mock endpoint that returns controlled failures) and ensure the step
references the test namespace and the log command (kubectl logs -l
app=hyperfleet-adapter) so the impact is limited and easily reversible.
🧹 Nitpick comments (3)
test-design/testcases/cluster/adapter-framework.md (3)
127-127: Consider using more realistic example values forobserved_generation.The value
12345appears as a placeholder across multiple test cases. While this is acceptable for examples, using values like1,2, or describing it as<generation_number>might better indicate this field contains the actual generation number from the Kubernetes resource.📝 Suggested clarification
- "observed_generation": 12345, + "observed_generation": 1, // Actual generation from cluster resourceOr use a placeholder format:
- "observed_generation": 12345, + "observed_generation": <generation_number>,Also applies to: 291-291, 416-416
36-36: Track completion of test data placeholders.All test cases include the note "(The detailed test data will be added soon)" in Step 1. Since these tests are marked as "Draft", this is appropriate, but consider:
- Creating tracking issues for each test case's test data
- Adding a TODO section at the document level listing what's needed
- Estimating when test data will be available to support automation efforts
Would you like me to generate a checklist or tracking template for the missing test data across all test cases?
Also applies to: 185-185, 347-347, 475-475, 604-604
13-13: Optional: Fix table formatting for markdown linter compliance.The markdown linter (markdownlint-cli2) flags table formatting issues where pipes are missing spaces for the "compact" style. While this doesn't affect readability or functionality, fixing it would ensure clean linter output.
🎨 Suggested formatting fix
Current format:
|-----------|-----------|Suggested format with compact spacing:
| --------- | --------- |This applies to all metadata tables at lines 13, 160, 323, 452, and 582.
Also applies to: 160-160, 323-323, 452-452, 582-582
|
/test lint |
yingzhanredhat
force-pushed
the
ying-e2e
branch
from
4e8c82b to
43975c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test-design/testcases/cluster/adapter-framework.md`:
- Around line 786-788: Fix the typo "paramter" to "parameter" in the test steps
text: update the line containing "Configure adapter with simulateResult paramter
to make adapter job stuck" (and verify any other occurrences of "paramter") so
it reads "Configure adapter with simulateResult parameter to make adapter job
stuck"; leave "Configure adapter with maxWaitTimeSeconds parameter" unchanged.
43975c2 to
22b80cc
Compare
|
|
||
| --- | ||
|
|
||
| ## Test Title: Adapter framework can detect and report invalid adapter configuration failures |
There was a problem hiding this comment.
This seems to me more of a unit tests, since it doesn't imply multiple coponents.
wdyt?
There was a problem hiding this comment.
Agree it. Is it enough to cover the adapter failure deployment scenarios in unit/integration test? Is it required to add the real adapter failed deployed scenarios in E2E test?
|
|
||
| --- | ||
|
|
||
| ## Test Title: Adapter framework can report execution status to cluster API endpoints |
There was a problem hiding this comment.
This is a tier 0 case. I think the first case already covers it.
There was a problem hiding this comment.
Do we need to cover different results?
for example : adapter failed at 'Available' or 'Applied' , Or adapter failed at postActions to report the result to hyperfleet api ?
yingzhanredhat
force-pushed
the
ying-e2e
branch
from
22b80cc to
4d5dba5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@test-design/testcases/adapter-framework/adapter-framework.md`:
- Line 252: Update the incomplete sentence "The second and third adapters for
preconditions to be met" to include the missing verb and remove the double
space; change it to "The second and third adapters wait for preconditions to be
met." Ensure the corrected sentence replaces the original exact phrase in
adapter-framework.md.
- Line 113: The Priority cell in the testcases table uses "Tier 1" but all other
testcases use the compact format (Tier0, Tier1, Tier2); update the table entry
that currently reads "**Priority** | Tier 1" to use "Tier1" so it matches the
project's naming convention and other testcases (search for the table row
containing the "**Priority**" cell in adapter-framework.md or the string "Tier
1" to locate the exact spot).
- Line 53: Change the sentence "Adapter can communicates with HyperFleet API
successfully" to correct verb agreement by using either "Adapter can communicate
with HyperFleet API successfully" or "Adapter communicates with HyperFleet API
successfully"; update the line in the test case description where that exact
phrase appears to use one of these corrected forms.
- Line 198: Update the inconsistent priority tier text in the markdown table by
replacing the cell value "Tier 1" with "Tier1" so it matches the formatting used
elsewhere; specifically edit the table row containing "**Priority** | Tier 1" in
adapter-framework.md to read "**Priority** | Tier1".
- Around line 450-451: There are two consecutive "**Action:**" labels
duplicated; remove the first duplicate occurrence so only a single "**Action:**"
remains before the action content. Locate the adjacent duplicated markdown
labels (“**Action:**” followed immediately by another “**Action:**”) and delete
the extra one so the section uses a single "**Action:**" header.
🧹 Nitpick comments (2)
test-design/testcases/adapter-framework/adapter-framework.md (2)
511-511: Standardize horizontal rule to three dashes.Line 511 uses "-----" (5 dashes) while all other horizontal rules in the document use "---" (3 dashes). Maintain consistency.
♻️ Proposed fix
- # }
}
----- +---
534-540: Complete or remove the unfinished test case.This test case section contains only a
<TODO>marker with empty Preconditions and Test Steps sections. Consider either:
- Completing the test case before merging
- Removing it from this PR and tracking it separately
- If intentionally incomplete, add a comment explaining when it will be completed
Do you want to complete this test case now, or should it be removed and tracked in a separate issue?
yingzhanredhat
force-pushed
the
ying-e2e
branch
from
4d5dba5 to
65abfbb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@test-design/testcases/adapter-framework/adapter-framework.md`:
- Around line 170-173: Update the Step 4 action text under "Step 4: Verify
Kubernetes Resources Management" to a complete, explicit instruction: replace
the incomplete line "Verify resource created" with something like "Verify that
the expected Kubernetes resource(s) were created successfully (for example,
check that the Deployment/Pod/Service specified earlier exists and is in the
Ready state) and confirm their status using kubectl or your cluster UI." Include
mention of which resource types to check (Deployment, Pod, Service, etc.) so the
verification is unambiguous.
- Around line 134-136: Rewrite the awkward phrasing and spacing in the adapter
resources description: replace the line "Configure adapter resources section
with templated manifests using Go template syntax: (The detailed test data will
be added soon)" with a cleaner sentence like "Configure adapter resources using
templated manifests (Go template syntax); detailed test data will be added
soon." Also fix the list item wording by changing "K8s Resource(e.g Namespace)
created" to "K8s resource (e.g., Namespace) created" (add spacing, lowercasing,
and comma) and keep/verify "Configure resource labels for discovery" reads
clearly.
- Line 102: Update the test title string "K8S object can be created correctly on
the regional cluster without maestro enable" to correct grammar and product-name
casing by changing it to "K8S object can be created correctly on the regional
cluster without Maestro enabled"; locate and edit the header line in
test-design/testcases/adapter-framework/adapter-framework.md that contains the
exact title text and replace only the phrase "without maestro enable" with
"without Maestro enabled".
🧹 Nitpick comments (1)
test-design/testcases/adapter-framework/adapter-framework.md (1)
533-538: Replace<TODO>with concrete content or track it.Leaving a placeholder in the test spec makes the scenario unexecutable. If this is intentionally deferred, please add a tracking issue ID or brief scope note.
Do you want me to draft the missing Preconditions/Test Steps section or open a tracking issue template?
|
/test lint |
|
/test images-images |
|
/test e2e-images |
yingzhanredhat
force-pushed
the
ying-e2e
branch
from
65abfbb to
90a3a79
Compare
|
/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 |
openshift-merge-bot
bot
merged commit f43255a
into
openshift-hyperfleet:main
3 participants
Add some positive E2E testcase according to new scope
Summary by CodeRabbit
Documentation
Chores