Bug 66705 – [Chromium] Expand CPU/GPU/Skia/CG into a matrix in graphics_type.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dimitri Glazkov (Google)]

Move CG from being a graphics type to a version suffix.

[Dimitri Glazkov (Google)]

Created attachment 104724 [details]
Patch

[Dimitri Glazkov (Google)]

I also need to modify downstream overrides to use the suffixes.

[Dimitri Glazkov (Google)]

(In reply to comment #2)
> I also need to modify downstream overrides to use the suffixes.

GOOD NEWS! The downstream is clear.

[Adam Barth]

Comment on attachment 104724 [details]
Patch

I don't think this is correct.  How will this work when we add the Skia bots?  Will we need to add mac-skia entries to all of these lines?

[Dimitri Glazkov (Google)]

(In reply to comment #4)
> (From update of attachment 104724 [details])
> I don't think this is correct.  How will this work when we add the Skia bots?  Will we need to add mac-skia entries to all of these lines?

Well, if they are different, they will have different sets of expectations. I don't see any other choices? Would you like to leave them as just MAC, LEOPARD, SNOWLEOPARD, and when the Skia expectations are added, they'll be MAC-SKIA, etc.?

[Dimitri Glazkov (Google)]

Talking w/dpranke, he suggested flattening these orthogonal dimensions in the graphics type: CPU-CG GPU-CG, CPU and GPU. This way, it will be closer in meaning and not as invasive. For now, we could say that MAC : foo = FAIL implies both CG and SKIA failure. WDYT?

[Adam Barth]

That sounds like a good idea.  According to a recent discussion with jamesr, we'll need to have both CG and Skia versions of the GPU results anyway, so we'll need all four entries in the CPU/GPU, CG/Skia matrix.

[Dimitri Glazkov (Google)]

Created attachment 104739 [details]
Patch

[Dimitri Glazkov (Google)]

Committed r93545: <http://trac.webkit.org/changeset/93545>

Is there not a unittest somewhere that exercises _generate_all_test_configurations() for the Mac configurations?  This CL changes the set of configurations that will be returned, but the only unittest change I see here is for Vista.

[Dimitri Glazkov (Google)]

(In reply to comment #10)
> Is there not a unittest somewhere that exercises _generate_all_test_configurations() for the Mac configurations?  This CL changes the set of configurations that will be returned, but the only unittest change I see here is for Vista.

I think there is: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py#L346

But please add more tests if the isn't!

I have added a unittest for port.all_test_configurations() in https://bugs.webkit.org/show_bug.cgi?id=67211 .

That unittest confirms that the Mac configurations can now have any one of these 4 graphics_types:
cpu, gpu, cpu-cg, gpu-cg

While the non-Mac configurations can have one of these 2 graphics_types:
cpu, gpu

Are these assumptions correct?
1. the "cpu-cg" and "gpu-cg" graphics_types are not being exercised at all yet
2. lines in test_expectations that specify "CPU", when run on the Mac, currently apply to tests that are run using either Skia or Core Graphics
3. our goal is for test_expectations lines that specify "CPU" to apply only to tests that are run using Skia
4. our goal is for test_expectations lines that specify "CPU-CG" to apply only to tests that are run using Core Graphics

One more question: How should chromium.py know whether configure the graphics_type as CPU or CPU-CG?  

Ideally, it could query the DumpRenderTree binary to see whether it was built against Skia or CG.  Is that possible?

Or maybe I should add a runtime flag to run_webkit_tests.py and depend on the caller to pass it correctly?  (Seems dangerous...)

[Dirk Pranke]

(In reply to comment #13)
> One more question: How should chromium.py know whether configure the graphics_type as CPU or CPU-CG?  
> 
> Ideally, it could query the DumpRenderTree binary to see whether it was built against Skia or CG.  Is that possible?
> 
> Or maybe I should add a runtime flag to run_webkit_tests.py and depend on the caller to pass it correctly?  (Seems dangerous...)

The port name is effectively a runtime flag already. It might be nice to have a way to check that the binary was compiled to match, but let's hope that the CG code doesn't live long enough to make this necessary.

[Adam Barth]

> Or maybe I should add a runtime flag to run_webkit_tests.py and depend on the caller to pass it correctly?  (Seems dangerous...)

You should be able to pass --platform=chromium-mac to select the Skia configuration and --platform=chromium-cg-mac to select the CG configuration.

Thanks, Dirk and Adam.  As Adam wrote, I will use the port name ("--platform" command line argument) to allow the caller to override the default chromium-cg-mac platform if appropriate.

When we change the default value for use_skia from 0 to 1, I will change the default mac platform from chromium-cg-mac to chromium-mac.

The builder configurations in http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/builders.py already specify "chromium-mac-leopard" or "chromium-cg-mac-leopard" as appropriate for each buildbot.