Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate wpt #13966 #10

Closed
wants to merge 5 commits into from
Closed

Duplicate wpt #13966 #10

wants to merge 5 commits into from

Conversation

lukebjerring
Copy link
Owner

jgraham and others added 5 commits November 7, 2018 14:52
Traditionally testharness tests ran in a auxillary browsing context
opened using window.open and with access to the opener. This works
well because the long-lived nature of the opener helps to avoid some
of the race conditions that would otherwise occur. But it doesn't work
*that* well; the recent refactor to stop continually focusing the
opener broke tests that alter document.domain or otherwise prevent the
opener being same-domain with the test window. And future platform
features may cause the opener to be nulled out entirely so even a
postMessage based fix wouldn't work.

To solve all of this, this patch refactors things so that the initial
window doesn't contain any logic at all and is just used to keep the
browser alive between tests. Most of the logic moves to
testharnessreport.js which is loaded once per test. In order to get
the right timeout when timeout_multiplier is set this requires an
addition to the product API in wptrunner to expose a function for
getting the timeout multiplier. This allows us to get the
timeout_multiplier for testharness tests upfront and avoids the need
to change the content of testharnessreport when we start running
testharness tests, or to restart the server for each test type.

The main issue with the single-window implementation is that we need
to start injecting script once the test page has loaded
testharnessreport.js. For most browsers we are able to use
pageLoadStrategy=eager to control this; in that case we can start
running tests once DOMContentLoaded is reached. Chrome doesn't support
this pageLoadStrategy, however, so we have to fake support with custom
script.
@lukebjerring lukebjerring deleted the wptrunner_tlbc branch December 11, 2018 14:46
lukebjerring pushed a commit that referenced this pull request May 6, 2019
chromedriver doesn't allow changing Object.prototype to add enumerable
properties, but this test requires setting some values on
Object.prototype.  When Object.prototype.a is set to:

  {b: {c: 'on proto'}}

chromedriver fails with:

    JavascriptErrorException: javascript error (500): Maximum call stack size exceeded
      (Session info: chrome=72.0.3626.121)

    Remote-end stacktrace:

    #0 0x563ff3a32a59 <unknown>
    #1 0x563ff39cb7f3 <unknown>
    #2 0x563ff38fcd7c <unknown>
    #3 0x563ff38ff78c <unknown>
    #4 0x563ff38ff5f7 <unknown>
    #5 0x563ff38ffbe7 <unknown>
    #6 0x563ff38fff1b <unknown>
    #7 0x563ff38a3f7a <unknown>
    #8 0x563ff3899bf2 <unknown>
    #9 0x563ff38a37b7 <unknown>
    #10 0x563ff3899ac3 <unknown>
    #11 0x563ff38782d2 <unknown>
    #12 0x563ff3879112 <unknown>
    #13 0x563ff39fe865 <unknown>
    #14 0x563ff39ff32b <unknown>
    web-platform-tests#15 0x563ff39ff70c <unknown>
    web-platform-tests#16 0x563ff39d940a <unknown>
    web-platform-tests#17 0x563ff39ff997 <unknown>
    web-platform-tests#18 0x563ff39e9947 <unknown>
    web-platform-tests#19 0x563ff3a1a800 <unknown>
    web-platform-tests#20 0x563ff3a3c8be <unknown>
    web-platform-tests#21 0x7f3bf4545494 start_thread
    web-platform-tests#22 0x7f3bf2d58a8f clone

    Ran 1 tests finished in 2.0 seconds.
      • 0 ran as expected. 0 tests skipped.
      • 1 tests had errors unexpectedly

Work around this problem by cleaning up the test environment so
Object.prototype no longer has the override by the time chromedriver
tries to inspect the test result.

While here, fix the other tests to use the t.add_cleanup() function
so they'll cleanup their test environment in case they exit in
some other way besides reaching t.done().

The underlying chromedriver issue is tracked upstream at
https://crbug.com/chromedriver/2555.

Bug: 934844
Change-Id: Id1b4ab2a908bfbc001e2a2d045eeec3ef01c24d9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants