Skip to content

NETOBSERV-2388 QE Frontend tests migration#1317

Open
memodi wants to merge 3 commits intonetobserv:mainfrom
memodi:qe-tests-migration
Open

NETOBSERV-2388 QE Frontend tests migration#1317
memodi wants to merge 3 commits intonetobserv:mainfrom
memodi:qe-tests-migration

Conversation

@memodi
Copy link
Member

@memodi memodi commented Mar 6, 2026

Description

NETOBSERV-2388 Migrating QE's frontend e2e tests to here

# Export required environment variables
export KUBECONFIG=/path/to/kubeconfig
export CYPRESS_LOGIN_USERS="kubeadmin:your-password"
export CYPRESS_LOGIN_IDP="kube:admin"

# Run tests
./scripts/run-e2e-tests.sh

https://gist.github.com/memodi/afc650d3d36e76e0de51c578cf835d02

Dependencies

n/a

Checklist

  • Does the changes in PR need specific configuration or environment set up for testing?
    • if so please describe it in PR description.
  • I have added thorough unit tests for the change.
  • QE requirements (check 1 from the list):
    • Standard QE validation, with pre-merge tests unless stated otherwise.
    • Regression tests only (e.g. refactoring with no user-facing change).
    • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

@openshift-ci
Copy link

openshift-ci bot commented Mar 6, 2026

[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 mffiedler 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

@memodi
Copy link
Member Author

memodi commented Mar 9, 2026

/cc @Amoghrd @leandroberetta

@openshift-ci openshift-ci bot requested review from Amoghrd and leandroberetta March 9, 2026 20:32
## Tests naming
- All tests must have all lower case in their naming to maintain alphabetical order of tests execution.

## Flaky tests
Copy link
Member

Choose a reason for hiding this comment

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

I dont think this section is valid anymore? More of these tests should be less flaky now

export const conversationID = '[data-test=th-_HashId] > .pf-v5-c-table__button'
export const flowRTT = '[data-test=th-TimeFlowRttMs] > .pf-v5-c-table__button'
export const dscp = '[data-test=th-Dscp] > .pf-v5-c-table__button'
export const DNSLatency = '[data-test=th-DNSLatency] > .pf-v5-c-table__column-help > .pf-v5-c-table__button'
Copy link
Member

Choose a reason for hiding this comment

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

These vars need to start with lower-case. Its not part of this PR scope, but will be good to have it fixed

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated for DNS* var names, however I'd like to contain only changes that are related to this migration work in this PR.

export const doubleLeftShift = backwardShift + "> div:nth-child(1) > button"
}

Cypress.Commands.add('showAdvancedOptions', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I think some of the commands added here were taken from the console repo. So there might be repetition and can be removed

Copy link
Member Author

Choose a reason for hiding this comment

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

any idea which commands are repeated? for e.g.: I only see showAdvancedOptions defined here in this file, no where else.

Copy link
Member

Choose a reason for hiding this comment

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

I see repeated funcs in here

Copy link
Member Author

Choose a reason for hiding this comment

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

pushed commit to remove duplicate cypress commands, I have kept checkPopupItems and changeQueryOption since those have different implementations.

@Amoghrd
Copy link
Member

Amoghrd commented Mar 10, 2026

@memodi thanks! Couple of questions:

  • How are we going to handle different selectors in older OCP versions?
  • Just to confirm if the answer to the first question is that these tests are run only on latest OCP versions, then are we going to still use openshift-tests-private repo for running the tests?
  • There are about 6 failing tests in the gist pasted above. AFAIK all tests should be passing so is there some issue in the test or is it due to migration?
  • Are all the funcs ported over in the views files used? Just wanted to confirm if the whole file was ported over or only the funcs used by NetObserv

@memodi
Copy link
Member Author

memodi commented Mar 10, 2026

@Amoghrd:

  • How are we going to handle different selectors in older OCP versions?

We'll need to branches based on OCP versions here in web-console repo.

  • Just to confirm if the answer to the first question is that these tests are run only on latest OCP versions, then are we going to still use openshift-tests-private repo for running the tests?

No, once this is merged, I'll create branches and make adjustments to tests/libs for older version in their PRs.

  • There are about 6 failing tests in the gist pasted above. AFAIK all tests should be passing so is there some issue in the test or is it due to migration?

I expect some flakes with UI tests, but that shouldn't block the migration. I'll continue to fix the tests during this PR and even after.

  • Are all the funcs ported over in the views files used? Just wanted to confirm if the whole file was ported over or only the funcs used by NetObserv

I've tried to minimize the code which we're not using from openshift-tests-private, but it's possible some extra stuff may have trickled in since functions could call other functions in the same file.

Copy link
Member Author

Choose a reason for hiding this comment

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

this file needs to be removed from PR.

@Amoghrd
Copy link
Member

Amoghrd commented Mar 10, 2026

One other question I forgot to ask, will this script replace the qe-e2e-console-tests check on PRs once its merged?

@memodi
Copy link
Member Author

memodi commented Mar 10, 2026

One other question I forgot to ask, will this script replace the qe-e2e-console-tests check on PRs once its merged?

Not immediately, but eventually all the CI will need updating and new pipelines will need to be created.

Comment on lines +13 to +14
/web/src/**/*.js
/web/src/**/*.spec.js
Copy link
Member

Choose a reason for hiding this comment

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

why excluding js sources ? Although we probably don't have any, no js file should be generated in the web/src , or is it not correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have lot of js files locally from past build commands I had ran, don't you get those js artifacts when building locally?

Copy link
Member

Choose a reason for hiding this comment

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

weird, no, I never have that ... Maybe old stuff ?

"esModuleInterop": true,
"resolveJsonModule": true,
"baseUrl": ".",
"types": [
Copy link
Member

Choose a reason for hiding this comment

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

This would restrict a lot the dependency types available, is that something really needed?
Also the exclusion list below reduces the type checking coverage.
If a different tsconfig is needed for e2e tests we can perhaps create another one, rather than changing the main .one

Copy link
Member Author

Choose a reason for hiding this comment

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

done, there's tsconfig.json for e2e tests I removed these from here.

@memodi memodi force-pushed the qe-tests-migration branch from 9404ea3 to de1ef81 Compare March 10, 2026 19:28
@openshift-ci
Copy link

openshift-ci bot commented Mar 10, 2026

@memodi: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/qe-e2e-console-tests f14b03f link false /test qe-e2e-console-tests
ci/prow/images f14b03f link true /test images
ci/prow/plugin-cypress f14b03f link true /test plugin-cypress

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

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.

3 participants