Bug 79956 – need to clone the GPU baselines and expectations into the new "virtual" dir

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

[Dirk Pranke]

As part of the changes for bug 76501, I am adding the ability to run "virtual test suites" in NRWT. The way this is going to work is that you can declare a directory foo to mirror a directory bar (so that we run all the tests in bar), but run the tests with an additional set of command line flags, and support a new set of directories for baselines in the fallback path.

So, to be concrete, I'm adding a new directory 'LayoutTests/platform/chromium/virtual_gpu/fast/canvas' that will point to 'fast/canvas'. Any baselines that were originally in platform/chromium-gpu-mac/fast/canvas will now need to be copied into platform/chromium-mac/platform/chromium/virtual_gpu/fast/canvas according (and same for the win and linux ports).

We will also need to list any expectations lines twice; any test in fast/canvas that is expected to fail will also need a listing in platform/chromium/virtual_gpu/fast/canvas.

This is all straightforward; once we have the new virtual dirs working stably we can then delete the chromium-gpu directories and the "GPU" expectations.

[Stephen White]

How is platform/chromium-mac/platform/chromium/virtual_gpu/fast/canvas
better than platform/chromium-gpu-mac/fast/canvas?

AFAICT, we'll have the same number of test results files, and twice the number of test_expectations lines, and longer and more confusing directory paths to find them.

I'm not convinced this is actually better.   So... convince me?  :)

[Adam Barth]

The latter doesn't require gardeners to do any special GPU-specific work and therefore easier to maintain.

[Adam Barth]

Sorry, "the former" rather.

[Dirk Pranke]

We will no longer need the 'webkit_gpu_tests' step on the bot - the tests will run as part of the main test run. This allows us to delete a fair amount of complexity in the tools for dealing with additional "ports" that aren't really ports.

I think removing the 'gpu port' as a concept will remove a fair amount of cognitive overhead that we currently inflict on the team. I think the 'virtual suite' concept is less overhead and will actually be more useful down the road for other things as well.

[Stephen White]

(In reply to comment #4)
> We will no longer need the 'webkit_gpu_tests' step on the bot - the tests will run as part of the main test run. This allows us to delete a fair amount of complexity in the tools for dealing with additional "ports" that aren't really ports.

Oh right, that's a pretty important step.

It's a shame we can't keep the GPU modifier, though.  When I tried this experimentally (by copying) for canvas/philip, it added ~65 lines to test_expectations, which I suppose is a bullet we have to bite.

Just to bikeshed a little, I'm not too fond of "virtual_gpu" as a directory name.  It sounds like the GPU is virtual.  Could it just be "gpu" instead?  Or maybe "virtual/gpu"?

And platform/chromium-mac/platform/chromium/virtual_gpu/fast/canvas is way long.  Could it be
platform/chromium-mac/gpu/fast/canvas
instead?  As long as nobody adds a new "gpu" directory off of LayoutTests, there should be no conflicts.

[Dirk Pranke]

(In reply to comment #5)
> (In reply to comment #4)
> > We will no longer need the 'webkit_gpu_tests' step on the bot - the tests will run as part of the main test run. This allows us to delete a fair amount of complexity in the tools for dealing with additional "ports" that aren't really ports.
> 
> Oh right, that's a pretty important step.
> 
> It's a shame we can't keep the GPU modifier, though.  When I tried this experimentally (by copying) for canvas/philip, it added ~65 lines to test_expectations, which I suppose is a bullet we have to bite.
> 

Yeah. As I mentioned in my note on chromium-dev@, I think we need to move to checking in failing expectations and deleting lines in the expectations file, so this hopefully goes away soon.

An alternative, though, that occurred to me a while ago would be to add in code that could inherit the expectation just like we inherit the baselines. If you wanted to have different expectations for the same test and the virtual test, you'd still need two lines (just like you need two lines for cpu/gpu today, of course).

I'm not sure if this expectations-inheriting is worth it, though, or if we should just move to checking in the expectations.

> Just to bikeshed a little, I'm not too fond of "virtual_gpu" as a directory name.  It sounds like the GPU is virtual.  Could it just be "gpu" instead?  Or maybe "virtual/gpu"?

Bikeshedding welcome here :) virtual_gpu is the best JamesR and I came up with ... we thought it was important to indicate somehow that this was a virtual directory and not a regular one. virtual/gpu would be fine, though.

> 
> And platform/chromium-mac/platform/chromium/virtual_gpu/fast/canvas is way long.  Could it be
> platform/chromium-mac/gpu/fast/canvas
> instead?  As long as nobody adds a new "gpu" directory off of LayoutTests, there should be no conflicts.

That wouldn't be consistent with the way we handle platform-specific baselines for other (non-virtual) tests, and I would prefer not to have to special case the logic.

[Dirk Pranke]

James is not wedded to virtual_gpu, and I actually like the idea of having a convention that you can find all of the virtual tests in one place, so virtual/gpu it is.

[Dirk Pranke]

Created attachment 129569 [details]
Patch

[Dirk Pranke]

Note that there are a bunch of baselines I'll be adding with this, and a line to SKIP the tests for now in test_expectations.txt.

[Dirk Pranke]

Committed r109296: <http://trac.webkit.org/changeset/109296>