TechIsHiring / techishiring-website

A space where great Engineers and great jobs can find each other! We're on Twitter and LinkedIn and have a newsletter on Substack!
https://www.techishiring.com
MIT License
52 stars 34 forks source link

Add About Page E2E tests #162

Open dgodongm opened 1 year ago

dgodongm commented 1 year ago

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

vercel[bot] commented 1 year ago

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

@TechIsHiring first needs to authorize it.

dgodongm commented 1 year ago

@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?).

marktnoonan commented 1 year ago

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

dgodongm commented 1 year ago

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

marktnoonan commented 1 year ago

thanks @dgodongm and apologies for my slow response!

marktnoonan commented 1 year ago

@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

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

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

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.

dgodongm commented 1 year ago

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

dgodongm commented 11 months ago

@marktnoonan checking back on if the feedback revisions look ok

dgodongm commented 11 months ago

@marktnoonan Friendly nudge on this