Bug 186638 - [GTK] Tests that are failing on the EWS test queue
Summary: [GTK] Tests that are failing on the EWS test queue
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-14 17:13 PDT by Carlos Alberto Lopez Perez
Modified: 2018-06-19 14:48 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2018-06-14 17:13:36 PDT
On r232844 <https://trac.webkit.org/r232844> we enabled tests on the GTK EWS test queue.
Since that this are some of the tests failing on the queue https://webkit-queues.webkit.org/queue-status/gtk-wk2-ews

printing/print-close-crash.html [ Timeout ]
fast/media/print-restores-previous-mediatype.html [ Timeout ]
fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
editing/execCommand/print.html [ Timeout ]
animations/play-state-paused.html [ Failure ]
imported/w3c/web-platform-tests/streams/piping/error-propagation-forward.html [ Failure Pass ]
http/tests/misc/cached-scripts.html [ Crash Pass ]
imported/w3c/web-platform-tests/FileAPI/FileReader/workers.html [ Crash Pass ]
imported/mozilla/svg/dynamic-stroke-opacity-01.svg [ Crash Pass ]
imported/mozilla/css-transitions/test_animation-finished.html [ Timeout Pass ]
imported/blink/fast/text/international/complex-text-space-in-second-font.html [ Crash Pass ]
fast/mediastream/change-tracks-media-stream-being-played.html [ Crash Pass ]
compositing/repaint/repaint-on-layer-grouping-change.html [ Failure  Pass ]
Comment 1 Carlos Alberto Lopez Perez 2018-06-14 17:19:16 PDT
Expectations updated on https://trac.webkit.org/changeset/232861
Comment 2 Carlos Alberto Lopez Perez 2018-06-14 17:44:18 PDT
Some of this failures may have been caused by some revision on the range [232841-232860] that is the interval between almost zero-failures (when testing on the EWS was enabled) and the above gardening commit
Comment 3 Carlos Alberto Lopez Perez 2018-06-14 18:56:00 PDT
More expectations updated on https://trac.webkit.org/changeset/232865
Comment 4 Michael Catanzaro 2018-06-14 19:07:10 PDT
(In reply to Carlos Alberto Lopez Perez from comment #1)
> Expectations updated on https://trac.webkit.org/changeset/232861

You added them to the part of the file where we keep the "expected" expected failures (desirable failures or tests for unimplemented features)... they should be moved.
Comment 5 Michael Catanzaro 2018-06-15 10:13:36 PDT
I noticed a huge number of issues today that were caused by the new expectations added here.

I've audited all the crash expectations added here. There are no recent recorded crashes on the GTK release bot for most of the tests with added crash expectations. For two of the tests, there are recorded crashes, but I investigated and determined they were both random events caused by the memory pressure handler. In my judgment, none of these should have crash expectations. If they are producing a real crash backtrace on the EWS, we should file separate bug reports for those with a backtrace.

The only new crash expectation I am not removing are these two:

fast/mediastream/change-tracks-media-stream-being-played.html
media/video-currentTime-delay.html

These two I will split out into new bugs.

I also audited all three of the new non-flaky timeouts. In each case, the tests are not timing out on the release bot, and in fact have never timed out recently. In these cases, since we do know they've recently timed out on EWS and can't blame the memory pressure monitor, I'll add Pass expectations rather than removing the new expectations.

In the case of the non-flaky failure added for animations/play-state-paused.html, the test does have one recent failure, but it almost always passes. I'll add a  Pass expectation.

(In reply to Michael Catanzaro from comment #4)
> You added them to the part of the file where we keep the "expected" expected
> failures (desirable failures or tests for unimplemented features)... they
> should be moved.

I've move them.
Comment 6 Carlos Alberto Lopez Perez 2018-06-15 12:53:13 PDT
And another attempt at making the EWS happy landed in r232889: <https://trac.webkit.org/changeset/232889>
Comment 7 Michael Catanzaro 2018-06-15 15:18:40 PDT
I feel like we're taking two steps forward, one step back. :)

I've landed another follow-up in https://trac.webkit.org/changeset/232895, partially reverting r232889. I want to make sure these tests are really crashing (i.e. have real backtraces and associated bug reports, and aren't just victims of the memory pressure limit you removed earlier today).

I'll watch the EWS later tonight to see if it reports any more crashes from these tests, but note the EWS seems to be broken now (not running any tests).
Comment 8 Carlos Alberto Lopez Perez 2018-06-18 04:14:55 PDT
(In reply to Michael Catanzaro from comment #7)
> I feel like we're taking two steps forward, one step back. :)
> 

And I feel like unless you are willing to compromise on marking all the flaky tests we are seeing on the EWS (even without a crash log), we will have to revert this back ASAP.

> I've landed another follow-up in https://trac.webkit.org/changeset/r,
> partially reverting r232889. I want to make sure these tests are really
> crashing (i.e. have real backtraces and associated bug reports, and aren't
> just victims of the memory pressure limit you removed earlier today).
> 

I'm pretty sure they are not. I ensured to update the EWS manually to r232882 and restarted it manually before looking at the changes I commited into r232888.

> I'll watch the EWS later tonight to see if it reports any more crashes from
> these tests, but note the EWS seems to be broken now (not running any tests).

I see it broken all the time because its unable to pass tests, but I can see the output of each text run at https://webkit-queues.webkit.org/queue-status/gtk-wk2-ews
Comment 9 Carlos Alberto Lopez Perez 2018-06-18 04:19:38 PDT
(In reply to Carlos Alberto Lopez Perez from comment #8)

> I see it broken all the time because its unable to pass tests, but I can see
> the output of each text run at
> https://webkit-queues.webkit.org/queue-status/gtk-wk2-ews

I stand corrected.. there was something wrong with the test step there.. trying to fix it now.
Comment 10 Carlos Alberto Lopez Perez 2018-06-18 04:44:31 PDT
(In reply to Carlos Alberto Lopez Perez from comment #9)
> (In reply to Carlos Alberto Lopez Perez from comment #8)
> 
> > I see it broken all the time because its unable to pass tests, but I can see
> > the output of each text run at
> > https://webkit-queues.webkit.org/queue-status/gtk-wk2-ews
> 
> I stand corrected.. there was something wrong with the test step there..
> trying to fix it now.

Fixed.. but issue remains the same.. random crashes everywhere:

https://webkit-queues.webkit.org/results/8230086


Regressions: Unexpected crashes (3)
  fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
  http/tests/misc/cached-scripts.html [ Crash ]
  legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]
Comment 11 Carlos Alberto Lopez Perez 2018-06-18 05:56:13 PDT
(In reply to Carlos Alberto Lopez Perez from comment #10)

> Regressions: Unexpected crashes (3)
>   fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
>   http/tests/misc/cached-scripts.html [ Crash ]
>   legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]

Crash log for fast/hidpi/broken-image-icon-very-hidpi.html obtained on the EWS bot: http://termbin.com/a4of
Comment 12 Carlos Alberto Lopez Perez 2018-06-18 05:57:54 PDT
(In reply to Carlos Alberto Lopez Perez from comment #11)
> (In reply to Carlos Alberto Lopez Perez from comment #10)
> 
> > Regressions: Unexpected crashes (3)
> >   fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
> >   http/tests/misc/cached-scripts.html [ Crash ]
> >   legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]
> 
> Crash log for fast/hidpi/broken-image-icon-very-hidpi.html obtained on the
> EWS bot: http://termbin.com/a4of

stderr: g_bytes_get_size: assertion 'bytes != NULL' failed
Comment 13 Carlos Alberto Lopez Perez 2018-06-18 06:18:52 PDT
(In reply to Carlos Alberto Lopez Perez from comment #12)
> (In reply to Carlos Alberto Lopez Perez from comment #11)
> > (In reply to Carlos Alberto Lopez Perez from comment #10)
> > 
> > > Regressions: Unexpected crashes (3)
> > >   fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
> > >   http/tests/misc/cached-scripts.html [ Crash ]
> > >   legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]
> > 
> > Crash log for fast/hidpi/broken-image-icon-very-hidpi.html obtained on the
> > EWS bot: http://termbin.com/a4of
> 
> stderr: g_bytes_get_size: assertion 'bytes != NULL' failed

Ok.. I have determined that this one is caused because the package gnome-icon-theme is not installed in the EWS machine (but it is on the bot).
We should add this to the install-dependencies script
Comment 14 Michael Catanzaro 2018-06-18 07:18:45 PDT
(In reply to Carlos Alberto Lopez Perez from comment #11)
> (In reply to Carlos Alberto Lopez Perez from comment #10)
> 
> > Regressions: Unexpected crashes (3)
> >   fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
> >   http/tests/misc/cached-scripts.html [ Crash ]
> >   legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]

Yes, as expected. I was planning to watch the EWS this weekend to try to find backtraces for the crashing tests, but it was broken all weekend. Anyway, I intend to help with this. I don't think it's too much effort to take the backtrace and report a bug when EWS finds a crashing test.

> Crash log for fast/hidpi/broken-image-icon-very-hidpi.html obtained on the
> EWS bot: http://termbin.com/a4of

Weird... although you should probably install gnome-icon-theme, I think this is a WebKit bug worth reporting. Our current code assumes that we have missingImage icons in the GResource, which is apparently not true. See here:

static Ref<Image> loadMissingImageIconFromTheme(const char* name)
{
    // ...
    if (iconInfo) {
        // Is the icon installed?
        return WTFMove(icon);
    }

    return loadImageFromGResource(name);
}

That's not right, we shouldn't continue on to loadImageFromGResource if the image is not guaranteed to be in the resource.
Comment 15 Michael Catanzaro 2018-06-18 07:28:46 PDT
Looking over the current state of the EWS queue, I think we are both wrong here:

(In reply to Carlos Alberto Lopez Perez from comment #8) 
> And I feel like unless you are willing to compromise on marking all the
> flaky tests we are seeing on the EWS (even without a crash log), we will
> have to revert this back ASAP.

We won't have to do this, because I don't see any recent bugs where our EWS went red, even though it is fixed and running tests again. At least, there are no such bugs currently displayed on the queue page.

But getting backtraces is much harder than I expected, because the EWS does not attach test results unless it goes red. It is impossible to get a backtrace from just the results log. We can still get a backtrace each time the EWS actually goes red, but it would be nice to have access to test results for every run. :/
Comment 16 Adrian Perez 2018-06-18 07:51:26 PDT
(In reply to Michael Catanzaro from comment #14)
> (In reply to Carlos Alberto Lopez Perez from comment #11)
> > (In reply to Carlos Alberto Lopez Perez from comment #10)
> > 
> > > Regressions: Unexpected crashes (3)
> > >   fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
> > >   http/tests/misc/cached-scripts.html [ Crash ]
> > >   legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]
> 
> Yes, as expected. I was planning to watch the EWS this weekend to try to
> find backtraces for the crashing tests, but it was broken all weekend.
> Anyway, I intend to help with this. I don't think it's too much effort to
> take the backtrace and report a bug when EWS finds a crashing test.
> 
> > Crash log for fast/hidpi/broken-image-icon-very-hidpi.html obtained on the
> > EWS bot: http://termbin.com/a4of
> 
> Weird... although you should probably install gnome-icon-theme, I think this
> is a WebKit bug worth reporting. Our current code assumes that we have
> missingImage icons in the GResource, which is apparently not true. See here:
> 
> static Ref<Image> loadMissingImageIconFromTheme(const char* name)
> {
>     // ...
>     if (iconInfo) {
>         // Is the icon installed?
>         return WTFMove(icon);
>     }
> 
>     return loadImageFromGResource(name);
> }
> 
> That's not right, we shouldn't continue on to loadImageFromGResource if the
> image is not guaranteed to be in the resource.

I think that WebKitGTK+ should include within itself a copy of the
resources it uses which currently end up being loaded from the GNOME
theme, to avoid the runtime dependency on the “gnome-icon-theme”
package. Using icons which are part of GTK+ itself would still be
fine, of course. Should we open a new bug for this?
Comment 17 Carlos Alberto Lopez Perez 2018-06-18 08:20:44 PDT
(In reply to Adrian Perez from comment #16)
> (In reply to Michael Catanzaro from comment #14)
> > (In reply to Carlos Alberto Lopez Perez from comment #11)
> > > (In reply to Carlos Alberto Lopez Perez from comment #10)
> > > 
> > > > Regressions: Unexpected crashes (3)
> > > >   fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
> > > >   http/tests/misc/cached-scripts.html [ Crash ]
> > > >   legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]
> > 
> > Yes, as expected. I was planning to watch the EWS this weekend to try to
> > find backtraces for the crashing tests, but it was broken all weekend.
> > Anyway, I intend to help with this. I don't think it's too much effort to
> > take the backtrace and report a bug when EWS finds a crashing test.
> > 
> > > Crash log for fast/hidpi/broken-image-icon-very-hidpi.html obtained on the
> > > EWS bot: http://termbin.com/a4of
> > 
> > Weird... although you should probably install gnome-icon-theme, I think this
> > is a WebKit bug worth reporting. Our current code assumes that we have
> > missingImage icons in the GResource, which is apparently not true. See here:
> > 
> > static Ref<Image> loadMissingImageIconFromTheme(const char* name)
> > {
> >     // ...
> >     if (iconInfo) {
> >         // Is the icon installed?
> >         return WTFMove(icon);
> >     }
> > 
> >     return loadImageFromGResource(name);
> > }
> > 
> > That's not right, we shouldn't continue on to loadImageFromGResource if the
> > image is not guaranteed to be in the resource.
> 
> I think that WebKitGTK+ should include within itself a copy of the
> resources it uses which currently end up being loaded from the GNOME
> theme, to avoid the runtime dependency on the “gnome-icon-theme”
> package. Using icons which are part of GTK+ itself would still be
> fine, of course. Should we open a new bug for this?

Yes, please.
Comment 18 Adrian Perez 2018-06-18 08:35:02 PDT
(In reply to Carlos Alberto Lopez Perez from comment #17)
> (In reply to Adrian Perez from comment #16)
>
> > [...]
> > 
> > I think that WebKitGTK+ should include within itself a copy of the
> > resources it uses which currently end up being loaded from the GNOME
> > theme, to avoid the runtime dependency on the “gnome-icon-theme”
> > package. Using icons which are part of GTK+ itself would still be
> > fine, of course. Should we open a new bug for this?
> 
> Yes, please.

Done, that's bug #186767
Comment 19 Michael Catanzaro 2018-06-18 11:19:23 PDT
(In reply to Carlos Alberto Lopez Perez from comment #10) 
> Regressions: Unexpected crashes (3)
>   fast/hidpi/broken-image-icon-very-hidpi.html [ Crash ]
>   http/tests/misc/cached-scripts.html [ Crash ]
>   legacy-animation-engine/fast/css/getFloatValueForUnit.html [ Crash ]

I've pushed expectations for the first two (bug #176767 and bug #186778). Let's keep watching for legacy-animation-engine/fast/css/getFloatValueForUnit.html and any others.