Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jwa(front): Add UI tests with Cypress #6891

Merged
merged 3 commits into from Jan 16, 2023

Conversation

orfeas-k
Copy link
Contributor

This PR adds more UI tests with Cypress for functionalities of the JWA frontend. More specifically, they check that:

  • Index page renders every Notebook name into the table
  • Index page shows correct Status icon for all notebooks

Everything implementation-specific (mock API, node version etc) mentioned in the VWA UI tests PR stand for this one too.

Comment on lines 113 to 120
- name: Serve UI & run Cypress tests in Firefox
uses: cypress-io/github-action@v4.2.0
with:
working-directory: components/crud-web-apps/jupyter/frontend
start: npm run serve
install: false
browser: firefox
wait-on: "http://localhost:4200"
Copy link
Member

Choose a reason for hiding this comment

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

@orfeas-k since the setup for running the Firefox tests is exactly the same with the one for Chrome, what about moving this step into the one that runs the Chrome steps?

This way we avoid the duplicate work of running the tests twice

Copy link
Member

Choose a reason for hiding this comment

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

The downside of merging the two actions would be that if in the future we don't mock the whole backend, then we'll need to break these actions again.

But I don't think we are going to move towards this direction for at least the next 2 months

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I have no strong preference over this so I 'm sending a fixup!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just pushed a fixup for this @kimwnasptd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see the following error here https://github.com/kubeflow/kubeflow/actions/runs/3911407976/jobs/6684764715

[3094:0113/125035.603611:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported.

My guess is that this has to do with Cypress github action. We could always skip using cypress-io/github-action@v4.2.0 and do sth like this:

run: |
npm run serve &
npm run ui-test-ci-all 

ui-test-ci-all runs tests both in Chrome and Firefox. I think the above paired with a command in order to wait on localhost:4200 would be more than enough.
cc @kimwnasptd

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 it's going to be a little bit involved to wait on localhost:4200 to be ready to serve index.html.

Let's see if we can get something to work. Else if it's too much trouble let's revert back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decided to use wait-on as suggested by Cypress in order to wait for localhost:4200. https://docs.cypress.io/guides/continuous-integration/introduction#Solutions

@orfeas-k orfeas-k force-pushed the feature-orfeas-jwa-ui-tests branch 2 times, most recently from 91ae59b to 6c7c712 Compare January 13, 2023 15:32
Orfeas Kourkakis added 3 commits January 13, 2023 17:38
 - Upgrade Cypress to version ^10.10.0
 - Add integration tests with Cypress to check that:
   * Index page renders every Notebook name into the table
   * Index page shows correct Status icon for all notebooks

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
@kimwnasptd
Copy link
Member

Nice!

/lgtm
/approve

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kimwnasptd

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

The pull request process is described here

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

@google-oss-prow google-oss-prow bot merged commit 6774b10 into kubeflow:master Jan 16, 2023
@orfeas-k orfeas-k deleted the feature-orfeas-jwa-ui-tests branch February 15, 2023 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants