Add "point style" image tests #5629
Conversation
|
Tests didn't run as smooth as I expected, I think the diff come from border aliasing. Anyone on Linux able to screenshot the image diff errors using I don't know if Travis allows to store artifact for each build. |
|
@simonbrunel screenshot attached. It fails for me only on Firefox. All tests pass on Chrome. I wonder though if you can't see how these are failing if that will make it hard for you to develop going forward? What OS are you on and why doesn't it work there? Do we also need to update the developer documentation to mention this?
|
Replace the old style canvas "mock" context checks by image based unit tests which are easier to maintain and allow more flexibility in the drawing logic since we are not testing the context calls but the final painted result.
simonbrunel
force-pushed
the
chore/point-style-tests
branch
from
fddb322
to
7a5515b
Compare
@benmccann Win10 but I think it's due to hardware acceleration which is certainly not available on Travis. If I disable it on my machine, then I get the same results as you posted in your last comment. So one solution would be to explicitly disable hardware acceleration when running unit tests and for everyone (not only on Travis). This could also help on images containing text. I updated this PR with GPU disabled and new fixture images. @etimberg @benmccann thoughts?
If we enforce software rendering, I don't think it's needed. However it would be great to port this comment to the docs, maybe in developers/contributing. |
| @@ -1,4 +1,6 @@ | |||
| describe('Chart.controllers.bubble', function() { | |||
| describe('auto', jasmine.specsFromFixtures('controller.bubble')); | |||
There was a problem hiding this comment.
Is there a reason we need this describe block but we don't need anything for the element.point tests?
There was a problem hiding this comment.
There is the same describe('auto') for element.point. It's used to group all it() generated for each .json in the fixture folder under the same "auto" label, so I think it's better to have it:
|
Cool! Always disabling the GPU seems like a good solution to me |
simonbrunel
force-pushed
the
chore/point-style-tests
branch
from
8bcae99
to
a955f9d
Compare
Explicitly disable hardware acceleration to make image diff more stable when ran on Travis and dev machine.
simonbrunel
force-pushed
the
chore/point-style-tests
branch
from
a955f9d
to
35dc937
Compare
We are receiving a few PRs related to the point style, so it could be better to replace the old style canvas "mock" context checks by image based unit tests which are easier to maintain and allow more flexibility in the drawing logic since we are not testing the context calls but the final painted result.
Related to #4811 #5180 #5623