gl2d image tests (finally) #1037
Conversation
- which are now reproducible via #980
- warn when gl2d are ran batch - skip over multiple subplot test on CI, see #980 for details
- reuse svg version in jasmine test instead
- that (IMHO) don't test additional behaviour - as gl2d image test is expensive on CI, we need to reduce the number of gl2d mocks free up test time
|
@monfera would you mind reviewing this PR? |
| @@ -35,6 +35,7 @@ | |||
| "test-jasmine": "karma start test/jasmine/karma.conf.js", | |||
| "citest-jasmine": "karma start test/jasmine/karma.ciconf.js", | |||
| "test-image": "node tasks/test_image.js", | |||
| "test-image-gl2d": "node tasks/test_image.js gl2d_* --queue", | |||
There was a problem hiding this comment.
run npm run test-image-gl2d to run gl2d image tests locally.
| 'gl2d_stacked_subplots', | ||
|
|
||
| // not sure why this one still fails on CircleCI | ||
| 'gl2d_pointcloud-basic' |
There was a problem hiding this comment.
@dfcreative did you ever get this mock working on CI?
There was a problem hiding this comment.
... maybe a commit of yours never made it in #980
There was a problem hiding this comment.
@etpinard I suppose the pointcloud CI testing failure is for the reason discussed from this comment #980 (comment)
Any way I can simulate the workings of CI locally, or read about its constituents? I could try and see what's going on with the pointcloud test blending. Although it should not hold back this PR.
There was a problem hiding this comment.
Any way I can simulate the workings of CI locally
You can try npm run test-image -- gl2d_pointcloud-basic but that results in a passing test on my machine. Not sure what differences on CI may lead to this issue.
Referencing #241
| * at the moment. | ||
| * | ||
| * For more info see: | ||
| * https://github.com/plotly/plotly.js/pull/980 |
There was a problem hiding this comment.
Nice doc reference in the code!
| 'gl2d_stacked_subplots', | ||
|
|
||
| // not sure why this one still fails on CircleCI | ||
| 'gl2d_pointcloud-basic' |
There was a problem hiding this comment.
@etpinard I suppose the pointcloud CI testing failure is for the reason discussed from this comment #980 (comment)
Any way I can simulate the workings of CI locally, or read about its constituents? I could try and see what's going on with the pointcloud test blending. Although it should not hold back this PR.
| "separators": ".,", | ||
| "hidesources": false | ||
| } | ||
| } |
There was a problem hiding this comment.
With lots of test mock removals, aren't they going to be missed, or are they covering things covered in other test cases?
There was a problem hiding this comment.
are they covering things covered in other test cases?
To me eyes, the removed mocks were not adding anything to our test coverage %.
| } | ||
| ], | ||
| "layout": { | ||
| "title": "Connect gaps test" | ||
| "title": "Connect gaps + line opacity test" |
There was a problem hiding this comment.
OK I see this is one type of new coverage. Btw. indeed the image tests are rather expensive, around 1s each.
| } | ||
|
|
||
| if(isCI) { | ||
| console.log('Filtering out multiple-subplot gl2d mocks:'); |
There was a problem hiding this comment.
Maybe this could be a slightly more general log entry b/c the pointcloud test is in this set too -- or we might want to alter the pointcloud test such that it doesn't test the blending effect (I can do it in a separate small PR) but even then, at least most of the pointcloud stuff would be tested, vs. no pointcloud test on CI right now.
There was a problem hiding this comment.
we might want to alter the pointcloud test such that it doesn't test the blending effect (I can do it in a separate small PR)
that would be great!
- to make it work, make sure pointcoud mocks are tested first.
This PR adds image test support for gl2d mocks locally and on CircleCI.
It builds on top of @dfcreative's incredible work in #980 🍻
IMPORTANT as stated in #980, image test support for gl2d will be limited. In details:
Because of the first limitation, creating gl2d images is expansive. So, in this PR, a few gl2d mocks were merged or removed to bring down the number of gl2d mocks to about 20. This makes the gl2d image test command run in about 30 seconds on CI: