New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 662172 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Some testharness.js tests that use promise_test now have error messages.

Project Member Reported by qyears...@chromium.org, Nov 3 2016

Issue description

With the latest version of testharness.js (being added in https://codereview.chromium.org/2468053002),
some layout tests that use testharness.js started getting more error messages when run.

Specifically, tests that use promise_test are now failing with an error message sometimes ("assert_not_equals: got disallowed value undefined"), where at some point it appears that an object that testharness.js expected to be Promise was actually undefined.

The specific change to testharness.js that caused this is: https://github.com/w3c/testharness.js/pull/220.

Tests that started having some failures:

  http/tests/budget/interfaces.html
  http/tests/credentialmanager/passwordcredential-fetch.html
  http/tests/fetch/serviceworker/request-base-https-other-https.html
  http/tests/fetch/serviceworker/request.html
  http/tests/fetch/window/request-base-https-other-https.html
  http/tests/fetch/window/request.html
  http/tests/fetch/workers/request-base-https-other-https.html
  http/tests/fetch/workers/request.html
  http/tests/notifications/serviceworkerregistration-service-worker-fetch-resources.html
  http/tests/streams/readable-streams/count-queuing-strategy-integration.html
  media/encrypted-media/encrypted-media-unique-origin.html
  nfc/push.html

Archived test results:
https://storage.googleapis.com/chromium-layout-test-archives/linux_trusty_blink_rel/1370/layout-test-results/results.html

yhirano@, for the http/tests/fetch tests, could you see if this is something that could be fixed in the test? I'm not sure whether this new error message indicates something that could be improved about the tests or not.
 
I ran into this in some of my own tests. In all cases it was a bug in the test, where the test was not correctly returning a promise, despite using promise_tests.

The count-queuing-strategy-integration.html one has already been fixed upstream in web platform tests.

Comment 2 by tkent@chromium.org, Nov 4 2016

Cc: jrumm...@chromium.org harkness@chromium.org alexande...@intel.com mkwst@chromium.org mvanouwe...@chromium.org
Add harkness@ for http/tests/budget/interfaces.html
Add mkwst@ for http/tests/credentialmanager/passwordcredential-fetch.html
Add mvanouwerkerk@ for http/tests/notifications/serviceworkerregistration-service-worker-fetch-resources.html
Add jrummell@ for media/encrypted-media/encrypted-media-unique-origin.html
Add alexander.shalamov@intel.com for nfc/push.html

Comment 3 by tkent@chromium.org, Nov 4 2016

Components: -Blink>LayoutTests Internals>Media>Encrypted Blink>SecurityFeature
looking at nfc/push.html
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af2af12b8a17d3176148b15ce17e5236879ae60a

commit af2af12b8a17d3176148b15ce17e5236879ae60a
Author: harkness <harkness@chromium.org>
Date: Fri Nov 04 16:17:10 2016

Remove outdated interface tests which were hidden by promise_test behaviour.

A recent change to testharness.js (https://codereview.chromium.org/2468053002)
causes promise_test to throw a TypeError exception when the test returns
something other than a promise. Previously, the test could hide test
failures if the asynchronous test failures resolved after the default
promise created by promise_test.

The change to promise_test behaviour caused the budget service's interface.html
test to start failing because it did not return a promise, but it also
exposed the root problem that outdated code should have been failing
already.

This CL removes the interfaces-expected.txt which reported the promise_test
misuse, and also removes the outdated code.

BUG= 662172 

Review-Url: https://codereview.chromium.org/2479813003
Cr-Commit-Position: refs/heads/master@{#429907}

[delete] https://crrev.com/59f175e074584e056425e97224eae073eabd7cdd/third_party/WebKit/LayoutTests/http/tests/budget/interfaces-expected.txt
[modify] https://crrev.com/af2af12b8a17d3176148b15ce17e5236879ae60a/third_party/WebKit/LayoutTests/http/tests/budget/interfaces.html

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eeafa338d2075075df0760d14935c5c9f3102190

commit eeafa338d2075075df0760d14935c5c9f3102190
Author: harkness <harkness@chromium.org>
Date: Fri Nov 04 16:45:57 2016

Fix notification test to return a promise.

A recent change to testharness.js (https://codereview.chromium.org/2468053002)
causes promise_test to throw a TypeError exception when the test returns
something other than a promise.

This CL fixes one notification test which was not explicitly returning the
promise.

BUG= 662172 

Review-Url: https://codereview.chromium.org/2478153003
Cr-Commit-Position: refs/heads/master@{#429915}

[delete] https://crrev.com/2e52a5a282f509f48e397626f5fcd30f5ab8b1fe/third_party/WebKit/LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-fetch-resources-expected.txt
[modify] https://crrev.com/eeafa338d2075075df0760d14935c5c9f3102190/third_party/WebKit/LayoutTests/http/tests/notifications/serviceworkerregistration-service-worker-fetch-resources.html

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/239d2c25b718e99695655af2c39577dd86d605c6

commit 239d2c25b718e99695655af2c39577dd86d605c6
Author: jrummell <jrummell@chromium.org>
Date: Fri Nov 04 22:08:01 2016

EME: Fix promise_test() to actually return a promise.

Latest testharness.js checks that promise_test() actually returns
something, so add missing return to the test.

BUG= 662172 
TEST=updated test still passes

Review-Url: https://codereview.chromium.org/2470363004
Cr-Commit-Position: refs/heads/master@{#430029}

[delete] https://crrev.com/233aec8779df5588b55e244650fb9962fe1978dd/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin-expected.txt
[modify] https://crrev.com/239d2c25b718e99695655af2c39577dd86d605c6/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin.html

Comment 9 by tkent@chromium.org, Nov 8 2016

Owner: yhirano@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/744439105b8e5e25f8ff915bf2d2dde429e7dc53

commit 744439105b8e5e25f8ff915bf2d2dde429e7dc53
Author: domenic <domenic@chromium.org>
Date: Thu Nov 10 03:00:43 2016

Start importing streams web platform tests

This will allow us to benefit from upstream fixes and avoid duplicating
the tests into the tree.

BUG= 662172 

Review-Url: https://codereview.chromium.org/2488083002
Cr-Commit-Position: refs/heads/master@{#431156}

[modify] https://crrev.com/744439105b8e5e25f8ff915bf2d2dde429e7dc53/third_party/WebKit/LayoutTests/W3CImportExpectations

Cc: -mvanouwe...@chromium.org
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fda5dd03cc24a00c1977f9d85d3c425ebf2be9d9

commit fda5dd03cc24a00c1977f9d85d3c425ebf2be9d9
Author: yhirano <yhirano@chromium.org>
Date: Wed Dec 28 02:33:45 2016

Fix a promise_test misuse in http/tests/fetch/ layout tests

BUG= 662172 

Review-Url: https://codereview.chromium.org/2591753003
Cr-Commit-Position: refs/heads/master@{#440824}

[modify] https://crrev.com/fda5dd03cc24a00c1977f9d85d3c425ebf2be9d9/third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/request.js
[delete] https://crrev.com/f1ebd4aac27a8b10650afd19f28fef936143ced6/third_party/WebKit/LayoutTests/http/tests/fetch/serviceworker/request-base-https-other-https-expected.txt
[delete] https://crrev.com/f1ebd4aac27a8b10650afd19f28fef936143ced6/third_party/WebKit/LayoutTests/http/tests/fetch/serviceworker/request-expected.txt
[delete] https://crrev.com/f1ebd4aac27a8b10650afd19f28fef936143ced6/third_party/WebKit/LayoutTests/http/tests/fetch/window/request-base-https-other-https-expected.txt
[delete] https://crrev.com/f1ebd4aac27a8b10650afd19f28fef936143ced6/third_party/WebKit/LayoutTests/http/tests/fetch/window/request-expected.txt
[delete] https://crrev.com/f1ebd4aac27a8b10650afd19f28fef936143ced6/third_party/WebKit/LayoutTests/http/tests/fetch/workers/request-base-https-other-https-expected.txt
[delete] https://crrev.com/f1ebd4aac27a8b10650afd19f28fef936143ced6/third_party/WebKit/LayoutTests/http/tests/fetch/workers/request-expected.txt

Status: Fixed (was: Assigned)

Sign in to add a comment