Bug 158993 - imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1.html is a flaky timeout
Summary: imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters...
Status: RESOLVED DUPLICATE of bug 158884
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-21 11:16 PDT by Alexey Proskuryakov
Modified: 2016-07-07 17:22 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2016-06-21 11:16:39 PDT
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
Comment 1 Alexey Proskuryakov 2016-06-21 11:53:30 PDT
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.
Comment 2 Alexey Proskuryakov 2016-06-21 11:59:11 PDT

*** This bug has been marked as a duplicate of bug 158884 ***
Comment 3 Frédéric Wang (:fredw) 2016-06-21 12:13:49 PDT
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...
Comment 4 Alexey Proskuryakov 2016-06-21 12:16:37 PDT
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.
Comment 5 Frédéric Wang (:fredw) 2016-06-21 12:35:18 PDT
(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).
Comment 6 youenn fablet 2016-06-21 15:12:11 PDT
(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.
Comment 7 Frédéric Wang (:fredw) 2016-06-21 21:48:18 PDT
(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...
Comment 8 youenn fablet 2016-06-22 01:47:31 PDT
> 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.
Comment 9 Frédéric Wang (:fredw) 2016-06-22 01:57:55 PDT
(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
Comment 10 Alexey Proskuryakov 2016-07-07 17:22:05 PDT
Seems like the test can be unmarked now, given that the original is fixed.