techishiring-website icon indicating copy to clipboard operation
techishiring-website copied to clipboard

Add About Page E2E tests

Open dgodongm opened this issue 1 year ago • 9 comments

Description

Main change is addition of aboutpage.cy.ts to add some basic About page tests. There are basic tests for each of the 3 major sections/components (About Header, About Banner, and About Details). The page doesn't really have functionality so this is more of a "render" test suite. I also modified a couple of components to include data-cy attributes to more reliably identify key elements.

Related Issue

#129, #130, #158 - This PR provides E2E tests, rather than component tests, for these issues.

Motivation and Context

Provides feature coverage for About page and its sub-components. The E2E tests in this PR may suffice in place of component tests though perhaps if components get configurable options, then component tests would also be warranted. Open to feedback on which approach (E2E or component or both) would be preferred.

How Has This Been Tested?

Run in Cypress app using Chrome browser

dgodongm avatar Sep 15 '23 18:09 dgodongm

Someone is attempting to deploy a commit to a Personal Account owned by @TechIsHiring on Vercel.

@TechIsHiring first needs to authorize it.

vercel[bot] avatar Sep 15 '23 18:09 vercel[bot]

@chadstewart @marktnoonan Could you take a look at this PR? One question (besides the E2E vs component test one mentioned in the description) is whether string checks are warranted (also any plans to move strings into resource files to enable localization?).

dgodongm avatar Sep 15 '23 18:09 dgodongm

Thanks @dgodongm, I'll check this out in the next few days!

marktnoonan avatar Sep 18 '23 14:09 marktnoonan

@marktnoonan @chadstewart Gentle ping to see if you might have a chance to take a look this week.

dgodongm avatar Sep 27 '23 16:09 dgodongm

thanks @dgodongm and apologies for my slow response!

marktnoonan avatar Sep 27 '23 16:09 marktnoonan

@dgodongm thanks for adding these tests! I'm going to roll through the tickets and check if all expected tests exist:

https://github.com/TechIsHiring/techishiring-website/issues/129

  • [x] Tests if the About page banner renders properly
  • [x] Tests if the 'Ask questions section' renders properly and the ~~button opens your email~~ (outdated requirement, current behavior navigate to Contact page)
  • [ ] Tests if the 'Find us on Social Media' component renders properly and links to the proper social media -- this is not tested yet, the viewport of the test needs to be increased to show this section of the component

https://github.com/TechIsHiring/techishiring-website/issues/130

  • [x] Test if the About Page content renders properly - since all this does is render 3 other components, it's covered since the tests cover the 3 other components

https://github.com/TechIsHiring/techishiring-website/issues/158

  • [x] an article element is used for the content
  • [-] Twitter, LinkedIn, and YouTube links exist (see cy.validateLink())
  • [ ] Anything else you can think of that is important, but we don't need to verify all of the text content since it's fine for text content to change without failing a test.

I'll make some comments inline the PR with suggestions of how we might make the tests a bit more specific.

I think there are good reasons to move some of this coverage to component tests, since I see e2e tests as higher level "journey" type tests, and component tests as more specific detail-oriented tests of the behavior, but in this situation there's no need to block on that or ask you to redo work - let's get this coverage merged in e2e and then we can move it around as needed.

marktnoonan avatar Oct 01 '23 15:10 marktnoonan

@marktnoonan I made a pass at addressing your feedback. Let me know what you think. cc @chadstewart

dgodongm avatar Oct 04 '23 17:10 dgodongm

@marktnoonan checking back on if the feedback revisions look ok

dgodongm avatar Oct 10 '23 13:10 dgodongm

@marktnoonan Friendly nudge on this

dgodongm avatar Oct 16 '23 20:10 dgodongm