imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1.html is a flaky timeout

• Component: Text
• Status: RESOLVED DUPLICATE

[Alexey Proskuryakov]

imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1.html frequently times out on the bots.

I suspect that it's a bug in the harness.

Easily reproducible for me with a debug build:

run-webkit-tests imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1.html --repeat 100 --no-build -v -f --no-retry

[Alexey Proskuryakov]

No, not a harness error.

When the test fails, document.fonts.ready promise just never gets resolved:

  window.addEventListener("load", function() {
    document.fonts.ready.then(function() {
      //  This doesn't get called!
      window.setTimeout(runTests, 250);
    });
  });

My theory is that the promise never gets fulfilled if the fonts finish loading before onload, which sounds like an actual WebKit bug.

[Alexey Proskuryakov]

*** This bug has been marked as a duplicate of bug 158884 ***

[Frédéric Wang (:fredw)]

mmh, so maybe until bug 158884 is fixed we will have to do something like this (not really ideal) workaround:

window.addEventListener("load", function() {
  window.setTimeout(runTests, T);
});

where T is large enough to be sure that the fonts are loaded and not too large to avoid exceeding the harness timeout...

[Alexey Proskuryakov]

The ideal workaround is something like this:

  var loaded = false;
  var fontsLoaded = false;

  window.addEventListener("load", function() {
    loaded = true;
    if (fontsLoaded)
      runTests();
  });
  document.fonts.ready.then(function() {
    fontsLoaded = true;
    if (loaded)
      runTests();
  });

But it's an imported test, so we can't really modify it.

[Frédéric Wang (:fredw)]

(In reply to comment #4)
> But it's an imported test, so we can't really modify it.

Indeed, the ideal would be not to change it. However, I already had to tweak the test to modify the resource paths and add this setTimeOut hack (which should not be needed per the spec, see bug 158884 comment 1).

[youenn fablet]

(In reply to comment #5)
> (In reply to comment #4)
> > But it's an imported test, so we can't really modify it.
> 
> Indeed, the ideal would be not to change it.

Ideally, if it is getting imported once, we should be able to reimport the tests again and again. Tests should be able to improve, like source code.

Is there a git repository, like for W3C wpt/css test suites?
If so, WebKit changes should be contributed to that repo.

For WPT tests, when I need to change a test, I add a comment like: //WEBKIT change:... to the WebKit test.

I then upstream the changes to W3C repo.
When I reimport the tests from W3C repo (once a while), the only remaining change is the removal of the // WEBKIT comment.

[Frédéric Wang (:fredw)]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > But it's an imported test, so we can't really modify it.
> > 
> > Indeed, the ideal would be not to change it.
> 
> Ideally, if it is getting imported once, we should be able to reimport the
> tests again and again. Tests should be able to improve, like source code.
> 
> Is there a git repository, like for W3C wpt/css test suites?
> If so, WebKit changes should be contributed to that repo.
> 
> For WPT tests, when I need to change a test, I add a comment like: //WEBKIT
> change:... to the WebKit test.
> 
> I then upstream the changes to W3C repo.
> When I reimport the tests from W3C repo (once a while), the only remaining
> change is the removal of the // WEBKIT comment.

Yes, the repo is at https://github.com/MathML/MathMLinHTML5-tests for now and indeed I agree with you that it is great if browser vendors contribute there when there are mistakes in the test. The change discussed here is actually a workaround for font loading bug in WebKit (bug 158884). So my point is that it is fine to do a small hack so that we can run the actual MathML test but ideally we should not deviate too much from upstream and even less contribute back such hacks ;-). The best remains to fix bug 158884, so that the test passes when we open http://tests.mathml-association.org/mathml/presentation-markup/scripts/subsup-parameters-1.html in WebKit :-)

BTW, this test also has another change to disable the SuperscriptShiftUpCramped part until bug 156401 is fixed.

Finally, the path to the fonts has also been modified to be executed... That also seems to be what is done in some tests for LayoutTests/imported/w3c/canvas when loading the CanvasTest.ttf font...

[youenn fablet]

> Finally, the path to the fonts has also been modified to be executed... That
> also seems to be what is done in some tests for
> LayoutTests/imported/w3c/canvas when loading the CanvasTest.ttf font...

This was an issue for w3c/canvas.
Many of these tests are now in LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-canvas-element/
At that location, the tests are mostly unmodified (modifications are handled by the test importer).

I am not sure how applicable it would be for your test repo, but you might want to use the same test importer for the mathml repo.
Ideally, you should only need to add a new test repository entry in LayoutTests/imported/w3c/resources/TestRepositories.
Maybe add some import rules in LayoutTests/imported/w3c/resources/ImportExpectations.
Then, you could run Tools/Scripts/import-w3c-tests.

[Frédéric Wang (:fredw)]

(In reply to comment #8)
> I am not sure how applicable it would be for your test repo, but you might
> want to use the same test importer for the mathml repo.
> Ideally, you should only need to add a new test repository entry in
> LayoutTests/imported/w3c/resources/TestRepositories.
> Maybe add some import rules in
> LayoutTests/imported/w3c/resources/ImportExpectations.
> Then, you could run Tools/Scripts/import-w3c-tests.

Thanks it's good to know that this is already covered, I'll give it a try when I have time. Note that the plan is to submit these tests to the W3C in the long term: https://github.com/MathML/MathMLinHTML5-tests/issues/1

[Alexey Proskuryakov]

Seems like the test can be unmarked now, given that the original is fixed.