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

creating first visual test for login page #12922

Closed
wants to merge 12 commits into from

Conversation

a-arias
Copy link

@a-arias a-arias commented Dec 19, 2024

Summary

This is a POC of an implementation of visual testing with percy and cypress.

Occurred changes and/or fixed issues

This is for login test page only.

Technical notes summary

This is a POC using percy for visual testing

Areas or cases that should be tested

-No areas
-Used chrome for local testing
-Run the test using the command: npm run cy:run:percy

Areas which could experience regressions

Login page

Screenshot/Video

No

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@a-arias a-arias requested a review from rancher-max January 7, 2025 23:14
@izaac
Copy link
Contributor

izaac commented Jan 8, 2025

As our sorry cypress gets older, could this Percy Browserstack service work along their Automate service ? To run both functional and visual. It could be worth to take a look for the migration from Sorry.

loginPage.canSubmit().then((canSubmit) => {
if (canSubmit) {
// Take a snapshot for visual diffing
cy.percySnapshot('Login test');
Copy link
Contributor

Choose a reason for hiding this comment

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

Using an external service like this could trivialize the screenshots storage for diff, and the reporting looks great too. Also it looks really straightforward to take the snapshots.

@cnotv
Copy link
Member

cnotv commented Jan 9, 2025

As our sorry cypress gets older, could this Percy Browserstack service work along their Automate service ? To run both functional and visual. It could be worth to take a look for the migration from Sorry.

To debug issues styles related, to wait 15 minutes would mean increasing development time by a tremendous amount and hard to address to a specific component.

Also the service Percy is limited to 5k screenshots, where every single push to a PR will trigger like at least 100 times, if we consider each existing test.
If you add the fact that we have a lot of flaky tests, this would increase the amount even further.

As discussed today in the planning, this is the original issue which I'm working now since November. On top of that I've been investigating visual testing for unit tests which seems not currently achievable.

@izaac
Copy link
Contributor

izaac commented Jan 15, 2025

@cnotv I still can't tell where those 15 minutes you've mention are coming from. Could you elaborate?

@cnotv
Copy link
Member

cnotv commented Jan 15, 2025

@cnotv I still can't tell where those 15 minutes you've mention are coming from. Could you elaborate?

It seems like there's more cases and reach 20 minutes sometimes, however I just checked the Test pipeline: https://github.com/rancher/dashboard/actions/runs/12786815636

Screenshot 2025-01-15 at 18 08 34

@izaac
Copy link
Contributor

izaac commented Jan 15, 2025

That's a standard run. And this is only executing in the login test. I don't see why this has anything to do with those numbers.

@cnotv
Copy link
Member

cnotv commented Jan 15, 2025

That's a standard run. And this is only executing in the login test. I don't see why this has anything to do with those numbers.

If you want to use E2E tests to generate screenshots then you should consider E2E timing. I am not aware of what is the final plan, so I just gave opinion based on what I see.

@izaac
Copy link
Contributor

izaac commented Jan 15, 2025

@cnotv you may be right, but I still can't tell how's that relevant here. But you could participate and call it out. Or open an issue to collect those timings and come with a more accurate number.

@cnotv
Copy link
Member

cnotv commented Jan 15, 2025

@cnotv you may be right, but I still can't tell how's that relevant here. But you could participate and call it out. Or open an issue to collect those timings and come with a more accurate number.

Sorry, but for what should I open the issue exactly?
Also I am already working on this issue since months, so not so sure what do you mean.

@izaac
Copy link
Contributor

izaac commented Jan 15, 2025

@cnotv that's ok. I think we circled back on to how you've come up with those extra 15 minutes. But whatever is implemented I'm fine.

@cnotv
Copy link
Member

cnotv commented Jan 15, 2025

@cnotv that's ok. I think we circled back on to how you've come up with those extra 15 minutes. But whatever is implemented I'm fine.

As I mentioned in the talk, the solutions are non-exclusive to each other. In mine I considered a development approach, while probably I may miss some QA requirements from your side. In the end you guys do the manual tests and know where is needed and where is not.

rancher-max
rancher-max previously approved these changes Jan 27, 2025
Copy link
Member

@rancher-max rancher-max left a comment

Choose a reason for hiding this comment

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

LGTM pending CI succeeding

@a-arias a-arias closed this Feb 6, 2025
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.

4 participants