Add Cursor e2e test generation rules#106
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sayak-redhat The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
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:
WalkthroughAdds a contributor-facing E2E test rulebook (.cursor/rules/e2e-rules.md) that prescribes write-scope restrictions, workflows to derive tests from PRs/Jira, coverage deduplication, test-case templates, Ginkgo/Go conventions, pre-commit scope checks, and generation guardrails. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/rules/e2e-rules.md:
- Line 299: Several fenced code blocks in the markdown use plain triple
backticks with no language specified; update each instance of a code fence that
currently looks like ``` to include an appropriate language tag (e.g., ```go or
```text) so markdownlint MD040 is satisfied—search for code fences with no
language in the document and replace them with the correct language for those
code snippets.
- Around line 30-43: The doc contains contradictory scope rules: the rule "ONLY
create or modify files inside `test/`" conflicts with later clauses that permit
writing/committing `output/` (e.g. the sentence "The generated `test-cases.md`
is written to a **local** output directory (e.g. `output/<key>/`)"). Update the
first rule to explicitly permit generated output directories by replacing "ONLY
create or modify files inside `test/`" with a scoped rule that allows
writing/committing generated files under `output/<key>/` (or add a clear
exception sentence), and ensure the three occurrences of the output-exemption
text are consistent (the "generated `test-cases.md`" line and the two other
`output/` exemption lines) so the document unambiguously states that only test
source edits are allowed except for generated output written to `output/<key>/`.
- Around line 299-328: The duration constants at the bottom (DefaultInterval,
ShortInterval, DefaultTimeout, ShortTimeout, TestContextTimeout) use shorthand
literals like "10s" and "5m" which are not valid Go duration syntax; update each
to the canonical Go form using the time package (e.g., DefaultInterval = 10 *
time.Second, DefaultTimeout = 5 * time.Minute, etc.), ensure the file imports or
references time where needed and that the constant names (DefaultInterval,
ShortInterval, DefaultTimeout, ShortTimeout, TestContextTimeout) exactly match
the equivalents in test/e2e/utils/constants.go.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 22063276-69e0-4206-8793-a842e38dc095
📒 Files selected for processing (1)
.cursor/rules/e2e-rules.md
…e format - Refine Step 6 with 10 impact categories (Core Functionality, Config Edge Cases, Dynamic Behavior, Integration Gaps, Multi-tenant, Error Handling, Upgrade/Compatibility, Performance, Security, Real Customer Scenarios) - Add prioritization rules (Critical / High / Medium) - Add structured test case ID format: <TICKET>-TC-NNN - Enhance Step 7 template with Coverage Gap field and per-step Expected results - Add "Writing good test steps" guidance for concrete, actionable test plans Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e_test.go`:
- Around line 341-368: The test assertions in the e2e test contradict the
production SCC generated by generateSpireAgentSCC; update the expectations in
the It block that reads the scc variable (and references SpireAgentSCCName) to
match production: set Expect(scc.AllowHostNetwork).To(BeTrue()),
Expect(scc.AllowHostPorts).To(BeTrue()),
Expect(scc.AllowPrivilegedContainer).To(BeTrue()), expect
AllowPrivilegeEscalation to be non-nil and true (adjust the pointer assertion
accordingly), and change
Expect(scc.RunAsUser.Type).To(Equal(securityv1.RunAsUserStrategyMustRunAsRange));
alternatively, if you intend to change production behavior instead, leave the
test and open a follow-up to modify generateSpireAgentSCC to align with the
current assertions.
- Around line 370-427: The test's expectations for the spire-agent DaemonSet
(variables ds, podSpec, agentContainer and container SecurityContext sc)
contradict production settings; update the assertions to match
pkg/controller/spire-agent/daemonset.go (HostNetwork should be true, DNSPolicy
should equal corev1.DNSClusterFirstWithHostNet, container sc.Privileged should
be true, sc.AllowPrivilegeEscalation and sc.ReadOnlyRootFilesystem may be
nil/unset, and sc.Capabilities.Drop should not be asserted to contain "ALL"), or
alternatively move this spec check into a separate test that documents the
required privileged configuration for SPIRE Agent's node attestation
(referencing utils.SpireAgentDaemonSetName and the "spire-agent" container name)
and adjust the Expect(...) calls accordingly.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d1f9edf-4147-4e3c-b29f-0beffc05e2a0
📒 Files selected for processing (5)
.cursor/rules/e2e-rules.mdoutput/pr-105/test-cases.mdtest/e2e/e2e_suite_test.gotest/e2e/e2e_test.gotest/e2e/utils/constants.go
✅ Files skipped from review due to trivial changes (1)
- output/pr-105/test-cases.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .cursor/rules/e2e-rules.md
Clarify that test/ and output/ are two separate top-level directories, not output/ being inside test/. Addresses CodeRabbit review comment about contradictory scope rules. Made-with: Cursor
Made-with: Cursor
|
@sayak-redhat: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What it does:
What it doesn't do:
Summary by CodeRabbit