Closed (fixed)
Project:
Drupal core
Version:
8.7.x-dev
Component:
phpunit
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
21 May 2019 at 09:38 UTC
Updated:
24 Jun 2019 at 13:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
alexpottSo Thunder uses 1024x768 as its default JS testing size. I think that that is better that 800x600 which no one uses and is very small. So let's set it in core and see what happens.
Note the patch attached includes #2901792: Disable all animations in Javascript testing as that is necessary for consistent JS testing but it also reverts a change from that.
Comment #3
alexpottHmmmm... so what's weird is that without
The following command fails...
sudo -u _www ./vendor/bin/phpunit -v -c ./core core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTest.php --filter testWidgetOEmbedIt fails with:
However if I run it like testbot ie with:
$ sudo MINK_DRIVER_ARGS_WEBDRIVER='["chrome", {"browserName":"chrome","chromeOptions":{"args":["--disable-gpu","--headless"]}}, "http://localhost:4444"]' -u _www ./vendor/bin/phpunit -v -c ./core core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTest.php --filter testWidgetOEmbedIt passes! And then remove the --headless and fails. So there's something about running it in a browser that you can see that is causing issues.
Comment #4
alexpottSo taking screenshots of what happens with headless vs not headless you can see that it's the youtube view that is behaving differently. Somehow in the non headless it is playing!!!
Passing test with --headless
Failing test without --headless
Comment #5
alexpottSo preventing the embedded media loading in the iframe fixes the locally testing when --headless is not present.
Comment #7
alexpottPatch without #2901792: Disable all animations in Javascript testing as that's been committed.
The patch no longer attempts to remove the oembedded content. It should work BUT there's a bug in chromedriver - see https://bugs.chromium.org/p/chromedriver/issues/detail?id=2758 so I've added the @todo and left things how there are for now.
We should probably open another issue to discuss whether we should be using real youtube videos in testing. However on the plus side this does mean we are testing with real content doing what a real user would do - which kinda is the point of this type of testing.
Comment #8
lendude> 80 characters
Discussed the chosen screen size with @alexpott on slack, 1024x768 makes sense as a default so that it still fits on most laptops when running the tests locally. Updated the IS for this.
Comment #9
alexpottComment #10
lendudeRTBC assuming this comes back green.
Comment #11
catchCommitted c7b8bc5 and pushed to 8.8.x. Thanks!
Comment #12
catchAnd cherry-picked to 8.7.x.