Bug 1489591 [wpt PR 12898] - [testharness.js] Reject non-thenable values, a=testonly
authorjugglinmike <mike@mikepennisi.com>
Tue, 11 Sep 2018 09:59:49 +0000
changeset 436001 c2ff4ef0946b09717162a824fdea1a1d1fd8b635
parent 436000 2734d9b8dd34d1317a652df36643224bd61f4e44
child 436002 3f6470c8ab4782ef9e384bc16dbe032cea19a200
push id34625
push userdvarga@mozilla.com
push dateThu, 13 Sep 2018 02:31:40 +0000
treeherdermozilla-central@51e9e9660b3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1489591, 12898
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1489591 [wpt PR 12898] - [testharness.js] Reject non-thenable values, a=testonly Automatic update from web-platform-tests[testharness.js] Reject non-thenable values (#12898) Previously, any test defined via the `promise_test` API would fail immediately if its body returned the value `undefined`. The test would *not* fail if it returned any other value. Because the motivation for using `promise_test` is to track resolution with a "thenable" value (i.e. an object with a "then" method), this was overly permissive and allowed contributors to write tests which spuriously passed [1]. Update testharness.js to enforce more stringent criteria on the value return by `promise_test` bodies: that it is a "thenable" value. This change is incompatible with an exiting functional test, but it does not effect any tests in WPT (as verified by a survey using both the Chrome and Firefox browsers). Update the functional test accordingly. [1] cca6b6845678d9b5f792b886f3f7045d1d2cf0a7 -- wpt-commits: 86579034357501943927f3dc4abf75d76c477383 wpt-pr: 12898
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/docs/_writing-tests/testharness-api.md
testing/web-platform/tests/resources/test/tests/functional/promise.html
testing/web-platform/tests/resources/testharness.js
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -588436,17 +588436,17 @@
    "7bf9e9aa9e6c6c7a8c057c9dbe4151194ca3edd0",
    "support"
   ],
   "docs/_writing-tests/testdriver.md": [
    "eb9b9fb0413862ab41c2064dec88fc10e3f7611a",
    "support"
   ],
   "docs/_writing-tests/testharness-api.md": [
-   "bb5524532915a58e4fab3c3bb89a41bbe2a46b4a",
+   "952c8365fb7028c00eeca7ee5949310ecce95913",
    "support"
   ],
   "docs/_writing-tests/testharness.md": [
    "9c21452607e8f3f9e5bbf3ef2fb7771441f4a680",
    "support"
   ],
   "docs/_writing-tests/visual.md": [
    "2d46fbe10dd1be3a6e16dcc75c8b2f8d2b695d82",
@@ -636320,17 +636320,17 @@
    "d4c62794c4f77abf460cd484fd548a59e1ed16e3",
    "support"
   ],
   "resources/test/tests/functional/promise-with-sync.html": [
    "234f5476e9cdaf8c388cdaaa2e6464bc9120fe3d",
    "support"
   ],
   "resources/test/tests/functional/promise.html": [
-   "bdf6dc3ec2af07a9799243cbc7b15da939961363",
+   "9db1dec0f9e3e973e57d08f2ebed256b82bbd0ab",
    "support"
   ],
   "resources/test/tests/functional/queue.html": [
    "4ea32a2bc8ee64b5841596f240291ec7fa514274",
    "support"
   ],
   "resources/test/tests/functional/single-page-test-fail.html": [
    "5826a2ef15c00d817197333de1f444cf1ac51e8b",
@@ -636476,17 +636476,17 @@
    "5e8f640c6659d176eaca4c71cc1798b7285540b7",
    "support"
   ],
   "resources/testharness.css.headers": [
    "e828b629858d07afd989b80894986315bac16cc7",
    "support"
   ],
   "resources/testharness.js": [
-   "f0c24635017dad6275c99dc149ab1739470eeb36",
+   "85e211ff60ae559d7ff39994c33a2f05056e1ef2",
    "support"
   ],
   "resources/testharness.js.headers": [
    "5e8f640c6659d176eaca4c71cc1798b7285540b7",
    "support"
   ],
   "resources/testharnessreport.js": [
    "e5cb40fe0ef652be407d4c48b1c59391864cec7b",
--- a/testing/web-platform/tests/docs/_writing-tests/testharness-api.md
+++ b/testing/web-platform/tests/docs/_writing-tests/testharness-api.md
@@ -156,19 +156,19 @@ Test is finished.
 ## Promise Tests ##
 
 `promise_test` can be used to test APIs that are based on Promises:
 
 ```js
 promise_test(test_function, name, properties)
 ```
 
-`test_function` is a function that receives a test as an argument and returns a
-promise. The test completes when the returned promise resolves. The test fails
-if the returned promise rejects.
+`test_function` is a function that receives a test as an argument. It must
+return a promise. The test completes when the returned promise resolves. The
+test fails if the returned promise rejects.
 
 E.g.:
 
 ```js
 function foo() {
   return Promise.resolve("foo");
 }
 
--- a/testing/web-platform/tests/resources/test/tests/functional/promise.html
+++ b/testing/web-platform/tests/resources/test/tests/functional/promise.html
@@ -95,22 +95,26 @@ promise_test(
                   t.unreached_func("Expected failure."));
     },
     "unreached_func as reactor (should FAIL with an expected failure)");
 
 promise_test(
     function() {
         return true;
     },
-    "promise_test with function that doesn't return a Promise");
+    "promise_test with function that doesn't return a Promise (should FAIL)");
 
 promise_test(function(){},
              "promise_test with function that doesn't return anything");
 
 promise_test(
+    function() { return { then: 23 }; },
+    "promise_test that returns a non-thenable (should FAIL)");
+
+promise_test(
     function() {
         return Promise.reject("Expected rejection");
     },
     "promise_test with unhandled rejection (should FAIL)");
 
 promise_test(
     function() {
         return Promise.resolve(10)
@@ -165,25 +169,31 @@ promise_test(
     },
     {
       "status_string": "PASS",
       "name": "Promises are supported in your browser",
       "message": null,
       "properties": {}
     },
     {
-      "status_string": "PASS",
-      "name": "promise_test with function that doesn't return a Promise",
-      "message": null,
+      "status_string": "FAIL",
+      "name": "promise_test with function that doesn't return a Promise (should FAIL)",
+      "message": "promise_test: test body must return a 'thenable' object (received an object with no `then` method)",
       "properties": {}
     },
     {
       "status_string": "FAIL",
       "name": "promise_test with function that doesn't return anything",
-      "message": "assert_not_equals: got disallowed value undefined",
+      "message": "promise_test: test body must return a 'thenable' object (received undefined)",
+      "properties": {}
+    },
+    {
+      "status_string": "FAIL",
+      "name": "promise_test that returns a non-thenable (should FAIL)",
+      "message": "promise_test: test body must return a 'thenable' object (received an object with no `then` method)",
       "properties": {}
     },
     {
       "status_string": "FAIL",
       "name": "promise_test with unhandled exception in fulfill reaction (should FAIL)",
       "message": "promise_test: Unhandled rejection with value: object \"Error: Expected exception.\"",
       "properties": {}
     },
--- a/testing/web-platform/tests/resources/testharness.js
+++ b/testing/web-platform/tests/resources/testharness.js
@@ -571,17 +571,22 @@ policies and contribution forms [3].
         if (!tests.promise_tests) {
             tests.promise_tests = Promise.resolve();
         }
         tests.promise_tests = tests.promise_tests.then(function() {
             return new Promise(function(resolve) {
                 var promise = test.step(func, test, test);
 
                 test.step(function() {
-                    assert_not_equals(promise, undefined);
+                    assert(!!promise, "promise_test", null,
+                           "test body must return a 'thenable' object (received ${value})",
+                           {value:promise});
+                    assert(typeof promise.then === "function", "promise_test", null,
+                           "test body must return a 'thenable' object (received an object with no `then` method)",
+                           null);
                 });
 
                 // Test authors may use the `step` method within a
                 // `promise_test` even though this reflects a mixture of
                 // asynchronous control flow paradigms. The "done" callback
                 // should be registered prior to the resolution of the
                 // user-provided Promise to avoid timeouts in cases where the
                 // Promise does not settle but a `step` function has thrown an