add more screenshot tests #341
Conversation
There was a problem hiding this comment.
I would try to change the mock data to include a combination of red and yellow badges as well as non-outdated VUS.
There was a problem hiding this comment.
Changed to mutation collapsible based on our conversation
| @@ -37,6 +37,13 @@ export interface BaseCollapsibleProps { | |||
| showLoadingSpinner?: boolean; | |||
| } | |||
|
|
|||
| function getDataTestid(dataTestid: string, identifier: React.ReactNode) { | |||
There was a problem hiding this comment.
Why is the type of identifier not string?
There was a problem hiding this comment.
We allow a React.ReactNode to be passed in as the collapsible title for greater flexibility. In this case, I just do a check to make sure it's a string
There was a problem hiding this comment.
Thanks for clarifying. I wonder what's the best practice to assigning data-testid.
Is it better to manually generate unique test ids for each element in a list-like component.
Or, should we query for all element with data-testid='sample-id' (which will return a list) and use indices to get the one you're looking for? (ie in react-testing-library, there is a function getAllByTestId)
There was a problem hiding this comment.
I don't think we need the extra step of querying all elements with a testid and then using the index. Seems straightforward enough to just use the title. Could even use the uuid if we want, but I think that might be overkill, especially since we control the test data.
There was a problem hiding this comment.
The V600E collapsible is missing the hotspot and exon information. I assume you need to mock the request for it to show
| 'Date.now': process.env.DOCKER ? `() => ${DEFAULT_DATE}` : 'Date.now', | ||
| 'new Date()': process.env.DOCKER ? `new Date(${DEFAULT_DATE})` : 'new Date()', |
There was a problem hiding this comment.
Will this be picked up if we run tests locally and not through docker?
There was a problem hiding this comment.
This will not. I think it's just an edge case that we should leave alone (and not introduce additional complexity). The tests will fail anyway if run locally since the screen size will be off. The only reason I would ever run the tests locally is during development so you don't need to keep rebuilding the image. Running the tests locally should always be expected to fail.
resolves https://github.com/oncokb/oncokb-pipeline/issues/367