HYPERFLEET-779 - feat: add E2E test case automation skill to devtools plugin#26
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR bumps the plugin version to 0.5.0, adds an E2E Test Automation entry to the README (including CLI/tool name and metadata updates), and introduces a new e2e-test-automation skill with three documents: SKILL.md (automation workflow, compatibility ladder, phased control flow, and output templates), tier0-automation-logic.md (Tier0 constraints, adapter rules, and standardized control flow), and tier1-tier2-automation-logic.md (per-test adapter deployment lifecycle, patterns, and mapping to Ginkgo/Gomega). Sequence Diagram(s)sequenceDiagram
participant User as User (test author)
participant Tool as E2E Test Automation Tool
participant Repo as Codebase (pkg/, e2e/)
participant Deploy as Deploy Scripts / Env
participant Runner as Ginkgo/Gomega Runner
participant Cluster as Kubernetes / Adapters
User->>Tool: submit test-case document
Tool->>Repo: inspect pkg/ helpers, wrappers, existing tests
Tool-->>Tool: classify scenario (new vs update) and tier
alt Tier1/Tier2
Tool->>Deploy: trigger adapter deploy scripts (BeforeEach)
Deploy->>Cluster: deploy adapter(s)
Cluster->>Deploy: report readiness/registration
Deploy->>Tool: report readiness
end
Tool->>Tool: generate or update Ginkgo/Gomega test (It/BeforeEach/AfterEach)
Tool->>Runner: provide run instructions or execute focused It()
Runner->>Cluster: perform API calls and adapter interactions
Cluster->>Runner: return results and conditions
Runner->>Tool: deliver test artifacts & outcomes
alt Tier1/Tier2
Tool->>Deploy: trigger adapter undeploy scripts (AfterEach)
Deploy->>Cluster: remove test-specific adapter(s)
end
Tool->>User: deliver generated/updated code and run command
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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 |
7a41dad to
4996c7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md`:
- Around line 36-37: The doc must require pre-validation of adapter names before
any shell invocation: update the spec around deploy-scripts/deploy-adapter.sh
and every mention of `<adapter-name>` (including the other referenced sections)
to state that adapter names MUST be validated against a strict allowlist/regex
and passed only as a single positional argument (no string concatenation or
interpolation into shell commands), and add a short recommended validation rule
(e.g., allowed characters and length) and an explicit note to reject or fail CI
on any name that does not match before calling deploy-scripts/deploy-adapter.sh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72375e99-789b-4308-83fa-9eced2c019e3
📒 Files selected for processing (5)
hyperfleet-devtools/.claude-plugin/plugin.jsonhyperfleet-devtools/README.mdhyperfleet-devtools/skills/e2e-test-automation/SKILL.mdhyperfleet-devtools/skills/e2e-test-automation/tier0-automation-logic.mdhyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md
✅ Files skipped from review due to trivial changes (4)
- hyperfleet-devtools/README.md
- hyperfleet-devtools/.claude-plugin/plugin.json
- hyperfleet-devtools/skills/e2e-test-automation/SKILL.md
- hyperfleet-devtools/skills/e2e-test-automation/tier0-automation-logic.md
4996c7a to
b5b69a7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md (1)
34-37:⚠️ Potential issue | 🟠 MajorRequire strict adapter-name validation before any command usage.
The spec still allows adapter names to flow from test-case docs into shell/script patterns without a mandatory validation contract. Please require: strict allowlist regex + max length, pass only as a single positional arg (no interpolation), and fail fast (including CI) on mismatch. This is the root cause; it affects downstream command examples (e.g., Line 141, Line 256, Line 275).
Proposed doc hardening
### 2. Adapter Name from Test Case Document The test case document should specify which adapter to use. Adapter names are NOT hardcoded in the skill. +Before any command/script invocation, adapter names MUST be validated against a strict allowlist +(for example: `^[a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?$`, max 63 chars). +If validation fails, stop generation/execution and fail CI for that test case. +When invoking scripts, pass the validated adapter name only as a single positional argument +(never by shell string concatenation/interpolation).As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md` around lines 34 - 37, Update the "Adapter Name from Test Case Document" handling to enforce strict validation: require adapter names to match a defined allowlist regex (e.g., /^[a-z0-9_-]{1,32}$/) and a clear max-length, accept them only as a single positional argument (no interpolation into other strings or flags), and add fail-fast behavior so invalid names cause immediate CI/test failure; apply these checks to all places that consume adapter names in the doc/scripts/command examples (the adapter name usage in the Adapter Name section and the downstream command examples referenced in the comment) and document the exact regex and max-length contract so callers know the validation rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md`:
- Around line 34-37: Update the "Adapter Name from Test Case Document" handling
to enforce strict validation: require adapter names to match a defined allowlist
regex (e.g., /^[a-z0-9_-]{1,32}$/) and a clear max-length, accept them only as a
single positional argument (no interpolation into other strings or flags), and
add fail-fast behavior so invalid names cause immediate CI/test failure; apply
these checks to all places that consume adapter names in the doc/scripts/command
examples (the adapter name usage in the Adapter Name section and the downstream
command examples referenced in the comment) and document the exact regex and
max-length contract so callers know the validation rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 687cdc89-374e-4082-8682-73b4e945673f
📒 Files selected for processing (5)
hyperfleet-devtools/.claude-plugin/plugin.jsonhyperfleet-devtools/README.mdhyperfleet-devtools/skills/e2e-test-automation/SKILL.mdhyperfleet-devtools/skills/e2e-test-automation/tier0-automation-logic.mdhyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md
✅ Files skipped from review due to trivial changes (3)
- hyperfleet-devtools/.claude-plugin/plugin.json
- hyperfleet-devtools/skills/e2e-test-automation/tier0-automation-logic.md
- hyperfleet-devtools/skills/e2e-test-automation/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- hyperfleet-devtools/README.md
xueli181114
left a comment
There was a problem hiding this comment.
A few additional findings beyond existing comments.
Summary:
- 1 placeholder inconsistency across docs
- 1 deploy method contradiction within same doc
- 1 compatibility ladder violation in example code
- 1 README claim vs reality mismatch
- 1 overly broad grep suggestion
b5b69a7 to
53498ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md (2)
141-141:⚠️ Potential issue | 🟠 MajorUnify deployment guidance: helper API vs bash script is contradictory.
Line 141 and Lines 272-276 prescribe helper-based deployment, but Line 256 switches to bash deployment. Pick one canonical approach (prefer helper path if that is the intended standard) and align all mapping/examples to it.
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 256-256, 272-276
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md` at line 141, The documentation currently mixes two deployment methods; unify to the helper API by replacing the bash deployment examples with the helper call h.DeployAdapter(ctx, deployOpts) everywhere (including the snippet that now shows a bash script) and update any associated expectations (e.g., Expect(err).NotTo(HaveOccurred())) and parameters to match the helper's signature; remove or mark the bash script alternative as deprecated to ensure all mappings/examples consistently demonstrate the canonical helper-based deployment flow.
36-37:⚠️ Potential issue | 🟠 MajorRequire strict adapter-name validation before any shell-based deploy step.
Lines 36-37 and Line 256 allow adapter names sourced from test docs to flow into shell commands without a mandatory validation contract. This should explicitly require allowlist validation (e.g.,
^[a-z0-9]([-a-z0-9]*[a-z0-9])?$+ max length) and fail generation/CI on mismatch before invocation.As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 256-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md` around lines 36 - 37, Ensure any adapter name taken from test docs is validated against an allowlist before being used in shell command invocations: implement a strict validator that enforces the regex ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ and a configured max-length, apply it at the boundary where the "adapter" value is parsed/consumed (i.e., immediately before any shell-based deploy step or command construction that uses the adapter variable), and fail generation/CI with a clear error if the value does not match so no unvalidated adapter can flow into shell invocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md`:
- Around line 118-120: Placeholders in the template use mismatched delimiters
(e.g., "<adapter-name from test case document}" and
"testdata/adapter-configs/<adapter-name}/") which makes copying error-prone; fix
by normalizing the placeholder syntax across the document (choose one style and
apply it consistently) — for example, replace the mixed "<adapter-name from test
case document}" and "testdata/adapter-configs/<adapter-name}/" occurrences (and
the similar occurrence around the later placeholder) so they all use the same
delimiters like <adapter-name> (or alternatively {adapter-name}) throughout.
---
Duplicate comments:
In
`@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md`:
- Line 141: The documentation currently mixes two deployment methods; unify to
the helper API by replacing the bash deployment examples with the helper call
h.DeployAdapter(ctx, deployOpts) everywhere (including the snippet that now
shows a bash script) and update any associated expectations (e.g.,
Expect(err).NotTo(HaveOccurred())) and parameters to match the helper's
signature; remove or mark the bash script alternative as deprecated to ensure
all mappings/examples consistently demonstrate the canonical helper-based
deployment flow.
- Around line 36-37: Ensure any adapter name taken from test docs is validated
against an allowlist before being used in shell command invocations: implement a
strict validator that enforces the regex ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ and a
configured max-length, apply it at the boundary where the "adapter" value is
parsed/consumed (i.e., immediately before any shell-based deploy step or command
construction that uses the adapter variable), and fail generation/CI with a
clear error if the value does not match so no unvalidated adapter can flow into
shell invocations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c91c7602-24f7-423d-918e-3d0e17e7cc6c
📒 Files selected for processing (5)
hyperfleet-devtools/.claude-plugin/plugin.jsonhyperfleet-devtools/README.mdhyperfleet-devtools/skills/e2e-test-automation/SKILL.mdhyperfleet-devtools/skills/e2e-test-automation/tier0-automation-logic.mdhyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md
✅ Files skipped from review due to trivial changes (2)
- hyperfleet-devtools/.claude-plugin/plugin.json
- hyperfleet-devtools/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- hyperfleet-devtools/skills/e2e-test-automation/SKILL.md
53498ce to
5465262
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md (2)
141-149:⚠️ Potential issue | 🟠 MajorUnify deployment guidance (helper API vs raw shell) to avoid contradictory implementation paths.
This file still mixes helper-based lifecycle (
h.DeployAdapter,h.UninstallAdapter) with raw shell-based mapping examples (deploy-adapter.sh), which makes the contract ambiguous for generated tests.Suggested normalization
-| Deploy failure-simulation adapter | Bash: deploy-adapter.sh cl-failure-sim | +| Deploy failure-simulation adapter | h.DeployAdapter(ctx, deployOpts) | @@ -| Verify top-level status reflects failure | GetCluster(), check Ready=False, verify failure message | +| Verify top-level status reflects failure | GetCluster(), check Ready=False, verify failure message |(If shell is intentionally allowed here, state that exception explicitly and consistently in this doc and SKILL.md.)
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 256-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md` around lines 141 - 149, This doc mixes helper-based lifecycle calls (h.DeployAdapter, h.UninstallAdapter, h.CleanupTest<Resource>) with raw shell examples (deploy-adapter.sh) causing ambiguity; pick and document one canonical approach and update references accordingly: either (A) standardize the guide to use the helper API everywhere by replacing shell snippets and examples with h.DeployAdapter/h.UninstallAdapter/h.CleanupTest<Resource> usage and examples, or (B) explicitly state an exception that raw shell scripts are allowed and add a clear section in this doc and SKILL.md describing when/why deploy-adapter.sh is permitted and how it maps to the helper calls; ensure all instances (including the "Teardown (AfterEach)" and verification steps) consistently reference the chosen approach and remove contradictory examples.
34-37:⚠️ Potential issue | 🟠 MajorRequire adapter-name validation before any deploy script invocation.
Adapter names are taken from test-case docs and then used in command-oriented deployment flow, but this spec still doesn’t require strict pre-validation/rejection. That leaves command-injection risk in generated automation paths.
Proposed doc hardening
### 2. Adapter Name from Test Case Document The test case document should specify which adapter to use. Adapter names are NOT hardcoded in the skill. +Before using any adapter name in deployment/undeployment commands, validate it with a strict allowlist rule +(example: `^[a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?$`). +If validation fails, reject input and fail automation/CI immediately. @@ **1. Deploy Test-Specific Adapter (BeforeEach)** -- Use deployment scripts to deploy adapter +- Validate adapter name first, then deploy adapter as a single positional argument (no interpolation) @@ -| Deploy failure-simulation adapter | Bash: deploy-adapter.sh cl-failure-sim | +| Deploy failure-simulation adapter | Validate `cl-failure-sim` then run deploy command with positional arg only |As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 84-87, 256-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md` around lines 34 - 37, The spec currently allows adapter names from test-case documents to flow directly into deployment commands; add strict pre-validation before any deploy script invocation by implementing a whitelist check and rejection path for adapter names referenced in the "Adapter Name from Test Case Document" section and wherever the command-oriented deployment flow consumes test-case docs; ensure the deploy orchestration (the function or routine that builds/executes deployment commands) validates adapter-name against an allowed set, rejects or fails fast with a clear error for unknown/unsafe values, and applies input sanitization/escaping for any remaining runtime use to prevent command-injection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@hyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md`:
- Around line 141-149: This doc mixes helper-based lifecycle calls
(h.DeployAdapter, h.UninstallAdapter, h.CleanupTest<Resource>) with raw shell
examples (deploy-adapter.sh) causing ambiguity; pick and document one canonical
approach and update references accordingly: either (A) standardize the guide to
use the helper API everywhere by replacing shell snippets and examples with
h.DeployAdapter/h.UninstallAdapter/h.CleanupTest<Resource> usage and examples,
or (B) explicitly state an exception that raw shell scripts are allowed and add
a clear section in this doc and SKILL.md describing when/why deploy-adapter.sh
is permitted and how it maps to the helper calls; ensure all instances
(including the "Teardown (AfterEach)" and verification steps) consistently
reference the chosen approach and remove contradictory examples.
- Around line 34-37: The spec currently allows adapter names from test-case
documents to flow directly into deployment commands; add strict pre-validation
before any deploy script invocation by implementing a whitelist check and
rejection path for adapter names referenced in the "Adapter Name from Test Case
Document" section and wherever the command-oriented deployment flow consumes
test-case docs; ensure the deploy orchestration (the function or routine that
builds/executes deployment commands) validates adapter-name against an allowed
set, rejects or fails fast with a clear error for unknown/unsafe values, and
applies input sanitization/escaping for any remaining runtime use to prevent
command-injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21b7874f-551f-42d3-bb66-723bcf3fe20b
📒 Files selected for processing (5)
hyperfleet-devtools/.claude-plugin/plugin.jsonhyperfleet-devtools/README.mdhyperfleet-devtools/skills/e2e-test-automation/SKILL.mdhyperfleet-devtools/skills/e2e-test-automation/tier0-automation-logic.mdhyperfleet-devtools/skills/e2e-test-automation/tier1-tier2-automation-logic.md
✅ Files skipped from review due to trivial changes (2)
- hyperfleet-devtools/.claude-plugin/plugin.json
- hyperfleet-devtools/skills/e2e-test-automation/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- hyperfleet-devtools/README.md
5465262 to
754d2ca
Compare
754d2ca to
bddbdab
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 |
e103714
into
openshift-hyperfleet:main
Summary
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)Summary by CodeRabbit
New Features
Documentation
Chores