Playwright tests #1832
Playwright tests #1832
Conversation
…msw handlers with gql.tada inference
…laywright-tests
…laywright-tests
…laywright-tests
…laywright-tests
…laywright-tests
…laywright-tests
…laywright-tests
…laywright-tests
…laywright-tests
…laywright-tests
|
@fhenrich33 can you post a link in the description with a fully rendered file, like what @pylipp does with his? |
|
Good ADR @fhenrich33. There are some missing sentences in the context section that should be cleaned up, and I'd like that section on MSW to clarify that you're referring to the current production or development environment (or a specific part of it) rather than just saying "we". Similarly, for the Readme, it's good to be precise with your terminology. For example:
Otherwise, nice work, please request reviews whenever you're ready. |
There was a problem hiding this comment.
Are we removing the old tests and expecting to rewrite them again using Playwright? @fhenrich33
There was a problem hiding this comment.
Yes, for rewriting tests with Playwright; no, for rewriting tests that already exist. I'm almost finished with that right now!
| product: element.product!.name, | ||
| productCategory: element.product!.category.name, | ||
| gender: element.product!.gender, | ||
| product: element.product?.name, |
There was a problem hiding this comment.
quick qn: Why are we removing the non-null assertion operator here? Is it possible that product/location can be optional?
There was a problem hiding this comment.
Good that you brought that up!
! aka non-null assertion operator is a TypeScript compiler flag of sorts, telling TS to exclude null and undefined from the type of this property, or in a way telling it's okay if they end up being those types. This doesn't help on runtime as this is a type-only thing.
But since this piece of code blew up for me using mock data that had this property nullified in some entries, the right thing to do is to use ? or the optional chaining operator to avoid exceptions on trying to access properties inside an undefined or null object/property.
| - No built-in support for cross-browser testing. | ||
|
|
||
| 2. **Playwright** | ||
| - Pros: |
There was a problem hiding this comment.
I thought the main Pro argument is that you can easily generate mock data @fhenrich33 ?
There was a problem hiding this comment.
This would be the benefit of gql.tada, used with MSW request handlers. Playwright does have a very similar mocking capability as MSW, but we use the latter to allow us to mock yet-to-be-implemented schemas in the future.
| @@ -82,35 +84,104 @@ Afterwards: | |||
|
|
|||
| ## Testing | |||
|
|
|||
| Testing is done with React Testing Library and Jest. | |||
| Testing is done with [Playwright](https://playwright.dev/) for application, integration, and component testing. | |||
There was a problem hiding this comment.
feels like you have pasted the same section twice here. Also please ensure that the old section on react testing library is still in, but that it is clear that we are slowly deprecating it.
There was a problem hiding this comment.
Did you read from the README in the PR description section? I didn't find any dupes there.
Also, I'm replicating the entire test suite where we used React Testing Library with Playwright, so I don't think it makes sense to keep it in that case.
There was a problem hiding this comment.
Apologies @HaGuesto, it seems that the description didn't updated properly.
…laywright-tests
|
Good clear updates to the comparative evaluation. Nice work. |
ADR: https://github.com/boxwise/boxtribute/blob/58298c1ab5c793b3afd3a4420339d4b614f3e7bb/docs/adr/adr_frontend_tests.md
Updated FE README: https://github.com/boxwise/boxtribute/blob/3fc1483a3b3051206e900fbd1cb02520b3158594/front/README.md#testing