-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add support for image diff in headless chrome #6595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
While trying to get the server examples working I generated a competing strategy for image diffing by using Bokeh's ScriptHandlers and selenium ( #6589 ) that could be extended to all of the example types. I'll comment later, but we should consider the pros and cons of using selenium to drive a browser vs calling chrome headless via a subprocess. |
|
@mattpap what is the rationale for not dog-fooding our own built-in export capability. I would much prefer to do that, barring some exceptional reason not to. (Which is not to say we should not use headless chrome, in fact it's a motivation and way to make sure the built in export absolutely works with headless chrome) |
|
@bryevdv, it may be possible to have the same code for static image generation and testing, or at least a common core, but it may also be not desirable. Testing is not only about generating pretty images, but more importantly about dealing with failure. I need to know what failed and why (see integration tests and how fun it is to work with when things fail). If this can be done with selenium, sure we can proceed this way, but I doubt it. After all, selenium has to implement the least common value of all backends. Here I experiment with Chrome Debug Protocol, and this PR is more about it, than about switching to headless chrome (I would do this anyway, even if I had to use xvfb). With CDP we have access to chrome devtools, aka. complete debugger, so we have new possibilities open. We could do network traffic or memory usage monitoring, run our code with different settings (e.g. throttling, to see how it behaves under different kinds of load), or maybe even use debugger to dump stack frames in case of exceptions being thrown. I don't know how much this is doable or feasible, but it's worth exploring. At this point I don't want to be limited by unrelated stuff. |
|
@mattpap your point about additional capabilities over and above images for other kinds of testing is well-taken, but an approach using selenium for things that can be covered that way is also appealing, both for the possibility of using many actual browsers at some point, but also for some to simply make sure selenium usage is well-exercises constantly. #6589 adds minimal support testing server examples, which I would regard as highly valuable, so I think my inclination wrt to the two PRs would be "both" but on different time scales: finish the other PR in the short term, and find areas and ways in which chrome headless and CDP can off additional specific benefits and use those (later) where appropriate. Thoughts (both of you)? |
|
I think there's a middle road here where we could use selenium (and perhaps SauceLabs) to do image diffing to help solve our problem of being blindsided by platform-specific issues. I like the concept of using CDP for writing real integration tests - I agree that our current integration test infra is difficult to extend or debug and CDP could be a real improvement there. |
|
Another benefit of using chrome directly would be the ability to use emulation mode and test painting under different pixel ratios (per issue #6840). |
90bd0a9 to
3f9727c
Compare
3f9727c to
086990b
Compare
d24e882 to
27045fd
Compare
de3e5a2 to
e85b73c
Compare
e85b73c to
db38947
Compare
db38947 to
90e323b
Compare
|
Image differences are due to chrome having different defaults (fonts in particular). |
|
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This isn't going to work right now, because it's not yet hooked into testing infrastructure, and we need newer version of nodejs available as a conda package first. To play with this locally, install most recent google chrome (chromium won't work) and nodejs 8.1.x, and then issue:
This will generate
output.pngwith rendered plots. You can visitlocalhost:9222for remote debugger.EDIT:
--disable-gpuis needed only on Windows at this point.fixes #6594