Summary: | [chromium] Merge chromium-gpu layout test configurations into non-gpu versions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrienne Walker <enne> | ||||||
Component: | WebCore Misc. | Assignee: | James Robinson <jamesr> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, dglazkov, dpranke, enne, jamesr, ojan, scherkus, senorblanco, tkent, vangelis, vrk, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 72444 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Adrienne Walker
2011-11-15 11:36:45 PST
Completely agree. I think the best way to tackle this is piece-by-piece. The easiest thing to tackle is the compositing/webgl/3d animation tests - they should just run as normal layout tests using osmesa as appropriate. I'll take a look at doing this today. We can leave canvas 2d in a split configuration for now, with the assumption that longer-term skia could be responsible for testing these different paths. I think I can just make DRT always enable compositing (with the normal compositing triggers) and then just run the compositing/ directories through the normal DRT path. Our normal compositing triggers should not trip on any tests outside of these directories. Hm, another potential issue is media/ - currently we run that through the compositing and non-compositing paths separately and have separate baselines for the two. If we think running these tests two times is valuable then I could rig up in the same way as canvas 2d to run without the compositor on the main layout test run and then run with the compositor triggered by media in webkit_gpu_tests. Nutty idea: could we add a feature to DRT such that it mirrors a given directory, but with a variation like a js file forcibly injected into each one? Then we could have a virtual mirror of media/ called media-gpu/, and containing nothing but expectations. With this, we could still run tests in both configs, wouldn't have to manually duplicate them, and could eliminate the separate buildbot step. On the other hand, this forces different complexity into DRT and gardening tools. But, maybe this feature would be useful in cases other than GPU land? Created attachment 115236 [details]
work in progress
We currently have 562 files (expectations and tests) in LayoutTests/platform/chromium*gpu*/compositing/*. I plan to do the following in order to land this: 1.) Copy all existing compositing expectations in gpu directories to the non-gpu-specific directory. For example, everything in platform/chromium-gpu-win/compositing/ will be copied to platform/chromium-win/ 2.) Land the code change to DumpRenderTree and our test scripts to run the compositing tests on the normal layout test run and not in the GPU run 3.) Remove the compositing expectations from platform/chromium-*gpu*/ 4.) Remove the SKIP from these lines in test_expectations.txt: WONTFIX SKIP CPU CPU-CG : compositing = PASS TIMEOUT FAIL WONTFIX SKIP CPU CPU-CG : platform/chromium/compositing = PASS TIMEOUT FAIL WONTFIX SKIP CPU CPU-CG : animations/3d = PASS TIMEOUT FAIL to start running the tests. 5.) Garden the set of failures from these lines down to zero 6.) Remove the = FAIL lines for compositing/ and animations/3d from test_expectations.txt 7.) Profit 6a) Remove run-webkit-tests support for the special -gpu configurations 6b) Remove all knowledge of the -gpu bots from the flakiness dashboard (In reply to comment #7) > 6a) Remove run-webkit-tests support for the special -gpu configurations > 6b) Remove all knowledge of the -gpu bots from the flakiness dashboard Can't do those until the canvas 2d and media tests are resolved. I'm only planning to move the compositing/ tests over right now since we only ever want to run those in one configuration (with compositing enabled via osmesa software rasterizer). For canvas 2d and media we'll need to do a bit more work. (In reply to comment #4) > Nutty idea: could we add a feature to DRT such that it mirrors a given directory, but with a variation like a js file forcibly injected into each one? Then we could have a virtual mirror of media/ called media-gpu/, and containing nothing but expectations. With this, we could still run tests in both configs, wouldn't have to manually duplicate them, and could eliminate the separate buildbot step. On the other hand, this forces different complexity into DRT and gardening tools. But, maybe this feature would be useful in cases other than GPU land? We'd have to modify NRWT to know how to mirror the trees so that it "found" both sets of tests, but that is presumably doable. Also, your idea would I think require us to have 2x the number of expectations. I don't know how many of our expectations are duplicated today, or whether this would be much of an issue. I am curious how many of these tests could be expressed as reference tests. I also wonder if we could figure out how to express CPU/GPU as a reference test (i.e., the CPU's version should match the GPU's version)? [ If we didn't duplicate the expectations, we'd have to define a fallback path and modifiers for what happens when you get differing results, which would put us back to where we are today. ] Comment on attachment 115236 [details] work in progress Attachment 115236 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10372697 New failing tests: fast/media/mq-transform-03.html transitions/default-timing-function.html animations/missing-values-first-keyframe.html fast/repaint/block-selection-gap-in-composited-layer.html fast/media/mq-transform-02.html animations/opacity-transform-animation.html animations/missing-values-last-keyframe.html http://trac.webkit.org/changeset/100329, http://trac.webkit.org/changeset/100333 and http://trac.webkit.org/changeset/100337 complete step 1. (In reply to comment #10) > (From update of attachment 115236 [details]) > Attachment 115236 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/10372697 > > New failing tests: > fast/media/mq-transform-03.html > transitions/default-timing-function.html > animations/missing-values-first-keyframe.html > fast/repaint/block-selection-gap-in-composited-layer.html > fast/media/mq-transform-02.html > animations/opacity-transform-animation.html > animations/missing-values-last-keyframe.html These are most likely failing due to the compositing trigger for animating a non-3d transform. Currently they run with compositing disabled on the main layout test run. With this patch, they would run with compositing enabled. I think we should just rebaseline with the compositing expectations, since that's the mode we run in actual chromium. Created attachment 115263 [details]
Patch
Attachment 115263 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
ERROR: FAILURES FOR <lucid, x86_64, release, cpu> in LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:2735 Path does not exist. plugins/invalidate_rect.html
LayoutTests/platform/chromium/test_expectations.txt:2735: Path does not exist. plugins/invalidate_rect.html [test/expectations] [2]
Total errors found: 1 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 115263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115263&action=review > Tools/ChangeLog:10 > + and video are still controlled by an explicit trigger so that they are true when platform=chromium-gpu and false Nit: maybe say "media" instead of video (or both) for people that aren't aware that the two are synonymous? > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:431 > + 'accelerated_video': '--enable-accelerated-video', I think you need to change --accelerated-compositing to --accelerated-video in run_webkit_tests.py as well (line220). Committed r100355: <http://trac.webkit.org/changeset/100355> Comment on attachment 115263 [details]
Patch
Clearing flags, this patch landed.
(In reply to comment #16) > Committed r100355: <http://trac.webkit.org/changeset/100355> This completes step 2 from comment #6. I'm leaving this bug open to cover the rest of the steps there. Committed r100360: <http://trac.webkit.org/changeset/100360> http://trac.webkit.org/changeset/100360 handles steps 3 and 4. Committed r100415: <http://trac.webkit.org/changeset/100415> OK, the compositing tests are now running on the main webkit_tests run only and I've gardened down to 16 failures, which seems small enough to handle by more specialized bugs. I'm going to mark this bug resolved. Open issues are how to handle the canvas 2d and media tests. Right now, those tests are still using webkit_gpu_tests. I'll open up bugs for those two categories tomorrow to discuss options. |