tests(increase-coverage): weekly tests PR #4202
Conversation
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Passing run #4046 ↗︎
Details:
Review all test suite changes for PR #4202 ↗︎ |
|||||||||||||||
|
tests failing in the CI are unrelated to my changes |
tests/utils/screenShotPaths.ts
Outdated
| mpr: { | ||
| mprDisplayedCorrectly: 'mprDisplayedCorrectly.png', | ||
| angle: { | ||
| viewportLoaded: 'viewportLoaded.png', |
There was a problem hiding this comment.
why there are so many viewportLoaded that basically the same thing?
There was a problem hiding this comment.
The idea is, along side checking that the network is idle, each test should check for an image that indicates the viewer is in a state that’s ready for testing (no black viewports, etc), one reliable way to do it, to get a screenshot of the loaded state, and compare against it until its ready, if that state is not reached in time just fail the test and exit.
i think each test should have it’s own loaded state images, duplicated or not its fine, in this case they all have the same state so it looks the same but that won’t always be the case, for example a more advanced test that touches 3D hanging protocols, should have multiple images like 3DOnlyLoaded, 3DPrimaryLoaded, then if you would like to test the trackballrotate tool, you wait first for the viewer state to match that image, then test the tool.
There was a problem hiding this comment.
I think this is also more reliable than CS3D events cuz they are not always accurate either
tests/Probe.spec.ts
Outdated
| }); | ||
|
|
||
| test('should display the probe tool', async ({ page }) => { | ||
| await checkForScreenshot(page, page, screenShotPaths.probe.viewportLoaded); |
There was a problem hiding this comment.
I don't understand why we start by checking the screenshot, that should be a separate test to check loading
There was a problem hiding this comment.
I answered above
IbrahimCSAE
force-pushed
the
test/weekly-test-pr
branch
from
5d49bd7 to
b96eac0
Compare
Context
This is a weekly tests PR.