EDSC-4151: Convert /path/search/granules.cy.js to Playwright #1773
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1773 +/- ##
==========================================
+ Coverage 82.40% 83.22% +0.82%
==========================================
Files 700 700
Lines 17503 17503
Branches 4565 4595 +30
==========================================
+ Hits 14423 14567 +144
+ Misses 3027 2790 -237
- Partials 53 146 +93 ☔ View full report in Codecov by Sentry. |
| await route.fulfill({ | ||
| body: JSON.stringify(projectGranuleProjectGranuleBody), | ||
| headers: { | ||
| ...commonGranulesHeaders, |
There was a problem hiding this comment.
| ...commonGranulesHeaders, | |
| ...commonGranulesHeaders, | |
| 'access-control-expose-headers': 'cmr-hits', |
I'm surprised we didn't need this here I think we should just add 'access-control-expose-headers': 'cmr-hits',
to the commonGranulesHeaders file then you don't have ot append it each call
There was a problem hiding this comment.
I'm not sure either come to think of it, because this part of the conditional is definitely getting called. It may have been unnecessary in some of the tests but I just went through and added it to all of them (though I seemingly missed this one)
| } | ||
|
|
||
| route.fulfill({ | ||
| body: JSON.stringify(temporalTimelineBody), |
There was a problem hiding this comment.
Just in general the reason you have to use Stringify alot here can be worked around by doing a
| body: JSON.stringify(temporalTimelineBody), | |
| json: temporalTimelineBody |
There was a problem hiding this comment.
I found a good bit of formatting inconsistencies that we should clean up but nothing major. I am a little confused how we ended up with 100k+ mocks in this PR if all we were doing is converting an already existing test. Maybe we can sync up tomorrow and you can fill me in on that.
There was a problem hiding this comment.
All files should end with a new line. It might be worth spending a few minutes exploring if we could enforce this with an eslint config or another linting tool.
There was a problem hiding this comment.
File should end with a new line
There was a problem hiding this comment.
File should end with a newline
There was a problem hiding this comment.
File should end in a new line
There was a problem hiding this comment.
File should end in a new line
There was a problem hiding this comment.
Looks like tabs not spaces
There was a problem hiding this comment.
Using tabs instead of spaces
| await page.route('**/search/granules/timeline', (route) => { | ||
| const request = route.request() | ||
| const body = request.postData() | ||
| if (body) { |
There was a problem hiding this comment.
It seems kind of odd to have an if block in a test. I see this in a few spots. Why would body not exist when running this test?
There was a problem hiding this comment.
I wondered the same thing, but this was the functionality in the previous test file I converted it from. I did console log both parts of the conditional in a few of these tests to see if they were both getting hit.
| await page.route('**/search/granules.json', (route) => { | ||
| const request = route.request() | ||
| const body = request.postData() | ||
| expect(body).toBe('browse_only=true&echo_collection_id=C1214470488-ASF&page_num=1&page_size=20') |
There was a problem hiding this comment.
We generally have new lines to separate blocks of variables, expect blocks, and function calls. There are lots of occurrences of this in this file. Make sure all the spacing is consistent please :)
Happy to tag up tomorrow on this. I didn't create any new mocks for this test rewrite, I only reused the ones being utilized by the cypress test file. I'll definite work out all the spacing inconsistencies in the json files. Unless they somehow got changed by moving directories, they've been like that the whole time, so I'll go through and make sure they are all conforming. |
|
Added newlines to the end of all the files, and changed the spacing to be consistent across all files. |
Overview
What is the feature?
Converting Granules cypress tests to playwright
What is the Solution?
Rewrite relevant cypress tests in playwright.
What areas of the application does this impact?
e2e testing
Testing
Reproduction steps
paths/search/granules/granules.spec.jsChecklist