Skip to content

HYPERFLEET-1183 - test: Updating e2e tests to remove double coverage …#129

Open
ma-hill wants to merge 2 commits into
openshift-hyperfleet:mainfrom
ma-hill:HYPERFLEET-1183-remove-dups
Open

HYPERFLEET-1183 - test: Updating e2e tests to remove double coverage …#129
ma-hill wants to merge 2 commits into
openshift-hyperfleet:mainfrom
ma-hill:HYPERFLEET-1183-remove-dups

Conversation

@ma-hill

@ma-hill ma-hill commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes duplicate test coverage for version and channel filtering and uniqueness constraints. The tests in hyperfleet-api cover the cases that the filtering.go and uniqueness.go tests were doing, making these tests redundant. Also since the Versions and Channels don't have adapters, the e2e tests are now thinner and only have a simple CRUD lifecycle for versions and channels. The additional testing is at the API layer.

HYPERFLEET-1183

Changes

  • Removed e2e/version/filtering.go (110 lines) — filtering by spec.is_default and spec.enabled is already validated in the CRUD lifecycle test's PATCH/LIST verification
  • Removed e2e/version/uniqueness.go (100 lines) — duplicate name constraints are enforced by the API and covered by integration tests
  • Added PATCH label test to e2e/version/crud_lifecycle.go that validates label updates and filters the result via LIST with a label selector
  • Added PATCH label test to e2e/channel/crud_lifecycle.go with the same verification pattern
  • Promoted both CRUD lifecycle tests from labels.Tier1 to labels.Tier0 since they now cover the baseline CRUD + filtering path

Notes

In conjunction with the change here: openshift-hyperfleet/hyperfleet-api#225

The removed tests were originally written to validate spec-level filtering and uniqueness constraints in isolation, but these behaviors are implicitly tested whenever PATCH and LIST operations succeed in the CRUD tests. The new PATCH label assertions explicitly verify that filtering works as expected, making the standalone files unnecessary.

Test Plan

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • E2E tests verify PATCH operations and filtering behavior

@openshift-ci openshift-ci Bot requested review from crizzo71 and rafabene June 18, 2026 16:54
@openshift-ci

openshift-ci Bot commented Jun 18, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign crizzo71 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: d10c1595-c26f-4971-9dc7-0188fecf37d6

📥 Commits

Reviewing files that changed from the base of the PR and between 099e767 and 3a65626.

📒 Files selected for processing (2)
  • e2e/channel/crud_lifecycle.go
  • e2e/version/crud_lifecycle.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift-hyperfleet/architecture (manual)
  • openshift-hyperfleet/hyperfleet-api (manual)
  • openshift-hyperfleet/hyperfleet-sentinel (manual)
  • openshift-hyperfleet/hyperfleet-adapter (manual)
  • openshift-hyperfleet/hyperfleet-broker (manual)
🚧 Files skipped from review as they are similar to previous changes (1)
  • e2e/channel/crud_lifecycle.go

📝 Walkthrough

Summary by CodeRabbit

  • Tests
    • Added test cases verifying label updates and generation increments for channels and versions
    • Updated test classification levels for channel and version lifecycle tests
    • Removed test suites for version filtering and uniqueness validation

Walkthrough

Channel and version CRUD lifecycle e2e suites are reclassified from labels.Tier1 to labels.Tier0. Each suite gains a new ginkgo.It block that issues a PATCH to update resource labels (crud=<resourceID>), asserts the generation field increments to 2, then calls the corresponding List API with a label selector and verifies exactly one result with the expected resource ID. Concurrently, e2e/version/filtering.go (110 lines, three-version ListVersions filter tests) and e2e/version/uniqueness.go (100 lines, cross-channel uniqueness and 409-conflict tests) are deleted outright.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is incomplete/truncated ('…' indicates more text) and does not clearly communicate the main change—removing duplicate e2e test coverage. Expand the title to something like 'Remove duplicate e2e tests for version filtering and uniqueness' to clearly convey the primary change.
✅ Passed checks (10 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the rationale for removing redundant tests, outlines specific changes, and justifies the tier promotion for CRUD lifecycle tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed No tokens, passwords, credentials, or secrets found in log output. All logging via GinkgoWriter.Printf outputs only safe resource IDs and names.
No Hardcoded Secrets ✅ Passed No hardcoded secrets, API keys, tokens, passwords, or sensitive credentials detected in the modified test files or removed code.
No Weak Cryptography ✅ Passed No weak cryptography patterns detected. Comprehensive audit of 64 Go files found no usage of banned primitives (crypto/md5, crypto/des, crypto/rc4, SHA1 for security), custom crypto implementations...
No Injection Vectors ✅ Passed String concatenation in test files (lines 101, 112) uses server-generated IDs; ListResources() applies url.QueryEscape() to search parameters, preventing injection. No CWE-89, CWE-78, CWE-79, CWE-5...
No Privileged Containers ✅ Passed PR modifies only Go test files (e2e/channel/crud_lifecycle.go, e2e/version/crud_lifecycle.go, removes e2e/version/filtering.go and e2e/version/uniqueness.go). No Kubernetes manifests, Helm template...
No Pii Or Sensitive Data In Logs ✅ Passed All logging statements use test-only ginkgo.GinkgoWriter and log only auto-generated test resource IDs and field names, not PII, session IDs, credentials, or sensitive data.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/channel/crud_lifecycle.go`:
- Around line 90-105: The test has a race condition where the static label value
"channel-lifecycle" used in both the PatchChannel call and the ListChannels
label selector can conflict with parallel test runs, causing the HaveLen(1)
assertion on line 103 to fail nondeterministically. Replace the hardcoded label
value "channel-lifecycle" with a unique, test-specific identifier (such as one
generated using a UUID or timestamp) in both the labels map passed to
PatchChannel and in the label selector query passed to ListChannels. This
ensures each test run uses isolated label values that won't interfere with
parallel executions.
🪄 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: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 0ae38443-e613-405b-99ac-60036d224c95

📥 Commits

Reviewing files that changed from the base of the PR and between 7839d03 and 099e767.

📒 Files selected for processing (4)
  • e2e/channel/crud_lifecycle.go
  • e2e/version/crud_lifecycle.go
  • e2e/version/filtering.go
  • e2e/version/uniqueness.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift-hyperfleet/architecture (manual)
  • openshift-hyperfleet/hyperfleet-api (manual)
  • openshift-hyperfleet/hyperfleet-sentinel (manual)
  • openshift-hyperfleet/hyperfleet-adapter (manual)
  • openshift-hyperfleet/hyperfleet-broker (manual)
💤 Files with no reviewable changes (2)
  • e2e/version/filtering.go
  • e2e/version/uniqueness.go

Comment thread e2e/channel/crud_lifecycle.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant