Bug 66705 - [Chromium] Expand CPU/GPU/Skia/CG into a matrix in graphics_type.
Summary: [Chromium] Expand CPU/GPU/Skia/CG into a matrix in graphics_type.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-22 13:17 PDT by Dimitri Glazkov (Google)
Modified: 2011-08-30 14:07 PDT (History)
6 users (show)

See Also:


Attachments
Patch (132.76 KB, patch)
2011-08-22 13:20 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Patch (3.96 KB, patch)
2011-08-22 14:31 PDT, Dimitri Glazkov (Google)
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2011-08-22 13:17:46 PDT
Move CG from being a graphics type to a version suffix.
Comment 1 Dimitri Glazkov (Google) 2011-08-22 13:20:42 PDT
Created attachment 104724 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2011-08-22 13:21:57 PDT
I also need to modify downstream overrides to use the suffixes.
Comment 3 Dimitri Glazkov (Google) 2011-08-22 13:42:27 PDT
(In reply to comment #2)
> I also need to modify downstream overrides to use the suffixes.

GOOD NEWS! The downstream is clear.
Comment 4 Adam Barth 2011-08-22 13:52:48 PDT
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?
Comment 5 Dimitri Glazkov (Google) 2011-08-22 13:55:31 PDT
(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.?
Comment 6 Dimitri Glazkov (Google) 2011-08-22 14:04:36 PDT
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?
Comment 7 Adam Barth 2011-08-22 14:08:16 PDT
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.
Comment 8 Dimitri Glazkov (Google) 2011-08-22 14:31:56 PDT
Created attachment 104739 [details]
Patch
Comment 9 Dimitri Glazkov (Google) 2011-08-22 14:45:55 PDT
Committed r93545: <http://trac.webkit.org/changeset/93545>
Comment 10 epoger 2011-08-29 14:01:40 PDT
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.
Comment 11 Dimitri Glazkov (Google) 2011-08-29 14:14:44 PDT
(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!
Comment 12 epoger 2011-08-30 11:16:23 PDT
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
Comment 13 epoger 2011-08-30 12:56:44 PDT
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...)
Comment 14 Dirk Pranke 2011-08-30 13:47:12 PDT
(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.
Comment 15 Adam Barth 2011-08-30 14:01:05 PDT
> 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.
Comment 16 epoger 2011-08-30 14:07:38 PDT
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.