Bug 189063 – REGRESSION(r235408): GTK bots exiting early

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

The GTK bots are exiting early. I see many timeouts in certain animation and compositing tests, CSS3 tests, and editing tests:

https://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20(Tests)/r235437%20(7940)/results.html

The regression was introduced between r235407-r235410, inclusive. Most likely r235408 or r235409.

[Michael Catanzaro]

There are also ten failing webkitperl tests, which is unusual as these never fail. These may or may not be related, I don't know:

Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl .................................. Experimental keys on scalar is now forbidden at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262/Runner.pm line 929.
Compilation failed in require at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.
BEGIN failed--compilation aborted at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.

Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl .................................. 1/13 
#   Failed test 'test262 test failed, ignore expectations (exit code: 1)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 136.
#          got: '255'
#     expected: '1'

#   Failed test 'test262 test failed, ignore expectations (new failures: 2)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 142.
#          got: '0'
#     expected: '2'
Experimental keys on scalar is now forbidden at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262/Runner.pm line 929.
Compilation failed in require at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.
BEGIN failed--compilation aborted at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.

#   Failed test 'test262 test passed, ignore expectations (exit code: 0)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 136.
#          got: '255'
#     expected: '0'
Experimental keys on scalar is now forbidden at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262/Runner.pm line 929.
Compilation failed in require at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.
BEGIN failed--compilation aborted at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.

#   Failed test 'test262 tests newly failed (exit code: 1)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 136.
#          got: '255'
#     expected: '1'

#   Failed test 'test262 tests newly failed (new failures: 2)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 142.
#          got: '0'
#     expected: '2'
Experimental keys on scalar is now forbidden at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262/Runner.pm line 929.
Compilation failed in require at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.
BEGIN failed--compilation aborted at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.

#   Failed test 'test262 tests newly passed (exit code: 0)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 136.
#          got: '255'
#     expected: '0'
Experimental keys on scalar is now forbidden at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262/Runner.pm line 929.
Compilation failed in require at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.
BEGIN failed--compilation aborted at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.

#   Failed test 'test262 tests fails, expected failure (exit code: 0)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 136.
#          got: '255'
#     expected: '0'
Experimental keys on scalar is now forbidden at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262/Runner.pm line 929.
Compilation failed in require at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.
BEGIN failed--compilation aborted at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.

#   Failed test 'test262 tests fails, with unexpected error string (exit code: 1)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 136.
#          got: '255'
#     expected: '1'

#   Failed test 'test262 tests fails, with unexpected error string (new failures: 2)'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 142.
#          got: '0'
#     expected: '2'
Experimental keys on scalar is now forbidden at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262/Runner.pm line 929.
Compilation failed in require at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.
BEGIN failed--compilation aborted at /home/buildbot/wpe/wpe-linux-64-release-tests/build/Tools/Scripts/test262-runner line 45.

#   Failed test 'expectations yaml file format'
#   at Tools/Scripts/webkitperl/test262_unittest/test262-runner-tests.pl line 157.
# Looks like you failed 10 tests of 13.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #1)
> There are also ten failing webkitperl tests, which is unusual as these never
> fail. These may or may not be related, I don't know:

Well, they started failing in the same range of commits.

[Simon Fraser (smfr)]

You're sure it wasn't https://trac.webkit.org/changeset/235399/webkit ?

[Simon Fraser (smfr)]

The perl tests started failing here: https://trac.webkit.org/changeset/235359/webkit

[Michael Catanzaro]

(In reply to Simon Fraser (smfr) from comment #3)
> You're sure it wasn't https://trac.webkit.org/changeset/235399/webkit ?

Hm, that commit is mighty suspicious, but we do have successful test runs at r235402 and r235406, on two different bots. So I don't think so, not unless old versions of the test runner are somehow being used by mistake.

(In reply to Simon Fraser (smfr) from comment #4)
> The perl tests started failing here:
> https://trac.webkit.org/changeset/235359/webkit

Ooops, I misread the test results page. So that is surely unrelated. (Do you want to report the bug for it, or shall I?)

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #5)
> (In reply to Simon Fraser (smfr) from comment #3)
> > You're sure it wasn't https://trac.webkit.org/changeset/235399/webkit ?
> 
> Hm, that commit is mighty suspicious, but we do have successful test runs at
> r235402 and r235406, on two different bots. So I don't think so, not unless
> old versions of the test runner are somehow being used by mistake.

I've tested r235406 locally and verified it's good. Tested r235408 and verified it's bad. The only change in r235407 is guarded by PLATFORM(COCOA). So we can pin this on r235408.

The tests are printing on stderr:

FAIL: TestControllerRunLoop timed out.

Looking at the commit, I don't immediately notice anything problematic, so I'll need to attempt to debug why some particular test is stalling.

I did spot one unrelated bug in the commit, in Tools/DumpRenderTree/win/DumpRenderTree.cpp:

    // DumpRenderTree does not support checking for abandonded documents.
    if (!strcmp("#CHECK FOR ABANDONED DOCUMENTS", command)) {

I noticed that "abandonded" was misspelled. Then I noticed that the command here is different than everywhere else. Should probably be:

    // DumpRenderTree does not support checking for world leaks.
    if (!strcmp("#CHECK FOR WORLD LEAKS", command)) {

[Michael Catanzaro]

*** Bug 189079 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

Happy to help out if we can be on irc at the same time.

[Michael Catanzaro]

(In reply to Simon Fraser (smfr) from comment #8)
> Happy to help out if we can be on irc at the same time.

Thanks... it's been a busy week and next week will be too, so probably not anytime soon unfortunately.

> I did spot one unrelated bug in the commit, in
> Tools/DumpRenderTree/win/DumpRenderTree.cpp:
> 
>     // DumpRenderTree does not support checking for abandonded documents.
>     if (!strcmp("#CHECK FOR ABANDONED DOCUMENTS", command)) {
> 
> I noticed that "abandonded" was misspelled. Then I noticed that the command
> here is different than everywhere else. Should probably be:
> 
>     // DumpRenderTree does not support checking for world leaks.
>     if (!strcmp("#CHECK FOR WORLD LEAKS", command)) {

Don't forget about this. ^

[Simon Fraser (smfr)]

(In reply to Michael Catanzaro from comment #9)
> (In reply to Simon Fraser (smfr) from comment #8)
> > Happy to help out if we can be on irc at the same time.
> 
> Thanks... it's been a busy week and next week will be too, so probably not
> anytime soon unfortunately.
> 
> > I did spot one unrelated bug in the commit, in
> > Tools/DumpRenderTree/win/DumpRenderTree.cpp:
> > 
> >     // DumpRenderTree does not support checking for abandonded documents.
> >     if (!strcmp("#CHECK FOR ABANDONED DOCUMENTS", command)) {
> > 
> > I noticed that "abandonded" was misspelled. Then I noticed that the command
> > here is different than everywhere else. Should probably be:
> > 
> >     // DumpRenderTree does not support checking for world leaks.
> >     if (!strcmp("#CHECK FOR WORLD LEAKS", command)) {
> 
> Don't forget about this. ^

Fixed in r235408.

[Claudio Saavedra]

I debugged this a bit today, have some findings that might shed some light.

- This only happens if you run a series of tests sharing the DumpRendererTree. If you use --run-singly, the tests pass. So it probably has to be with the remainder state of DumpRendererTree.

- I noticed that there's a 20_s in the AsyncTask that is added for the world leaks in the offending commit. I'm not familiar with this but I'm guessing that this is the timeout for the task. The timeout for the tests is 5 s, AFAIR, so it's probable that the world leaks task is timing out. I increased the tests timeout when running them locally to 20s, and then the tests pass. Similarly, if I change the 20_s to 5_s, the tests also pass.

- But what I don't understand is why this task is being called at all? Isn't it so that --world-leaks needs to be called explicitly for the check to run? What I see is that this is done unconditionally in willDestroyWebView(). Is this intended?

[Claudio Saavedra]

(In reply to Claudio Saavedra from comment #11)

> What I see is that this is done unconditionally in willDestroyWebView(). Is
> this intended?

Apologies in advance if this is gross, but just not doing this unconditional call also allows the tests to pass without timing out.

[Simon Fraser (smfr)]

(In reply to Claudio Saavedra from comment #12)
> (In reply to Claudio Saavedra from comment #11)
> 
> > What I see is that this is done unconditionally in willDestroyWebView(). Is
> > this intended?
> 
> Apologies in advance if this is gross, but just not doing this unconditional
> call also allows the tests to pass without timing out.

Oh, that call in DestroyWebView should not be unconditional!

[Simon Fraser (smfr)]

Created attachment 348851 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 348851 [details]
Patch

Clearing flags on attachment: 348851

Committed r235646: <https://trac.webkit.org/changeset/235646>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Claudio Saavedra]

Glad to have helped!