New issue
Advanced search Search tips

Issue 709028 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 705125



Sign in to add a comment

Layout test .../user-timing/test_user_timing_clearMarks.html is flaky (result line order is non-deterministic)

Project Member Reported by ellyjo...@chromium.org, Apr 6 2017

Issue description

This test is flaky on the Mac webkit test bots:

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.webkit%2FWebKit_Mac10.11__retina_%2F14433%2F%2B%2Frecipes%2Fsteps%2Fwebkit_tests%2F0%2Fstdout

It fails with "text diff" but no further diagnostics. To blink for triage.
 
Blocking: 705125
Labels: OS-Mac
The test in question is virtual/mojo-loading/http/tests/w3c/webperf/submission/Intel/user-timing/test_user_timing_clearMarks.html

According to crbug.com/705125, it's a testharness.js test with duplicate test names.

Components: -Blink Blink>HTML

Comment 3 by tkent@chromium.org, Apr 7 2017

Components: -Blink>HTML Blink>PerformanceAPIs

Comment 4 by mkwst@chromium.org, Apr 11 2017

Owner: igrigo...@chromium.org
Status: Assigned (was: Untriaged)
Ilya, can you triage this?

Relatedly, can we import the WPT version of these tests rather than duplicating them so that they get deflaked for everyone?
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f52866b40aca4279164a4415b74b71806dd669b8

commit f52866b40aca4279164a4415b74b71806dd669b8
Author: mkwst <mkwst@chromium.org>
Date: Tue Apr 11 14:08:57 2017

Skip flaky 'test_user_timing_clearMarks.html'.

BUG= 709028 ,705125
TBR=rouslan@chromium.org

Review-Url: https://codereview.chromium.org/2815513004
Cr-Commit-Position: refs/heads/master@{#463617}

[modify] https://crrev.com/f52866b40aca4279164a4415b74b71806dd669b8/third_party/WebKit/LayoutTests/TestExpectations

Cc: igrigo...@chromium.org
Owner: panicker@chromium.org
> Relatedly, can we import the WPT version of these tests rather than duplicating them so that they get deflaked for everyone?

Yep, I think that's the right solution here.

Reassigning, as I'm way backlogged.. Shubhie, do you have bandwidth to take a look at this one?

Comment 7 by panicker@google.com, Apr 12 2017

Cc: qyears...@chromium.org panicker@chromium.org
Owner: ----
[sorry I only have consulting bandwidth right now]

Quinten - is this something you might be able to help with?
We want to import WPT tests for user-timing (and some others)

Labels: -OS-Mac Test-Layout
Status: Available (was: Assigned)
Summary: Layout test .../user-timing/test_user_timing_clearMarks.html is flaky (result line order is non-deterministic) (was: test_user_timing_clearMarks.html flaky on mac webkit_tests)
The wpt/user-timing tests are currently imported:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/user-timing/

The test file that looks like it corresponds to this one, https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/user-timing/clear_all_marks.any.js, is an ".any.js" test which means that it's actually two tests -- one ending in .any.worker.html and one ending in .html.

Enabling these tests is  bug 653514  (CL https://chromium-review.googlesource.com/c/461722/).

General note about layout tests: The stdio/stdout links on the build pages are usually useless, and to see actual results you need to find the "layout test results" link on the build page (example results: https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Mac10_12/1482/layout-test-results/results.html)

Example diff for this test:
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Mac10_12/1482/layout-test-results/http/tests/w3c/webperf/submission/Intel/user-timing/test_user_timing_clearMarks-pretty-diff.html

From this we can see that it's flaky because the order of the PASS lines is non-deterministic, not because some tests are failing. Which is good news for the test itself.

The test has an -expected.txt file because it has a "test harness error" at the top:

duplicate test names: "entryType should be "mark".", "startTime should greater than 0.", "duration of mark should be 0.", "Entries in entrylist should be in order.", "Entry "abc" should be one that we have set.", "Marks that we cleared should not exist anymore.", "Entry "1" should be one that we have set.", "There should be 2 entries.", "There should be 0 entries."

If that were resolved, it would be non-flaky. Additionally, if we changed the rules for testharness.js test output baseline matching so that order is ignored, then that would also make this non-flaky ( bug 711355 ).

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_tests&tests=http/tests/w3c/webperf/submission/Intel/user-timing/test_user_timing_clearMarks.html

Note that this applies to all platforms, not just mac:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_tests&tests=http/tests/w3c/webperf/submission/Intel/user-timing/test_user_timing_clearMarks.html

Comment 9 by panicker@google.com, Apr 13 2017

Cc: -panicker@chromium.org
Owner: panicker@chromium.org
Thanks for the detailed explanation! 
I'll take a look.
Shubhie, should this be marked Assigned?
 Issue 709410  has been merged into this issue.
Cc: panicker@chromium.org
Labels: -Pri-2 Pri-3
Owner: npm@chromium.org
Nicolas, could you take a look? (not urgent)
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc6b6ceedcc3436bb4484f70d6840a70b8bb8c1e

commit bc6b6ceedcc3436bb4484f70d6840a70b8bb8c1e
Author: Nicolas Pena <npm@chromium.org>
Date: Wed Aug 30 17:10:30 2017

Add more text to deduplicate messages in clearMarks test

This CL allows http/tests/w3c/webperf/resources/webperftestharnessextension.js
to include a test description so that testharness messages are deduplicated in
test_user_timing_clearMarks.html.

Link to test output: https://gist.github.com/npm1/089f864992955d2f17ffc1a3799a47a3

Bug:  chromium:709028 
Change-Id: Ib58681658d6b6c3a00a82462702a8869d5b28877
Reviewed-on: https://chromium-review.googlesource.com/641645
Commit-Queue: Nicolás Peña <npm@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498510}
[modify] https://crrev.com/bc6b6ceedcc3436bb4484f70d6840a70b8bb8c1e/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/bc6b6ceedcc3436bb4484f70d6840a70b8bb8c1e/third_party/WebKit/LayoutTests/http/tests/w3c/webperf/resources/webperftestharnessextension.js
[delete] https://crrev.com/edc71b56838145bca6a67069241ec70c8e630d9d/third_party/WebKit/LayoutTests/http/tests/w3c/webperf/submission/Intel/user-timing/test_user_timing_clearMarks-expected.txt
[modify] https://crrev.com/bc6b6ceedcc3436bb4484f70d6840a70b8bb8c1e/third_party/WebKit/LayoutTests/http/tests/w3c/webperf/submission/Intel/user-timing/test_user_timing_clearMarks.html

Comment 14 by npm@chromium.org, Sep 1 2017

Status: Fixed (was: Available)

Sign in to add a comment