How can I land new tests?

38 views
Skip to first unread message

Nico Weber

unread,
May 5, 2013, 9:50:24 PM5/5/13
to blink-dev
Hi,

https://codereview.chromium.org/14783007/ adds a new test. As far as I understand, the way to do this is to land the test with an "Failure" expectation and then grab the right baselines from the bots later.

However, the trybots always complain about my new test, even though I have added this baseline. I tried having just a line in TestExpectations, that entry and an -expected.txt file, and entry, txt, and png file. The bots rejected all these attempts. (See the various patch sets at the above URL).

How can I land new tests through the cq?

Thanks,
Nico

Nico Weber

unread,
May 5, 2013, 10:16:04 PM5/5/13
to blink-dev
dstockwell suggested removing the baselines and using "Missing" as expectation. That seems to work. Is this the right thing to do?

Ojan Vafai

unread,
May 5, 2013, 11:34:25 PM5/5/13
to Nico Weber, blink-dev
TL;DR: Adding a "Missing" entry + including the -expected.txt for the platform your working on is current best practice AFAIK.


There's multiple compounding problems here. Sorry for not catching this in your review. We've been so busy deleting things, there haven't been many patches adding new layout tests.

1. The try servers don't accept binary patches, so you can't have any pngs in your patch if you're going to use the commit queue (which you should of course!).
2. If a pixel test has a -expected.txt, but lacks a -expected.png the failure type will be "Missing", or if the -expected.txt is lacking, you also get "Missing"
3. Ideally you'd still include the -expected.txt file in your patches because it's useful for the reviewer doing the review.

There's a few solutions to this that have been proposed, probably all of which should be implemented. I'm not sure if anyone is actively working on any of them.

1. Long-term: we should be able to get expected results off the try bots and integrate them into your patch so you don't need to add any TestExpectations entries.
2. Hopefully, short-term: The trybots and rietveld need to be able to handle pngs.
3. Easy short-term: Add a "NeedsRebaseline" keyword that will basically map to "Failure ImageOnlyFailure Missing" so you can add a simple line to TestExpectations and so it's clear that this line was just added so the bots could cycle and you do your rebaseline (as a bonus, this avoids all the silly "# needs rebaseline" comments).

3 should be super easy to implement. Just need a volunteer willing to write some python.


P.S. Incidentally, the timeouts in your last commit queue attempt are my fault. I removed some slow lines based off the flakiness dashboard data only to discover the the flakiness dashboard is missing some data for some slow tests that pass: http://crbug.com/238159. I should be able to get that fixed tomorrow. In the meantime, I tried readding the lines to TestExpectations in https://codereview.chromium.org/14979002.

Dirk Pranke

unread,
May 6, 2013, 3:15:58 PM5/6/13
to Ojan Vafai, Nico Weber, blink-dev
On Sun, May 5, 2013 at 8:34 PM, Ojan Vafai <oj...@chromium.org> wrote:
TL;DR: Adding a "Missing" entry + including the -expected.txt for the platform your working on is current best practice AFAIK.


Yes :).

Nils Barth

unread,
May 7, 2013, 2:00:55 AM5/7/13
to Ojan Vafai, blink-dev
Ojan Vafai:
3. Easy short-term: Add a "NeedsRebaseline" keyword that will basically map to "Failure ImageOnlyFailure Missing" so you can add a simple line to TestExpectations and so it's clear that this line was just added so the bots could cycle and you do your rebaseline (as a bonus, this avoids all the silly "# needs rebaseline" comments).

3 should be super easy to implement. Just need a volunteer willing to write some python.

I like "super easy" (and useful) and "write some Python" (instead of "read some Perl"), and I've been working on tests lately, so I'd be delighted to take at least this part on; will follow-up off-list.

Nils Barth

unread,
May 7, 2013, 2:02:59 AM5/7/13
to Ojan Vafai, blink-dev
Ojan Vafai:
3. Easy short-term: Add a "NeedsRebaseline" keyword that will basically map to "Failure ImageOnlyFailure Missing" so you can add a simple line to TestExpectations and so it's clear that this line was just added so the bots could cycle and you do your rebaseline (as a bonus, this avoids all the silly "# needs rebaseline" comments).

3 should be super easy to implement. Just need a volunteer willing to write some python.

Hi Ojan, I'm happy to take this one!
* Where should I go in the code to hack on this, and
* are there any existing bugs I should refer to?
Reply all
Reply to author
Forward
0 new messages