NETOBSERV-2388: Migrate QE frontend tests pf-4 (OCP 4.14)#1453
NETOBSERV-2388: Migrate QE frontend tests pf-4 (OCP 4.14)#1453Amoghrd wants to merge 4 commits intonetobserv:main-pf4from
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Amoghrd
changed the title
Amoghrd
changed the title
|
@Amoghrd: This pull request references NETOBSERV-2388 which is a valid jira issue. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (23)
web/cypress/fixtures/netobserv/flowmetrics/without_drops.json-27-220 (1)
27-220:⚠️ Potential issue | 🟠 Major
without_drops.jsoncontains dropped-packet records.
The fixture used for the “Without drops” path includes many entries withPktDropLatestDropCause,PktDropPackets, andPktDropBytes(for example Line 30 onward). This undermines filter-semantic validation and can hide regressions in packet-drop query-option behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/netobserv/flowmetrics/without_drops.json` around lines 27 - 220, The fixture without_drops.json contains records with packet-drop fields (e.g. PktDropLatestDropCause, PktDropPackets, PktDropBytes) inside multiple "values" JSON payloads which violates the “Without drops” test; remove or replace any stream entries whose inner JSON contains those PktDrop* keys so that no record in without_drops.json includes PktDropLatestDropCause, PktDropPackets, PktDropBytes (search for those symbols in the file and update the corresponding "values" entries), and ensure the modified payloads keep other fields intact and syntactically valid JSON.web/cypress/fixtures/netobserv/dns_errors.yaml-22-24 (1)
22-24:⚠️ Potential issue | 🟠 MajorReplace Docker Hub reference with digest-pinned internal mirror image.
Line 22 uses Docker Hub with a mutable tag (
docker.io/massenz/dnsutils:2.4.0), which risks CI failures from rate limiting, registry outages, or image drift. Pin to a digest or use an internal registry mirror.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/netobserv/dns_errors.yaml` around lines 22 - 24, Replace the mutable Docker Hub image reference "image: docker.io/massenz/dnsutils:2.4.0" with a digest-pinned internal-mirror image; update the image line to point to your internal registry (e.g., internal-registry.example.com/massenz/dnsutils@sha256:...) so the fixture in web/cypress/fixtures/netobserv/dns_errors.yaml uses a content-addressable image instead of a tag, preserving the same behavior of the existing command (command: ["/bin/sh", "-ec", "while :; do dig nginx-service.test.svc.cluster.local; sleep 1 ; done"]) while eliminating drift/rate-limit risk.web/cypress/views/netobserv.ts-96-109 (1)
96-109:⚠️ Potential issue | 🟠 MajorRetry loop doesn't actually retry — Cypress commands are queued.
Cypress.$(...)reads the DOM synchronously at the moment the.then()callback runs, butcy.waitandcy.reloadinside the loop are only enqueued; they execute after the callback returns. As a result theifcondition always sees the same initial DOM, so either (a).co-disabledis absent once and the loop exits immediately, or (b) it's present once and you blindly queue 16 waits + 16 reloads regardless of whether access is granted sooner.Use recursion or
cy.shouldwith a retry assertion instead.🔁 Sketch of a working retry
const waitForAccess = (retries = 15) => { cy.get('div.loading-box').should('be.visible') cy.document().then(doc => { if (doc.querySelectorAll('.co-disabled').length && retries > 0) { cy.log(`user does not have access, retries left: ${retries}`) cy.wait(5000) cy.reload(true) waitForAccess(retries - 1) } }) } waitForAccess()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/netobserv.ts` around lines 96 - 109, The retry loop inside the .then() callback doesn't actually retry because Cypress commands are enqueued; replace the synchronous for-loop with a proper Cypress-retrying pattern—either implement a recursive helper (e.g., waitForAccess(retries)) that uses cy.get('div.loading-box').should('be.visible'), then cy.document() or cy.document().then(doc => doc.querySelectorAll('.co-disabled').length) to check access, and when blocked call cy.wait(5000); cy.reload(true); then recurse with retries-1, or use cy.should with a custom assertion that retries until .co-disabled is gone; update the logic that references "div.loading-box" and ".co-disabled" to use cy.document()/cy.get so checks happen during command execution rather than inside a synchronous loop.web/cypress/fixtures/netobserv/testuser-server-client.yaml-32-36 (1)
32-36:⚠️ Potential issue | 🟠 MajorMove
privileged: falsefromcapabilitiestosecurityContextlevel.
capabilitiesonly supportsaddanddropfields per the Kubernetes schema. Theprivilegedfield belongs directly undersecurityContext. Both occurrences—server container (lines 34–36) and client container (lines 95–97)—have this error and will be rejected by strict validators.🛠️ Proposed fix (applies to both containers)
securityContext: allowPrivilegeEscalation: false + privileged: false capabilities: drop: ["ALL"] - privileged: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/netobserv/testuser-server-client.yaml` around lines 32 - 36, The YAML places privileged: false under the capabilities block which is invalid; move privileged: false up one level into the same securityContext as allowPrivilegeEscalation and capabilities. Specifically, remove privileged from the capabilities object and add privileged: false alongside allowPrivilegeEscalation and capabilities in the securityContext for both the server and client containers (i.e., update the securityContext blocks that currently contain capabilities: drop: ["ALL"] to also include privileged: false).web/cypress/fixtures/netobserv/testuser-server-client.yaml-16-20 (1)
16-20:⚠️ Potential issue | 🟠 MajorMove
securityContextfrom Deployment spec to pod spec.Kubernetes Deployment's
specfield does not acceptsecurityContext. This configuration will be silently dropped bykubectl apply, and your pod will run without the intended security constraints.
securityContext(includingrunAsNonRootandseccompProfile) must be placed underspec.template.spec.securityContext, not at the Deployment level.Fix
spec: - securityContext: - runAsNonRoot: true - seccompProfile: - type: RuntimeDefault replicas: 1 selector: matchLabels: app: nginx template: metadata: labels: app: nginx spec: + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault containers:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/netobserv/testuser-server-client.yaml` around lines 16 - 20, The Deployment manifest places securityContext at the top-level spec where it will be ignored; move the securityContext block (including runAsNonRoot and seccompProfile) into the Pod template by adding it under spec.template.spec.securityContext so the settings apply to created Pods; update the YAML where securityContext currently sits to instead nest it under spec.template.spec and verify the keys runAsNonRoot and seccompProfile:type: RuntimeDefault remain unchanged.web/cypress/integration-tests/packet_drop_dashboards.cy.ts-29-32 (1)
29-32:⚠️ Potential issue | 🟠 MajorMalformed CSS selector — unterminated attribute quote.
'[data-surface=true][transform="translate(0, 0) scale(1)]'never closes the"opened aftertransform=. Sizzle treats this as invalid, soCypress.$(...).lengthis always0and the fallback "clear all filters" branch is effectively dead code — the very flake-mitigation the comment describes can never fire.Proposed fix
- if (Cypress.$('[data-surface=true][transform="translate(0, 0) scale(1)]').length > 0) { + if (Cypress.$('[data-surface=true][transform="translate(0, 0) scale(1)"]').length > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/packet_drop_dashboards.cy.ts` around lines 29 - 32, The CSS selector string is malformed (unterminated quote) in the check using Cypress.$ — fix the selector by closing the quoted attribute value so it reads '[data-surface=true][transform="translate(0, 0) scale(1)"]' (or change quoting style so the attribute value is properly delimited), replacing the existing selector literal; update the predicate that checks length so the clear-all-filters branch can actually run.web/cypress/integration-tests/netflow_cluster_admin_group.cy.ts-42-46 (1)
42-46:⚠️ Potential issue | 🟠 MajorAfter hook leaks FlowCollector and may leave stale cluster-role binding.
Setup installs the operator and creates a FlowCollector, but teardown never calls
Operator.deleteFlowCollector()(nor uninstalls the operator). Also, theremove-cluster-role-from-group cluster-adminis performed inline in the secondit— if that test fails before reaching L26, the binding persists across specs. Make teardown idempotent.Proposed fix
after("all tests", function () { + Operator.deleteFlowCollector() + // ensure binding is removed even if the 2nd test failed early + cy.adminCLI(`oc adm policy remove-cluster-role-from-group cluster-admin netobservadmins`) cy.adminCLI(`oc adm groups remove-users netobservadmins ${Cypress.env('LOGIN_USERNAME')}`) cy.adminCLI(`oc delete groups netobservadmins`) cy.uiLogout() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/netflow_cluster_admin_group.cy.ts` around lines 42 - 46, The after hook in netflow_cluster_admin_group.cy.ts must perform idempotent teardown: call the operator cleanup helpers (e.g., invoke Operator.deleteFlowCollector() and any uninstall operator routine that removes CRs and operator resources) and move/remove the inline cluster-admin binding removal into this after block; ensure removal of the cluster-role binding (the "remove-cluster-role-from-group cluster-admin" action) is executed from the after hook using cy.adminCLI and is wrapped to ignore not-found errors so the teardown is idempotent and safe if the test failed earlier, and keep the existing group deletion (oc delete groups netobservadmins) and cy.uiLogout there.web/cypress/integration-tests/dns_tracking.cy.ts-10-13 (1)
10-13:⚠️ Potential issue | 🟠 MajorCreate and clean up deterministic DNS traffic.
The suite enables DNSTracking but never deploys the DNS error generator fixture, so panel/query-summary assertions depend on ambient cluster traffic. The
afterhook also says DNS pods are deleted but none are created or removed.Proposed fix
Operator.install() cy.deployLoki(project, this) Operator.createFlowcollector("DNSTracking") + cy.adminCLI('oc create -f ./fixtures/netobserv/dns_errors.yaml') @@ after("Delete flowcollector and DNS pods", function () { + cy.adminCLI('oc delete -f ./fixtures/netobserv/dns_errors.yaml --ignore-not-found') Operator.deleteFlowCollector() cy.adminCLI(`oc adm policy remove-cluster-role-from-user cluster-admin ${Cypress.env('LOGIN_USERNAME')}`) })Also applies to: 82-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/dns_tracking.cy.ts` around lines 10 - 13, The test enables DNSTracking via Operator.createFlowcollector("DNSTracking") but never deploys the deterministic DNS error generator fixture nor removes it in the after hook, causing flaky assertions; update the spec to deploy the DNS traffic generator (use the existing fixture/deployment helper used elsewhere, e.g., cy.deployDnsErrorGenerator or similar) immediately after Operator.createFlowcollector in the before/setup sequence and add teardown in the after hook to delete the DNS generator pods (match by the generator's deployment/pod label or helper delete function) so the test creates deterministic DNS traffic and cleans up those pods when finished.web/cypress/integration-tests/quickFilters.cy.ts-43-48 (1)
43-48:⚠️ Potential issue | 🟠 MajorWait for the plugin rollout instead of sleeping.
A fixed
cy.wait(10000)can reload before the console plugin has restarted on slower clusters. Gate this on the plugin deployment/pod readiness or an existing helper that waits for the plugin to become available.Proposed direction
- // wait 10 seconds for plugin pod to get restarted - cy.wait(10000).then(() => { - cy.reload() - }) + // Wait for the console plugin rollout/availability here, then reload. + cy.reload()Also applies to: 64-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/quickFilters.cy.ts` around lines 43 - 48, Replace the fixed sleep (cy.wait(10000)) used after applying the patch with an active wait for the console plugin rollout/pod readiness: after invoking cy.adminCLI(`oc patch ...`) (where addQuickFilterPatch is used) call a command that polls the plugin deployment status (e.g., `oc rollout status deployment/<plugin-deployment>` or check pods with `oc get pods -l app=<plugin-label> -o jsonpath='{.items[*].status.conditions[?(@.type=="Ready")].status}'`) or use an existing test helper (e.g., cy.waitForPlugin or cy.waitForConsolePlugin) until it reports ready, then call cy.reload(); replace occurrences of cy.wait(10000) in quickFilters.cy.ts (and the similar block at lines 64-71) with this readiness-based wait and ensure you reference addQuickFilterPatch, cy.adminCLI, and cy.reload when making the change.web/cypress/integration-tests/client_performance.cy.ts-18-24 (1)
18-24:⚠️ Potential issue | 🟠 MajorStart timing and register intercepts before triggering loads.
beforeEachalready visits and waits for the page, then each test starts the timer later. Several intercepts are also registered after the UI action that can fire the request. These timings can pass while measuring cached/already-rendered content instead of load performance.Proposed direction
- cy.intercept('**/backend/api/flow/metrics*').as('call1') - cy.visit('/netflow-traffic') - // wait for all calls to complete - cy.wait('@call1', { timeout: 60000 }) + // Keep per-test intercepts and timers before the visit/tab click that triggers them.Also applies to: 27-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/client_performance.cy.ts` around lines 18 - 24, The test currently calls cy.visit() and starts timing after page load, and some cy.intercept registrations happen after the UI action so measured requests may be cached or already completed; move all cy.intercept(...) registrations and the timing start to before the triggering action (i.e., inside beforeEach before cy.visit('/netflow-traffic')), ensure you start the performance timer before the page load and then use cy.wait('@call1', { timeout: 60000 }) after visit to capture real load requests; update any additional intercepts referenced in the file (the ones noted between lines 27-91) similarly so they are registered prior to the action that fires them.web/cypress/integration-tests/table_queryopts.cy.ts-25-44 (1)
25-44:⚠️ Potential issue | 🟠 MajorRegister and assert intercepts before changing the query option.
The intercepts are added after
cy.changeQueryOption()andwaitForLokiQuery(), so they can miss the requests. They are also never waited on, so this test does not verify the limit-specific calls.Proposed fix pattern
- cy.changeQueryOption('500') - netflowPage.waitForLokiQuery() cy.intercept('GET', getTableLimitURL('500'), { fixture: 'netobserv/flowmetrics/table_500.json' - }).as('matchedUrl') + }).as('limit500') + cy.changeQueryOption('500') + cy.wait('@limit500') - cy.changeQueryOption('100') - netflowPage.waitForLokiQuery() cy.intercept('GET', getTableLimitURL('100'), { fixture: 'netobserv/flowmetrics/table_100.json' - }).as('matchedUrl') + }).as('limit100') + cy.changeQueryOption('100') + cy.wait('@limit100') - cy.changeQueryOption('50') - netflowPage.waitForLokiQuery() cy.intercept('GET', getTableLimitURL('50'), { fixture: 'netobserv/flowmetrics/table_50.json' - }).as('matchedUrl') + }).as('limit50') + cy.changeQueryOption('50') + cy.wait('@limit50')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/table_queryopts.cy.ts` around lines 25 - 44, Register the network intercepts before invoking cy.changeQueryOption so the requests are caught: call cy.intercept('GET', getTableLimitURL('500'), { fixture: 'netobserv/flowmetrics/table_500.json' }).as('table500') (and similarly for '100' and '50') prior to calling cy.changeQueryOption('500') / cy.changeQueryOption('100') / cy.changeQueryOption('50'); then after each change use netflowPage.waitForLokiQuery() and assert the request occurred by waiting on the corresponding alias with cy.wait('@table500') (and '@table100', '@table50') to ensure the limit-specific calls are verified.web/cypress/integration-tests/topology_edges_labels.cy.ts-41-47 (1)
41-47:⚠️ Potential issue | 🟠 MajorRegister the
ownersintercept before selecting the owners group.Line 41 can trigger the
groups=ownersrequest inbeforeEach, but the intercept is added later in the test. That makes the fixture non-deterministic or unused.Proposed direction
- topologyPage.selectScopeGroup("resource", "owners") + // Select the scope in the test after registering any required intercepts.it("(OCP-53591, memodi, Network_Observability) should verify group owners", function () { cy.intercept(getTopologyResourceScopeGroupURL('owners'), { fixture: 'netobserv/flowmetrics/Owners.json' }) + topologyPage.selectScopeGroup("resource", "owners") cy.get(topologySelectors.nGroups).its('length').should('be.gte', 5) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/topology_edges_labels.cy.ts` around lines 41 - 47, The test registers the cy.intercept for getTopologyResourceScopeGroupURL('owners') after topologyPage.selectScopeGroup("resource", "owners") which can cause the groups=owners request (triggered in beforeEach or by selectScopeGroup) to miss the stub; move the cy.intercept(getTopologyResourceScopeGroupURL('owners'), { fixture: 'netobserv/flowmetrics/Owners.json' }) so it runs before calling topologyPage.selectScopeGroup("resource", "owners") (or before any action that may trigger the owners request), ensuring the intercept for the owners fixture is in place when topologySelectors.nGroups is asserted.web/cypress/integration-tests/topology_groups.cy.ts-72-79 (1)
72-79:⚠️ Potential issue | 🟠 MajorRemove the framework-specific selector in favor of the stable app-owned ID.
.pf-v5-c-progress-stepperis a PatternFly implementation detail that can change. Use the existing#scope-step-2ID directly, which is already defined and used consistently in the test framework.Proposed fix
- cy.get('.pf-v5-c-progress-stepper').get('#scope-step-2 > div:nth-child(2) > button').click().then(slider => { + cy.get('#scope-step-2 > div:nth-child(2) > button').click().then(slider => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/topology_groups.cy.ts` around lines 72 - 79, The test uses a fragile PatternFly class selector ('.pf-v5-c-progress-stepper') — replace it with the app-owned stable ID '#scope-step-2' when selecting and clicking the slider button; specifically update the cy.get chain in topology_groups.cy.ts to target the '#scope-step-2' container (e.g. use cy.get('#scope-step-2').find('div:nth-child(2) > button').click()) so the click still triggers netflowPage.waitForLokiQuery() and the subsequent assertion on '#lastRefresh' remains unchanged.web/cypress/integration-tests/topology_groups.cy.ts-23-33 (1)
23-33:⚠️ Potential issue | 🟠 MajorMove the conditional filter cleanup into a Cypress callback.
The synchronous
Cypress.$()check on line 29 executes immediately, before the queuedcy.visit()andcy.get().click()commands complete. This causes the DOM inspection to run against a stale state, potentially missing filters that need clearing.Wrap the conditional in
cy.get('body').then()to ensure the DOM check runs after page load and navigation complete:Proposed fix
- if (Cypress.$('[data-surface=true][transform="translate(0, 0) scale(1)]').length > 0) { - cy.log("need to clear all filters") - cy.get('[data-test="filters"] > [data-test="clear-all-filters-button"]').should('exist').click() - } + cy.get('body').then(($body) => { + if ($body.find('[data-surface=true][transform="translate(0, 0) scale(1)]').length > 0) { + cy.log("need to clear all filters") + cy.get('[data-test="filters"] > [data-test="clear-all-filters-button"]').should('exist').click() + } + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/topology_groups.cy.ts` around lines 23 - 33, The synchronous DOM check using Cypress.$('[data-surface=true][transform="translate(0, 0) scale(1)]') in the beforeEach after netflowPage.visit() is running before the visit/click commands complete; wrap that conditional inside a Cypress callback such as cy.get('body').then(() => { ... }) so the DOM inspection executes after page load/navigation, then inside the callback perform the same existence check and call cy.get('[data-test="filters"] > [data-test="clear-all-filters-button"]').should('exist').click() when needed; update the beforeEach (function, netflowPage.visit, the data-surface selector and clear-all-filters-button call) to use this pattern so the filter cleanup is reliably executed.web/cypress/integration-tests/topology_edges_labels.cy.ts-19-29 (1)
19-29:⚠️ Potential issue | 🟠 MajorMove the DOM check into the Cypress chain.
The synchronous
Cypress.$()call evaluates before the queuedvisit()and click commands run, so it inspects stale DOM and skips needed filter cleanup.Proposed fix
- if (Cypress.$('[data-surface=true][transform="translate(0, 0) scale(1)]').length > 0) { - cy.log("need to clear all filters") - cy.get('[data-test="filters"] > [data-test="clear-all-filters-button"]').should('exist').click() - } + cy.get('body').then(($body) => { + if ($body.find('[data-surface=true][transform="translate(0, 0) scale(1)]').length > 0) { + cy.log("need to clear all filters") + cy.get('[data-test="filters"] > [data-test="clear-all-filters-button"]').should('exist').click() + } + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/topology_edges_labels.cy.ts` around lines 19 - 29, The DOM existence check using synchronous Cypress.$ is racing with the queued netflowPage.visit and click; replace it by moving the selector into the Cypress command chain (use cy.get('[data-surface=true][transform="translate(0, 0) scale(1)]') and a .then callback) so the check runs after visit() and the click, and inside that .then inspect $els.length and, if > 0, call cy.get('[data-test="filters"] > [data-test="clear-all-filters-button"]').should('exist').click(); update the beforeEach block (function beforeEach, netflowPage.visit, and the clear-all-filters-button usage) accordingly so no synchronous Cypress.$ is used.web/cypress/integration-tests/topology_view.cy.ts-75-77 (1)
75-77:⚠️ Potential issue | 🟠 MajorTruncate dropdown item is never clicked.
cy.byTestID('truncate-dropdown').click().byTestID("25")locates the option but never invokes.click()on it, so the value may not actually be applied. Line 77 applies.click()on the scope option but not here — the assertion at line 97 (truncateLength === 25) will only pass if something else (e.g., hover selection) triggers persistence.🔧 Proposed fix
- cy.byTestID('truncate-dropdown').click().byTestID("25") + cy.byTestID('truncate-dropdown').click().byTestID("25").click()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/topology_view.cy.ts` around lines 75 - 77, The test clicks the truncate-dropdown but never clicks the "25" option (cy.byTestID('truncate-dropdown').click().byTestID("25") only finds it), so update the selection to actually click the option: after opening the dropdown (cy.byTestID('truncate-dropdown').click()), explicitly click the option identified by byTestID("25") (e.g., cy.byTestID('25').click() or chain .byTestID('25').click()), ensuring the truncate option is applied before assertions that check truncateLength === 25; reference the truncate-dropdown selection and the byTestID("25") option in your change.web/cypress/integration-tests/overview_page.cy.ts-91-105 (1)
91-105:⚠️ Potential issue | 🟠 MajorComment says "uncheck" but code calls
.check()again — focus toggle is never disabled.Line 91 enables
#focus-switch; Line 102 is commented as "uncheck single focus toggle" but calls.check(), which is idempotent on an already-checked input. The subsequentcheckPanel/checkPanelsNumtherefore runs with focus-mode still on, contrary to intent.🔧 Proposed fix
// uncheck single focus toggle and verify panels - cy.get('#focus-switch').check() + cy.get('#focus-switch').uncheck() cy.checkPanel(overviewSelectors.defaultPanels) cy.checkPanelsNum();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/overview_page.cy.ts` around lines 91 - 105, The test mistakenly re-checks the focus toggle instead of disabling it: replace the second cy.get('#focus-switch').check() with a call that disables the toggle (e.g., cy.get('#focus-switch').uncheck() or an equivalent click) so focus mode is turned off before invoking cy.checkPanel(overviewSelectors.defaultPanels) and cy.checkPanelsNum(); ensure the element selector '#focus-switch' is used and the subsequent assertions call cy.checkPanel and cy.checkPanelsNum while focus is disabled.web/cypress/integration-tests/netflow_table.cy.ts-186-196 (1)
186-196:⚠️ Potential issue | 🟠 Major
eachiterator variable is unused — assertion doesn't iterate per-row.Inside
each((td) => { ... })the callback ignorestdand issues a freshcy.get('[data-test-td-column-id=SrcPort] > div > div'), which re-queries the whole table and asserts on the default (first) match. So both loops (Lines 186-188 and 194-196) effectively assert on the same single cell, not every row.🔧 Proposed fix
- cy.get('[data-test-td-column-id=SrcPort]').each((td) => { - cy.get('[data-test-td-column-id=SrcPort] > div > div').should('not.contain.text', 'loki (3100)') - }) + cy.get('[data-test-td-column-id=SrcPort]').each((td) => { + cy.wrap(td).find('div > div').should('not.contain.text', 'loki (3100)') + })(apply the symmetric fix at Lines 194-196.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/netflow_table.cy.ts` around lines 186 - 196, The each callback is ignoring the iterator variable `td` and re-querying the whole column, so replace the inner `cy.get('[data-test-td-column-id=SrcPort] > div > div')` with assertions scoped to the current element using `td` (e.g., wrap or find on `td`: use `cy.wrap(td).find('div > div').should(...)` or `cy.wrap(td).should('not.contain.text', ...)`) in the first loop, and apply the symmetric change to the second loop that asserts `contain.text 'loki (3100)'` so each row is checked individually.web/cypress/views/netflow-page.ts-318-341 (1)
318-341:⚠️ Potential issue | 🟠 MajorMove the warning-state parsing inside the Cypress callback.
warningExistsis set in a queued.then(), but Lines 329-338 run before that callback executes. The warning branch is effectively unreachable.Proposed fix
Cypress.Commands.add('checkQuerySummary', (metric) => { - let warningExists = false - let num = 0 - let metricStr: string - - cy.get(querySumSelectors.queryStatsPanel).should('exist').then(() => { - if (Cypress.$(querySumSelectors.queryStatsPanel + ' svg.query-summary-warning').length > 0) { - warningExists = true - } - }) - - if (warningExists) { - metricStr = metric.text().split('+ ')[0] - if (metricStr.includes('k')) { - num = Number(metricStr.split('k')[0]) - } else { - num = Number(metricStr) - } - } else { - num = Number(metric.text().split(' ')[0]) - } - - expect(num).to.be.greaterThan(0) + cy.get(querySumSelectors.queryStatsPanel).should('exist').then(($panel) => { + const warningExists = $panel.find('svg.query-summary-warning').length > 0 + const metricText = metric.text() + const metricStr = warningExists ? metricText.split('+ ')[0] : metricText.split(' ')[0] + const num = metricStr.includes('k') + ? Number(metricStr.split('k')[0]) + : Number(metricStr) + + expect(num).to.be.greaterThan(0) + }) });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/netflow-page.ts` around lines 318 - 341, In the Cypress.Commands.add('checkQuerySummary') implementation the warning detection (warningExists) is done inside a queued .then() but the metric parsing and expect() run synchronously after, causing the warning branch to never execute; move the metric parsing and the expect assertion inside the same cy.get(...).then(...) callback (or chain another .then) so the DOM check for querySumSelectors.queryStatsPanel and the Cypress.$(querySumSelectors.queryStatsPanel + ' svg.query-summary-warning') inspection happen before you read metric.text(), then compute num (handling 'k' suffix) and call expect(num).to.be.greaterThan(0) within that callback, keeping all references to the existing command name ('checkQuerySummary'), querySumSelectors.queryStatsPanel and the 'svg.query-summary-warning' selector.web/cypress/views/netflow-page.ts-130-143 (1)
130-143:⚠️ Potential issue | 🟠 MajorReplace PF5 class selectors with PF4-compatible alternatives.
Lines 131, 138, and 139 use
pf-v5-*selectors, but this code targets PF4. These classes don't exist in the PF4 DOM and will cause test failures.
- Line 131 (
next):.pf-v5-c-wizard__footer→ Use PF4 wizard footer selector (e.g.,.pf-c-wizard__footeror stable data-test attribute)- Line 138 (
update):button.pf-v5-c-button.pf-m-primary→ Use.pf-c-button.pf-m-primaryor data-test selector if available- Line 139 (
privilegedToggle): Replacepf-v5-l-flex,pf-v5-c-switch,pf-v5-c-switch__togglewith PF4 equivalents (e.g.,.pf-c-switch__toggle)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/netflow-page.ts` around lines 130 - 143, pluginSelectors is using PF5 class names that don't exist in the PF4 DOM which breaks tests; update the selectors for next, update, and privilegedToggle to PF4-compatible classes or stable data-test attributes: replace pluginSelectors.next (currently using .pf-v5-c-wizard__footer) with the PF4 wizard footer selector like .pf-c-wizard__footer or a data-test attr, replace pluginSelectors.update (button.pf-v5-c-button.pf-m-primary) with .pf-c-button.pf-m-primary or a data-test selector, and replace pluginSelectors.privilegedToggle (pf-v5-l-flex / pf-v5-c-switch / pf-v5-c-switch__toggle) with PF4 switch classes such as .pf-c-switch and .pf-c-switch__toggle or the equivalent data-test attribute. Ensure each replacement targets the same element and keep selectors stable (prefer data-test where available).web/cypress/integration-tests/networkpolicy_form.cy.ts-9-14 (1)
9-14:⚠️ Potential issue | 🟠 MajorMake test namespaces unique and cleanup-owned by hooks.
Hard-coded projects like
test0/test4can collide with existing namespaces, and the unconditionalaftercleanup would then delete them. Use a per-run prefix, deriveoperatingProject, and register every created namespace for hook cleanup.Proposed direction
-const projects: string[] = ['test0', 'test1', 'test2'] +const projectPrefix = `np-form-${Date.now()}-${Math.floor(Math.random() * 10000)}` +const projects: string[] = [0, 1, 2].map((i) => `${projectPrefix}-${i}`) ... -const operatingProject = 'test0' +const operatingProject = projects[0]- const projectName = 'test0' + const projectName = operatingProject- const newProject = 'test4' + const newProject = `${projectPrefix}-empty`Also applies to: 52-55, 93-98, 114-119, 131-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/networkpolicy_form.cy.ts` around lines 9 - 14, The tests use hard-coded namespace names (projects array and operatingProject) which can collide with existing namespaces and be deleted by the unconditional after hook; change to generate per-run unique names (e.g., prefix + random/suffix) for each entry in projects and podLabels, derive operatingProject from the generated projects array instead of a hard-coded string, and ensure every namespace you create is recorded in a shared list that the after/afterEach cleanup hook uses to only delete owned namespaces; update references to pod_label_key, ns_label_key, and fixtureFile usages accordingly so they reference the generated names rather than static values.web/cypress/integration-tests/networkpolicy_form.cy.ts-16-16 (1)
16-16:⚠️ Potential issue | 🟠 MajorRemove the top-level
.skip.This disables every test in the new spec, so the PR adds no executable coverage in CI.
Proposed fix
-describe.skip('(OCP-41858, OCP-45303) Console Network Policies form tests', function () { +describe('(OCP-41858, OCP-45303) Console Network Policies form tests', function () {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/networkpolicy_form.cy.ts` at line 16, The top-level test suite is disabled by using describe.skip; change the test suite declaration from describe.skip('(OCP-41858, OCP-45303) Console Network Policies form tests', ...) to describe('(OCP-41858, OCP-45303) Console Network Policies form tests', ...) so the spec runs in CI, and scan for any other top-level .skip usages in this file (e.g., other describe.skip or it.skip) and remove them if you want the tests to execute.web/cypress/integration-tests/networkpolicy_form.cy.ts-34-44 (1)
34-44:⚠️ Potential issue | 🟠 MajorReplace Cypress alias with imported fixture for cross-hook access.
cy.fixture(fixtureFile).as('testData')at line 34 is unreliable by the nestedbeforehook at line 150. Cypress resets aliases before each test; the 3 tests in the intervening "UI validating tests" describe block will reset it, leavingthis.testDataundefined when the nestedbeforeruns and tries to access it at line 153. SincetestFixtureis already imported at line 5, use it directly instead:- cy.fixture(fixtureFile).as('testData') cy.exec(`rm ${tmpFile}`);- this.testData.items.filter(item => (item.kind == 'ReplicationController')).forEach((item) => { labels_npods.set(item.spec.template.metadata.labels.name, item.spec.replicas) }) + testFixture.items + .filter(item => item.kind === 'ReplicationController') + .forEach(item => labels_npods.set(item.spec.template.metadata.labels.name, item.spec.replicas))Also verify
podsPageUtils.setProjectPodNamesAlias()andpodsPageUtils.setPodIPAlias()at lines 42 and 163 don't rely on.as()for state that needs to persist across tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/networkpolicy_form.cy.ts` around lines 34 - 44, Replace the fragile Cypress alias usage by removing cy.fixture(fixtureFile).as('testData') and instead reference the already-imported testFixture directly where the code expects persistent fixture data (e.g., in the nested before hook and any helpers); update calls to podsPageUtils.setProjectPodNamesAlias(project, label, aliasPrefix) and podsPageUtils.setPodIPAlias(...) to accept and use testFixture (or read from the imported module) rather than relying on Cypress aliases so the data persists across describe/before boundaries; ensure any helper functions that previously accessed this.testData are updated to take a fixture argument (or import testFixture) and use it consistently.
🟡 Minor comments (12)
web/cypress/fixtures/netobserv/flowmetrics/fully_dropped.json-12-30 (1)
12-30:⚠️ Potential issue | 🟡 Minor
totalEntriesReturnedis inconsistent with fixture payload.
Line 29 setstotalEntriesReturnedto0, but Lines 12-17 include one returned record. This can produce misleading Query Summary behavior in tests.Suggested fix
- "totalEntriesReturned": 0 + "totalEntriesReturned": 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/netobserv/flowmetrics/fully_dropped.json` around lines 12 - 30, The stats.summary.totalEntriesReturned value is wrong for this fixture—update the "totalEntriesReturned" field under "stats.summary" to match the actual number of records in the payload (the single record in the "values" array), i.e., set totalEntriesReturned to 1 so Query Summary tests reflect the fixture's data.web/cypress/views/netobserv.ts-252-282 (1)
252-282:⚠️ Potential issue | 🟡 MinorFragile Loki readiness check — will often skip tests on slow clusters.
Two concerns:
- The 15s
cy.waitafter applying the Loki manifests is almost certainly not enough time for the Loki pod to reachRunningon a fresh cluster (image pull + PVC bind can easily exceed that). The comment acknowledges this. Usecy.adminCLI('oc wait --for=condition=Ready pod/loki ...')or acy.shouldretry on the pod status instead of a fixed sleep.cy.byTestID('resource-status').invoke('text')is evaluated once — if the status is stillPending/ContainerCreating, the test skips unnecessarily. Wrap the check in a retried assertion so it polls up to a generous timeout before declaring failure.🛠️ Suggested direction
cy.adminCLI( `oc wait --for=condition=Ready pod/loki -n ${namespace} --timeout=180s`, { failOnNonZeroExit: false } ).then(r => { if (r.code !== 0) { cy.log('Loki not ready, skipping') context.skip() } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/netobserv.ts` around lines 252 - 282, Replace the fragile fixed sleep and one-time status read with a proper readiness wait and a retried UI check: after applying the manifests with cy.adminCLI (the block that uses lokiPodCmd and cy.adminCLI to apply 1-storage.yaml/2-loki.yaml and currently calls cy.wait(15000)), call oc wait via cy.adminCLI to wait for pod/loki Ready with a generous timeout (e.g., 180s) and short-circuit to context.skip() if oc wait fails; then keep podsPage.goToPodDetails(namespace, "loki") but change the cy.byTestID('resource-status').invoke('text') single-read into a retried assertion (cy.byTestID('resource-status').should(...) or equivalent) that polls until the text equals 'Running' within the same generous timeout and only then set lokiStatusCheck true, otherwise log and skip via context.skip().web/cypress/integration-tests/flow_dashboards_packets.cy.ts-32-32 (1)
32-32:⚠️ Potential issue | 🟡 MinorDuplicate test case ID with bytes dashboard spec.
This test uses
OCP-63790, which is also used byflow_dashboards_bytes.cy.tsfor a different assertion (bytes metrics vs packets metrics). Polarion/test-case IDs should be unique so results map to the right case. Double-check and assign a distinct ID for the packets variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/flow_dashboards_packets.cy.ts` at line 32, The test in flow_dashboards_packets.cy.ts uses the same Polarion ID OCP-63790 as the bytes test; update the `it` description to use a distinct test-case ID (e.g., OCP-63791 or another unique ID) so it no longer duplicates `flow_dashboards_bytes.cy.ts`; change the string in the `it("(OCP-63790, memodi, Network_Observability), should have flow dashboards for packets metrics", ...)` call to the new unique ID and confirm the new ID is reflected in any related test metadata or test-run mappings.web/cypress/integration-tests/Readme.md-5-8 (1)
5-8:⚠️ Potential issue | 🟡 MinorTypos and unit mismatch in guidelines.
- Line 5: "preferrably" → "preferably"; "less than 3mins but not more 3:30 seconds" mixes units — presumably "not more than 3:30 minutes".
- Line 8: "preferrably" → "preferably".
- Line 18: "deffault" → "default".
✏️ Proposed fix
-- Have each spec file execution time as low as possible, preferrably less than 3mins but not more 3:30 seconds. +- Have each spec file execution time as low as possible, preferably less than 3 minutes but no more than 3:30. ... -- Have each spec files with fewer tests, preferrably <= 4 `It` blocks. +- Have each spec file with fewer tests, preferably <= 4 `it` blocks. ... -- [flowRTT.cy.ts](flowRTT.cy.ts) `Verify flowRTT panels` can be flaky because deffault flowRTT panels may not be rendered the first time overview page is visited by the tests +- [flowRTT.cy.ts](flowRTT.cy.ts) `Verify flowRTT panels` can be flaky because default flowRTT panels may not be rendered the first time the overview page is visited by the tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/Readme.md` around lines 5 - 8, Fix typos and unit wording in the Readme bullets: change "preferrably" to "preferably" in both occurrences, reword "less than 3mins but not more 3:30 seconds" to "less than 3 minutes and not more than 3:30 minutes" (or "not more than 3 minutes 30 seconds"), correct "deffault" to "default", and adjust pluralization ("Have each spec files" → "Have each spec file") so the guidelines are grammatically consistent; update the lines in the Readme.md where these bullets appear.web/cypress/fixtures/netobserv/flowcollector/fc_packetDrop.yaml-19-27 (1)
19-27:⚠️ Potential issue | 🟡 MinorAdd
workload_egress_packets_totalfor consistency with other metric fixtures.The fixture includes ingress and egress counters for both node and namespace scopes, but only ingress for workload scope. This pattern inconsistency exists across similar fixtures like
fc_packetsMetrics.yaml,fc_bytesMetrics.yaml, andfc_networkalert.yaml, which all include both workload ingress and egress variants.🛠️ Proposed fix
- workload_ingress_packets_total + - workload_egress_packets_total - namespace_drop_packets_total🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/netobserv/flowcollector/fc_packetDrop.yaml` around lines 19 - 27, The includeList in fc_packetDrop.yaml is missing the workload_egress_packets_total metric which breaks consistency with other fixtures; update the includeList array to add "workload_egress_packets_total" alongside the existing workload_ingress_packets_total (keeping ordering consistent with node/namespace/workload ingress/egress patterns) so that the metrics set matches fc_packetsMetrics.yaml, fc_bytesMetrics.yaml, and fc_networkalert.yaml.web/cypress/integration-tests/packet_drop_dashboards.cy.ts-63-63 (1)
63-63:⚠️ Potential issue | 🟡 MinorCopy/paste comment — should say "Dropped Packets".
Line 61 just switched the metric to
PktDropPackets.- // verify Query Summary stats for Dropped Bytes metric + // verify Query Summary stats for Dropped Packets metric🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/packet_drop_dashboards.cy.ts` at line 63, The inline comment "// verify Query Summary stats for Dropped Bytes metric" is stale after switching the metric to PktDropPackets; update that comment to say "Dropped Packets" (or similar) to match the metric change where PktDropPackets is used so the comment and the test assertion referring to PktDropPackets are consistent.web/cypress/integration-tests/netflow_conversations.cy.ts-42-52 (1)
42-52:⚠️ Potential issue | 🟡 MinorWeak assertion + no-op wait.
cy.wait(10)does nothing useful, andexpect(nflows).to.be.gte(0)succeeds on an empty/zero result — i.e., the test passes even when conversation tracking produced no flows. Other specs here usegreaterThan(0)viacy.checkQuerySummary. Consider the same here (and drop the 10ms wait).- cy.wait(10) - expect(nflows).to.be.gte(0) + expect(nflows).to.be.greaterThan(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/netflow_conversations.cy.ts` around lines 42 - 52, The test uses a no-op cy.wait(10) and a weak assertion that allows zero results; remove the cy.wait(10) call and strengthen the assertion on the parsed nflows value (from ConversationsCnt) to require > 0 (e.g., use .to.be.greaterThan(0) or reuse cy.checkQuerySummary behavior) so the spec fails when no flows are produced; update the block that reads querySumSelectors.flowsCount / ConversationsCnt / nflows accordingly.web/cypress/integration-tests/flowRTT_dashboards.cy.ts-66-68 (1)
66-68:⚠️ Potential issue | 🟡 Minor
/\d* ms/matches' ms'with zero digits — too permissive.
\d*matches zero or more digits, so a label that lost its number entirely would still satisfy the assertion. Use\d+to actually validate an RTT value is present.🔧 Proposed fix
- cy.get('[data-test-id=edge-handler]').each((g) => { - expect(g.text()).to.match(/\d* ms/gm); - }); + cy.get('[data-test-id=edge-handler]').each((g) => { + expect(g.text()).to.match(/\d+ ms/); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/flowRTT_dashboards.cy.ts` around lines 66 - 68, The test assertion using the regex /\d* ms/ is too permissive because \d* allows zero digits; update the assertion in the cy.get('[data-test-id=edge-handler]').each callback to require at least one digit (use \d+), e.g. change the regex from /\d* ms/gm to /\d+ ms/ (or /\d+\s*ms/ if whitespace variance is expected) so RTT labels without a numeric value fail the test.web/cypress/integration-tests/netflow_table.cy.ts-191-191 (1)
191-191:⚠️ Potential issue | 🟡 Minor
not.have.classargument should not include the leading..Chai-jQuery's
have.classmatcher takes a class name, not a selector.'.disabled-value'with a dot will never match any element's class list, so the assertion is a no-op (always passes). Also the positive assertion on Line 178 uses'disabled-group', so the string here is likely wrong too.🔧 Proposed fix
- cy.get('#filters > .pf-c-toolbar__item > :nth-child(1)').should('not.have.class', '.disabled-value') + cy.get('#filters > .pf-c-toolbar__item > :nth-child(1)').should('not.have.class', 'disabled-group')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/netflow_table.cy.ts` at line 191, The assertion uses Chai-jQuery's have.class matcher incorrectly by passing a selector string with a leading dot and the wrong class name; update the assertion on the cy.get(...) call so the class argument is the plain class name (no leading '.') and matches the positive assertion used elsewhere (use 'disabled-group' not '.disabled-value'), i.e., change the should('not.have.class', '.disabled-value') call to should('not.have.class', 'disabled-group').web/cypress/fixtures/netobserv/test-server-client.yaml-76-79 (1)
76-79:⚠️ Potential issue | 🟡 MinorShell redirection order silences stdout but not stderr.
curl ... 2>&1 > /dev/nulldups stderr to the current stdout (still the terminal), then redirects stdout to/dev/null. Result: stderr keeps printing. To discard both streams use> /dev/null 2>&1.🔧 Proposed fix
- \ curl nginx-service.${SERVER_NS}.svc:80/data/100K 2>&1 > /dev/null ; sleep 5 \n + \ curl nginx-service.${SERVER_NS}.svc:80/data/100K > /dev/null 2>&1 ; sleep 5 \n🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/netobserv/test-server-client.yaml` around lines 76 - 79, The shell redirection in the loop string (the command containing "curl nginx-service.${SERVER_NS}.svc:80/data/100K 2>&1 > /dev/null") duplicates stderr to the old stdout before stdout is redirected, so stderr still prints; change the order to redirect stdout first then stderr (use "> /dev/null 2>&1") inside the quoted loop body so both stdout and stderr from the curl call are discarded.web/cypress/integration-tests/netflow_export.cy.ts-44-57 (1)
44-57:⚠️ Potential issue | 🟡 MinorCSV rename via
lsstdout is fragile and leaks across tests.Two concrete issues:
response.stdoutfromls cypress/downloadsincludes a trailing newline and lists every file in the directory. If a prior test leaves any file behind (e.g. a previous run wherermdidn't execute because areadFiletimed out), this interpolates multiple newline-separated names into themvcommand and it fails.- The
rmon Line 55 is inside the outer.then, so ifreadFilethrows the file is never cleaned up — next run starts dirty.Prefer globbing to the specific CSV and cleaning up in
afterEach:🔧 Suggested approach
- cy.get(exportSelectors.exportButton).should('exist').then((exportbtn) => { - cy.wrap(exportbtn).click() - // wait for download to complete - cy.wait(3000) - // get the CSV file name - cy.exec("ls cypress/downloads").then((response) => { - // rename CSV file to export_table.csv - cy.wrap(response.stdout).should('not.be.empty') - cy.exec(`mv cypress/downloads/${response.stdout} cypress/downloads/export_table.csv`) - cy.readFile('cypress/downloads/export_table.csv') - }) - cy.exec('rm cypress/downloads/export_table.csv') - }) + cy.get(exportSelectors.exportButton).should('exist').click() + cy.exec('ls cypress/downloads/*.csv | head -n1').then((response) => { + const csvPath = response.stdout.trim() + expect(csvPath).to.match(/\.csv$/) + cy.readFile(csvPath, { timeout: 15000 }).should('not.be.empty') + cy.exec(`rm -f ${csvPath}`) + })Also consider a global
afterEachthat wipescypress/downloads/to prevent cross-test pollution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/integration-tests/netflow_export.cy.ts` around lines 44 - 57, The export test currently relies on raw ls stdout and does cleanup inside a then branch which is fragile; change the mv/selection to target only CSVs (use a glob like cypress/downloads/*.csv via cy.exec or a helper that returns the first CSV filename and trim it) instead of interpolating full ls stdout, ensure you pick a single filename (e.g., first match) and pass that to the move/rename step, and move/remove cleanup out of the inner .then into a global afterEach (or an always-run cleanup step) so exportSelectors.exportButton click -> cy.exec/readFile steps still run but the downloaded file is always removed in afterEach; update references to the current cy.exec/readFile handling and the test’s cleanup to use this robust CSV-globbing and guaranteed afterEach removal.web/cypress/views/netflow-page.ts-72-75 (1)
72-75:⚠️ Potential issue | 🟡 MinorUse consistent selector approach for the more-options button.
Line 73 uses
#chips-more-options-button(id selector), while line 67 usescy.byTestID('chips-more-options-button')(data-test selector). If the element only has a data-test attribute, the id selector will fail and breakclearAllFilters().Proposed fix
clearAllFilters: () => { - cy.get('#chips-more-options-button').should('exist').click().then(() => { + cy.byTestID('chips-more-options-button').should('exist').click().then(() => { cy.contains("Clear all").should('exist').click() }) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/netflow-page.ts` around lines 72 - 75, In clearAllFilters(), the more-options button is selected with an id selector '#chips-more-options-button' which is inconsistent with and may fail against the data-test selector used elsewhere; update the selector to use cy.byTestID('chips-more-options-button') everywhere in clearAllFilters() (replace cy.get('#chips-more-options-button') with cy.byTestID('chips-more-options-button')) and keep the existing should('exist').click().then(...) and subsequent cy.contains("Clear all").should('exist').click() logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4eba5f32-947e-4fcc-9db1-80926d1aca93
📒 Files selected for processing (68)
web/cypress/fixtures/netobserv/dns_errors.yamlweb/cypress/fixtures/netobserv/flowcollector/fc.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_DNSTracking.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_bytesMetrics.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_conversations.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_flowRTT.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_lokiDisabled.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_networkalert.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_packetDrop.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_packetsMetrics.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_subnetLabel.yamlweb/cypress/fixtures/netobserv/flowcollector/fc_zoneMulticluster.yamlweb/cypress/fixtures/netobserv/flowmetrics/NS.jsonweb/cypress/fixtures/netobserv/flowmetrics/NSOwners.jsonweb/cypress/fixtures/netobserv/flowmetrics/Owners.jsonweb/cypress/fixtures/netobserv/flowmetrics/cluster.jsonweb/cypress/fixtures/netobserv/flowmetrics/containing_drops.jsonweb/cypress/fixtures/netobserv/flowmetrics/fully_dropped.jsonweb/cypress/fixtures/netobserv/flowmetrics/hosts.jsonweb/cypress/fixtures/netobserv/flowmetrics/hostsNS.jsonweb/cypress/fixtures/netobserv/flowmetrics/hostsOwners.jsonweb/cypress/fixtures/netobserv/flowmetrics/namespace.jsonweb/cypress/fixtures/netobserv/flowmetrics/owner.jsonweb/cypress/fixtures/netobserv/flowmetrics/resource.jsonweb/cypress/fixtures/netobserv/flowmetrics/table_100.jsonweb/cypress/fixtures/netobserv/flowmetrics/table_50.jsonweb/cypress/fixtures/netobserv/flowmetrics/table_500.jsonweb/cypress/fixtures/netobserv/flowmetrics/without_drops.jsonweb/cypress/fixtures/netobserv/flowmetrics/zone.jsonweb/cypress/fixtures/netobserv/image-digest-mirror-set.yamlweb/cypress/fixtures/netobserv/perf/flow_metrics_perf.jsonweb/cypress/fixtures/netobserv/perf/netflow_table_perf.jsonweb/cypress/fixtures/netobserv/perf/overview_perf_app.jsonweb/cypress/fixtures/netobserv/perf/overview_perf_ns.jsonweb/cypress/fixtures/netobserv/prom-role-roleBinding.yamlweb/cypress/fixtures/netobserv/test-pod.yamlweb/cypress/fixtures/netobserv/test-server-client.yamlweb/cypress/fixtures/netobserv/testuser-server-client.yamlweb/cypress/integration-tests/Readme.mdweb/cypress/integration-tests/client_performance.cy.tsweb/cypress/integration-tests/dns_dashboards.cy.tsweb/cypress/integration-tests/dns_tracking.cy.tsweb/cypress/integration-tests/flowRTT.cy.tsweb/cypress/integration-tests/flowRTT_dashboards.cy.tsweb/cypress/integration-tests/flow_dashboards_bytes.cy.tsweb/cypress/integration-tests/flow_dashboards_packets.cy.tsweb/cypress/integration-tests/health_dashboards.cy.tsweb/cypress/integration-tests/netflow_cluster_admin_group.cy.tsweb/cypress/integration-tests/netflow_conversations.cy.tsweb/cypress/integration-tests/netflow_developer_view.cy.tsweb/cypress/integration-tests/netflow_export.cy.tsweb/cypress/integration-tests/netflow_external_subnet.cy.tsweb/cypress/integration-tests/netflow_table.cy.tsweb/cypress/integration-tests/netflow_zone_multiCluster.cy.tsweb/cypress/integration-tests/networkpolicy_form.cy.tsweb/cypress/integration-tests/overview_page.cy.tsweb/cypress/integration-tests/packet_drop.cy.tsweb/cypress/integration-tests/packet_drop_dashboards.cy.tsweb/cypress/integration-tests/prom_datasource_only.cy.tsweb/cypress/integration-tests/quickFilters.cy.tsweb/cypress/integration-tests/static_plugin.cy.tsweb/cypress/integration-tests/table_queryopts.cy.tsweb/cypress/integration-tests/topology_edges_labels.cy.tsweb/cypress/integration-tests/topology_groups.cy.tsweb/cypress/integration-tests/topology_view.cy.tsweb/cypress/integration-tests/workloads.cy.tsweb/cypress/views/netflow-page.tsweb/cypress/views/netobserv.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main-pf4 #1453 +/- ##
=========================================
Coverage 57.43% 57.43%
=========================================
Files 196 196
Lines 9576 9576
Branches 1187 1187
=========================================
Hits 5500 5500
Misses 3712 3712
Partials 364 364
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Amoghrd
force-pushed
the
cypress-tests-pf4
branch
5 times, most recently
from
60a559f to
aab8bae
Compare
…P 4.14/PF4 This commit migrates all frontend Cypress tests from openshift-tests-private release-4.14 branch and fixes them for OpenShift 4.14/PatternFly 4 compatibility. Key changes: - Migrated 24 integration test files from openshift-tests-private - Fixed PF4 UI element selectors (display dropdowns, filters, exports) - Updated fixture directory structure and paths - Added TypeScript configuration for Cypress - Implemented helper functions and view page objects - Fixed filter interface for PF4 (column-filter-toggle, group toggles) - Corrected display dropdown selectors to use CSS class selectors - Fixed export functionality across all views - Updated column selectors to use correct casing - Added proper wait conditions for view options toolbar - Replaced nth-child tab selectors with text-based navigation - Added data-test attributes to histogram controls - Removed staticPlugin test - Added checkStorageClass validation in test hooks Test coverage includes: - DNS tracking and dashboards - Flow RTT tracking and dashboards - Packet drop detection and dashboards - Network traffic overview, table, and topology views - Conversations, zones, and multi-cluster scenarios - Export functionality - Quick filters and query options - Developer view and workloads - Performance testing - Health dashboards Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Amoghrd
force-pushed
the
cypress-tests-pf4
branch
from
ec11187 to
6f3d425
Compare
Changed from resetClearFilters() to clearAllFilters() since the "Clear all filters" button is available in PF4, but "Reset filters" button is only conditionally enabled. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
/ok-to-test |
openshift-ci
Bot
added
the
ok-to-test
|
New image: quay.io/netobserv/network-observability-console-plugin:930ddcc
It will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=930ddcc make set-plugin-image |
- topology_groups: Use dropdown instead of slider for owner scope selection - e2e.ts: Import netflow-page to register custom Cypress commands globally - quickFilters: Add clearAllFilters() call after first test Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
github-actions
Bot
removed
the
ok-to-test
- Fix retry loop in cypress/views/netobserv.ts to properly check for user access - Replace synchronous for-loop with recursive retry pattern - Ensures DOM checks happen during Cypress command execution - Fix security context in testuser-server-client.yaml - Move pod-level securityContext from Deployment spec to template.spec - Move privileged field from capabilities to securityContext level - Clean without_drops.json fixture to remove packet drop records - Remove 5 stream objects containing PktDrop fields - Ensures fixture correctly represents flows without drops - Fix topology_view test to use cy.uiLogin instead of cy.login Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@Amoghrd: The following test failed, say
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. |
2 participants
Summary
Migrates QE frontend Cypress tests from
openshift-tests-privaterelease-4.14 branch to support PatternFly 4 compatibility for OCP ≤4.14.Test Results
Failing tests passed on rerun
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Tests