ROX-35420: Decouple OCP plugin tests from CVE data - #21633
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
Skipping CI for Draft Pull Request. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #21633 +/- ##
==========================================
- Coverage 50.38% 50.34% -0.04%
==========================================
Files 2846 2846
Lines 218347 218351 +4
==========================================
- Hits 110007 109928 -79
- Misses 100356 100420 +64
- Partials 7984 8003 +19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughA new Cypress route matcher helper for GraphQL operations and a JSON fixture for image resources are added. The cveDetail and imageDetail Cypress test suites are refactored to intercept and assert on GraphQL requests/responses using route matcher maps instead of relying on extracted DOM text. ChangesGraphQL Interception Migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test as Cypress Test
participant CypressRuntime as Cypress
participant GraphQLAPI as GraphQL Backend
Test->>CypressRuntime: define route matcher map (getOcpRouteMatcherMapForGraphQL)
Test->>CypressRuntime: interceptAndWatchRequests / interactAndWaitForResponses
CypressRuntime->>GraphQLAPI: POST GraphQL operation (e.g. getImagesForCVE, getImageResources)
GraphQLAPI-->>CypressRuntime: response with acsAuthNamespaceHeader
CypressRuntime-->>Test: intercepted request/response data
Test->>Test: assert header value and table column visibility
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ui/apps/platform/cypress/integration-ocp/security/cveDetail.test.ts (2)
74-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated interception-unwrapping boilerplate.
const request = Array.isArray(interception) ? interception[0] : interception;is duplicated 4 times across this file andimageDetail.test.ts. Consider extracting a small helper (e.g., inhelpers/request.js) to unwrap a single interception.Also applies to: 97-104
🤖 Prompt for 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. In `@ui/apps/platform/cypress/integration-ocp/security/cveDetail.test.ts` around lines 74 - 79, The interception-unwrapping logic is duplicated in the Cypress tests, so extract a small reusable helper to normalize the result from waitForRequests into a single interception object. Add the helper in a shared location such as helpers/request.js, then update cveDetail.test.ts and imageDetail.test.ts to use it instead of repeating Array.isArray(interception) ? interception[0] : interception in each test block.
8-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShared route maps/fixtures/helper duplicated verbatim across test files.
cveListRouteMap,cveListFixtures, andvisitVulnerabilitiesPage(Lines 15-46) are byte-for-byte identical to the definitions inimageDetail.test.ts(Lines 14-46). Sinceroutes.tsalready centralizes route-matcher logic, extracting these shared constants/helper into it (or a shared spec helper) would avoid drift when GraphQL operation names change.🤖 Prompt for 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. In `@ui/apps/platform/cypress/integration-ocp/security/cveDetail.test.ts` around lines 8 - 46, The shared cveListRouteMap, cveListFixtures, and visitVulnerabilitiesPage definitions in cveDetail.test.ts are duplicated verbatim elsewhere, so move these shared test helpers/constants into a common location such as routes.ts or a shared spec helper and import them where needed. Reuse the existing getOcpRouteMatcherMapForGraphQL and interceptRequests-based setup so only one copy of the GraphQL operation names and fixture mappings remains to prevent drift across cveDetail.test.ts and imageDetail.test.ts.
🤖 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.
Nitpick comments:
In `@ui/apps/platform/cypress/integration-ocp/security/cveDetail.test.ts`:
- Around line 74-79: The interception-unwrapping logic is duplicated in the
Cypress tests, so extract a small reusable helper to normalize the result from
waitForRequests into a single interception object. Add the helper in a shared
location such as helpers/request.js, then update cveDetail.test.ts and
imageDetail.test.ts to use it instead of repeating Array.isArray(interception) ?
interception[0] : interception in each test block.
- Around line 8-46: The shared cveListRouteMap, cveListFixtures, and
visitVulnerabilitiesPage definitions in cveDetail.test.ts are duplicated
verbatim elsewhere, so move these shared test helpers/constants into a common
location such as routes.ts or a shared spec helper and import them where needed.
Reuse the existing getOcpRouteMatcherMapForGraphQL and interceptRequests-based
setup so only one copy of the GraphQL operation names and fixture mappings
remains to prevent drift across cveDetail.test.ts and imageDetail.test.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 19f95442-7879-4ec2-8e08-77aff320286f
📒 Files selected for processing (4)
ui/apps/platform/cypress/fixtures/vulnerabilities/workloadCves/getImageResources.jsonui/apps/platform/cypress/integration-ocp/routes.tsui/apps/platform/cypress/integration-ocp/security/cveDetail.test.tsui/apps/platform/cypress/integration-ocp/security/imageDetail.test.ts
🚀 Build Images ReadyImages are ready for commit f03b532. To use with deploy scripts: export MAIN_IMAGE_TAG=4.12.x-478-gf03b532130 |
386a961 to
d18d56b
Compare
|
/test ocp-4-21-ui-e2e-tests |
d18d56b to
864fc7c
Compare
|
/test ocp-4-21-ui-e2e-tests |
alwayshooin
left a comment
alwayshooin
left a comment
There was a problem hiding this comment.
lgtm just a few questions in the comments
dvail
added
backport release-4.10
backport release-4.11
2 participants
Description
Removes reliance on actual Scanner data for OCP e2e tests. This is in response to increased flakes due to not all data being available by the time the tests run. More concretely - tests are failing due to looking for CVEs in
stackroxnamespace, when only one or two images have been scanned and have visible results.Since none of the tests are checking behavior on actual CVE data, instead we mock the responses with fixtures like we recently did for the standalone app tests.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Faith in CI