Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(556)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2679543005: Remove flaky expectations with update-test-expectations script (Closed)

Created:
3 years, 10 months ago by qyearsley
Modified:
3 years, 10 months ago
Reviewers:
bokan, ojan
CC:
blink-reviews, chromium-reviews, jeffcarp
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove flaky expectations with update-test-expectations script The update-test-expectations script is meant to remove flaky expectations when it appears that according to the most recent results on the flakiness dashboard for that test, the test is no longer flaky. BUG=491764, 498539, 575766, 600248, 658305, 663838, 663840, 663848, 663851, 663853, 663872, 663874, 663877, 663879, 664817, 664839, 664840, 664841, 664842, 664846, 664850, 664855, 664856, 666991, 671480, 671618, 672204, 673296, 673632, 674720, 674858, 678488, 683800 Review-Url: https://codereview.chromium.org/2679543005 Cr-Commit-Position: refs/heads/master@{#448663} Committed: https://chromium.googlesource.com/chromium/src/+/b034403ab50b3a27bda35d6cf72efec57f2f1217

Patch Set 1 #

Patch Set 2 : Update expectations #

Patch Set 3 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -144 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 15 chunks +2 lines, -144 lines 0 comments Download

Messages

Total messages: 21 (13 generated)
qyearsley
Trying another run of update-test-expectations. I looked through the flakiness dashboard for all of these; ...
3 years, 10 months ago (2017-02-06 19:47:08 UTC) #3
ojan
lgtm I <3 this script!
3 years, 10 months ago (2017-02-07 05:44:22 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2679543005/20001
3 years, 10 months ago (2017-02-07 05:44:32 UTC) #11
commit-bot: I haz the power
Failed to apply patch for third_party/WebKit/LayoutTests/TestExpectations: While running git apply --index -p1; error: patch failed: ...
3 years, 10 months ago (2017-02-07 05:48:24 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2679543005/40001
3 years, 10 months ago (2017-02-07 16:46:30 UTC) #16
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/b034403ab50b3a27bda35d6cf72efec57f2f1217
3 years, 10 months ago (2017-02-07 17:51:32 UTC) #19
bokan
On 2017/02/06 19:47:08, qyearsley wrote: > Trying another run of update-test-expectations. > > I looked ...
3 years, 10 months ago (2017-02-07 19:09:41 UTC) #20
qyearsley
3 years, 10 months ago (2017-02-07 19:46:31 UTC) #21
Message was sent while issue was closed. 
On 2017/02/07 at 19:09:41, bokan wrote:
> On 2017/02/06 19:47:08, qyearsley wrote:
> > Trying another run of update-test-expectations.
> > 
> > I looked through the flakiness dashboard for all of these; some of the
> > relatively interesting tests (that aren't just all pass) are:
> > 
> >
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType...
> 
> These are "flaky" in the sense that they timeout/fail occasionally, but never
meet the "three-in-a-row" threshold to mark it as a failure, right?
> 
> > The only line that I think the script shouldn't have removed but removed
anyway
> > was a directory:
> > 
> > crbug.com/683800 [ Win7 Debug ] external/wpt/selection/ [ Failure Pass ]
> > 
> > I think the script shouldn't have removed this because I think it wasn't
able to
> > check the results for all of the tests under that directory.
> 
> Is it something special about this directory? I don't recall actually
supporting a whole directory being flaky, perhaps that needs to be added to the
script?

In general in TestExpectations, whole directories are supported, and the
expectation applies to all tests within that directory. This should generally be
discouraged, I think, but it's possible. Now, to properly support such
directories, we would want to check results for all tests within the directory,
and remove the line iff all subtests are not flaky. Filed bug
http://crbug.com/689642.

Powered by Google App Engine
This is Rietveld 408576698