-
Notifications
You must be signed in to change notification settings - Fork 34
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
Refactor out header checks in homepage E2E tests #165
base: dev
Are you sure you want to change the base?
Conversation
Someone is attempting to deploy a commit to a Personal Account owned by @TechIsHiring on Vercel. @TechIsHiring first needs to authorize it. |
@chadstewart @marktnoonan What do you think of this refactoring, and additional mobile test? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, nice to capture mobile and since everything is the same it makes perfect sense to pull the repeated code into a helper function.
Also there's a good point raised about component testing and the latest version of next, let's not sink too much time into solving those issues right away if the same tests will work fine from e2e.
Thanks @marktnoonan! @chadstewart - any feedback or are you good with merging? |
@chadstewart @marktnoonan Can this be merged? |
@chadstewart this needs you to authorize the Vercel step. Or I can merge without that since it's just tests. |
@chadstewart Friendly ping on this PR |
Description
Created validateHeader util module with helper methods similar to validateFooter, and updated homepage E2E tests to use these, adding distinct test for mobile as well.
Related Issue
#121 - provides some E2E coverage instead of component as it seems there's technical impediment for the latter - see comments in this issue
Motivation and Context
Ensure more consistent approach between footer and header checks. And, adding some coverage for modal menu on mobile.
How Has This Been Tested?
Tested locally on Chrome
Screenshots (if appropriate):
n/a