search

department-of-veterans-affairs / va.gov-team

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
282 stars 203 forks source link

Refactor Nightwatch Specs into Cypress #27067

Closed pjhill closed 3 years ago

pjhill commented 3 years ago

Description

The first scan of vets-website for Nightwatch specs may not have located them all. Upon attempting to remove the form tester helper files, it turns out there are still some specs that depend on it. We need to look for more Nightwatch specs and refactor them into Cypress.

Tasks

CBonade commented 3 years ago
CBonade commented 3 years ago

A few of these specs, aside from the one checked, are either partially converted or have been analyzed and found to be being covered by another test. More to come in the coming days.

CBonade commented 3 years ago

Comments on a few of these specs (will continue to add to this list as I go):

coronavirus-research/tests/00.covid-vaccine-trial.e2e.spec.js - covered by a new spec reviewAndSubmit.cypress.spec.js veteran-representative/tests/00.veteran-representative-entry.e2e.spec.js - this test is currently being skipped in Nightwatch, so it has been set up in Cypress the same way. However, removing this skip. this test passes top to bottom without issue.

CBonade commented 3 years ago

Temporarily on hold prioritizing Cypress reliability for GHA launch

CBonade commented 3 years ago

Resumed working through a few of these specs today while waiting for Github actions runs to complete. Checklist of current progress is up to date.

CBonade commented 3 years ago

Five more specs checked off today. Two of the remaining specs are pending Noah's in-progress effort on keyboard testing in Cypress. The last one, is quite a large set of data and I will begin working through that.

CBonade commented 3 years ago

Pre-needs spec is about 80% complete.

Additionally ran into an issue regarding duplicating a Nightwatch step that involved cross-origin navigation, as Cypress does not allow that. The solution after discussing with Peter that we are going to attempt to go with, is to mock the navigation and just ensure that it's trying to navigate to the correct url.

CBonade commented 3 years ago

I've removed the User nav test from the checklist as it's been problematic trying to get a cross-origin check working in Cypress. This test works locally with chromeWebSecurity disabled, but will not in CI. Therefore, I've commented out those lines in the meantime to get the rest of the completed specs into our regular testing routine to start testing their stability. As soon as the initial PR has passed checks, I'll flag it for review. Nightwatch has been added back into it as well, while we test out these new specs.

CBonade commented 3 years ago

Done a fair bit of research at this point for options to test cross-origin in Cypress. Cypress offers some workarounds, but none that work for the structure of our product. Therefore, that leaves us with just a couple options:

1) Test that the sign out button exists, assert it's text is correct, leave it at that (does not test our SSO sign out in any fashion). 2) Test that the errors we get are the correct errors for what we would expect if cross-origin was accessed in a Cypress spec, to verify at a minimum that an attempt was made to navigate to a different domain. 3) Rework the React code to either insert the href into the button which can then be verified by the test, or emit an event that can be caught and checked in the test as well that contains the url as a payload.

I went with #2 for now, and have it sitting in a branch. It checks both that we get a network error (expected making a request from a cross-domain url) and additionally checks that we get an appropriate Chrome Error inside Cypress when attempting to navigate cross-origin. Both these checks pass on the test:

https://github.com/department-of-veterans-affairs/vets-website/pull/18041

image.png

CBonade commented 3 years ago

Got the two outstanding reviews today, however unfortunately, some of the comments from that review resulted in an additional need for 7 more code owner reviews. The biggest change made here was the removal of {timeout: Timeouts.normal} from all cypress specs in favor of leaning on Cypress' built-in timeout. Other changes include moving specs around, deleting specs, among others. I imagine this will be in review for a bit longer.

CBonade commented 3 years ago

One review down, five to go!

CBonade commented 3 years ago

User nav test issues were solved today and merged in. The large PR with the bulk of the Nightwatch rewrites has not had any more reviews yet (still waiting on 4)

CBonade commented 3 years ago

No additional reviews today - still awaiting 4 teams.

CBonade commented 3 years ago

No new reviews, awaiting 4 more teams

CBonade commented 3 years ago

Only the two keyboard specs remain here, which is awaiting further development on the keyboard helper functions. I've started a new, clean task for the keyboard test work. This one is ready to close. https://app.zenhub.com/workspaces/vsp-5cedc9cce6e3335dc5a49fc4/issues/department-of-veterans-affairs/va.gov-team/29618

pjhill commented 3 years ago

It's done. Why did I wait so long to close this?!