disabling tests when they fail, and the coverage we get

[Paweł Hajdan, Jr.]

I'm worried about disabling tests when they fail, and it seems to happen especially often for webkit rolls. Just an example from today: http://codereview.chromium.org/7187013/, http://codereview.chromium.org/7193022/ .

tl;dr - how are WebKit rolls different? Usually when tests fail we revert the change, and not disable the tests.

In my opinion this is just wrong and we're losing coverage this way (I wonder how many tests got disabled this way and are still disabled). The idea seems to be to revert breaking changes, rather than disabling tests.

Of course WebKit rolls have to happen regularly, or we get even further behind. But shouldn't we identify the breaking WebKit revision and revert it, rather than disabling tests? That breaking revision has to be identified anyway to re-enable the tests.

Also, do we have all affected tests running on the relevant WebKit canary bots (something that'd take Chromium trunk and WebKit trunk and see if it explodes)?

[Jonathan Dixon]

On 17 June 2011 13:14, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

I'm worried about disabling tests when they fail, and it seems to happen especially often for webkit rolls. Just an example from today: http://codereview.chromium.org/7187013/, http://codereview.chromium.org/7193022/ .

tl;dr - how are WebKit rolls different? Usually when tests fail we revert the change, and not disable the tests.

AIUI (but IANAWD) webkit patches can be harder to land in the first place, and may be disrupting other webkits devs to keep rolling them in and out, so if the chromium side issue is broadly understood and under control we can take up the slack all on our side.

It looks like the specific changes in question are both covered by bugs (http://code.google.com/p/chromium/issues/detail?id=86346 http://code.google.com/p/chromium/issues/detail?id=86397) with a request on them to not roll-out the webkit patch and explanation they can't be looked at until Monday.

Your concern is if these sort of bugs just stack up one on another indefinitely. But when the webkit patch author is actively involved in them as here, that doesn't seem such a problem?

(one minor hitch was one of those patches missed the BUG= link, but was easy to find in the diff itself in this case)

 

In my opinion this is just wrong and we're losing coverage this way (I wonder how many tests got disabled this way and are still disabled). The idea seems to be to revert breaking changes, rather than disabling tests.

Of course WebKit rolls have to happen regularly, or we get even further behind. But shouldn't we identify the breaking WebKit revision and revert it, rather than disabling tests? That breaking revision has to be identified anyway to re-enable the tests.

Also, do we have all affected tests running on the relevant WebKit canary bots (something that'd take Chromium trunk and WebKit trunk and see if it explodes)?

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Stephen White]

AFAIK, we do not have any WebKit canaries running pyauto tests, which are the affected tests in this case.  If we don't have coverage upstream, experience shows that eventually a WebKit roll will break downstream which requires some painful bisecting by the gardener.  However, I believe Adam is on top of it in this case:  http://code.google.com/p/chromium/issues/detail?id=86397

I'm not sure how often this will occur in practice, but I've logged a bug for getting pyauto test coverage on the WebKit canaries:  http://code.google.com/p/chromium/issues/detail?id=86499

Stephen 

[Dirk Pranke]

AFAIK webkit rolls are not usually treated any differently than any
other commit. If a roll breaks a bot, you usually revert it unless
there's a good reason not to.

-- Dirk