I think this would be something specific to how we run reference tests from web-platform-tests, and probably shouldn't apply to -expected.html tests unless they suffer from the same problem and this is the right fix.

Might the fix be somewhere inside TestRunnerBindings::Install, close to the logic around reftest-wait?

Aren't "-expected.html" tests ref tests? The CL
https://codereview.chromium.org/2706033008
is about the flakiness of a ref test in fast/css3-text.

If you meant by testharness and JS tests, I agree. When tests calls testRunner.waitUntilDone() or setup({ explicit_done: true }), we don't have to--or shouldn't?--do this.

If a test is JS test but doesn't call waitUntilDone(), I'm not sure if we can detect that. Probably waiting for document.fonts.ready is harmless, but as foolip@ said in the public-test-infra@, I'm not sure it doesn't break web font tests that expects to timeout etc.

ksakamoto@ might know better?

Inside LayoutTest/external/wpt/, there's a MANIFEST.json, and in  issue 268729  things were changed such that we no longer rename the references, so there are no *-expected.html files in that subdirectory.

My idea was to do this extra waiting only for things that are considered reftests in MANIFEST.json. But I'd like to hear what jeffcarp@ and qyearsley@ thinks about this.

AFAIK all the our web font tests that test behavior while loading use waitUntilDone(), so waiting document.fonts.ready in all reftests without waitUntilDone() would be OK.

Excellent, it's great to have uncovered a source of flakiness :-D

As foolip@ noted in #11, reference tests in external/wpt are listed in MANIFEST.json along with their reference files... and now that csswg-test is merged into web-platform-tests, this also applies to all of those tests.

Although the way that we run reference tests is the same for wpt and for all of the other layout tests. So the timing of the pixel dumps for reference tests is determined by our test runner; explicitly adding the above snippet using testRunner works for individual layout tests outside of wpt, but we couldn't include anything referencing testRunner in wpt.

kojii@, ksakamoto@, is the next step to try to inject that JS snippet when running all tests? Do we know if this will make running the tests much slower?

> kojii@, ksakamoto@, is the next step to try to inject that JS snippet when running all tests?

I was thinking to change the test runner C++ code to do the equivalent thing as JS, and hoped to find someone familiar with test runner. I didn't think about injecting JS code, but that might work too.

I happened to find BlinkTestRunner::TestFinished() is where we force layout/paint by calling updateAllLifecyclePhases(), and then take screenshots. So somewhere before that, we should wait for document.fonts.ready being resolved either from C++ or by injecting JS?

> Do we know if this will make running the tests much slower?

To my knowledge, I don't think so. It will create a resolved Promise unless fonts are loading, but that should be cheap. If you choose to inject JS, ".then()" will run in the next microtask, but that should be cheap too.

Isn't this an upstream issue for WPT too?

Yes, wptrunner (which isn't how we run LayoutTests/external/wpt) would also need some changes in how reftests are run.