Web Inspector: add InspectorTest.expectException() and use it
authorbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Dec 2017 23:10:00 +0000 (23:10 +0000)
committerbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Dec 2017 23:10:00 +0000 (23:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180719

Reviewed by Matt Baker.

Source/WebInspectorUI:

This helps straighten out async test() functions that need
to test exceptional cases. Without this, every such method
needs try/catch boilerplate to leak the error outside the
scope of the catch-block and then assert that it exists.

* UserInterface/Test/TestHarness.js:
(TestHarness.prototype.expectException):
Added. This method takes a callback which is expected to raise
an exception either by throwing an Error instance or returning
a Promise that rejects with an Error instance. The method returns
a Promise that is either resolved with the expected exception
from one of the above sources, or is rejected with the non-exception
result that was returned or resolved by the callback.

LayoutTests:

* inspector/dom/highlightNode-expected.txt:
* inspector/dom/highlightNode.html:
Adopt the new helper method.

* inspector/unit-tests/test-harness-expect-functions-async-expected.txt: Added.
* inspector/unit-tests/test-harness-expect-functions-async.html: Added.
Add a separate async test suite for async expect* functions.

* inspector/unit-tests/test-harness-expect-functions-expected.txt:
* inspector/unit-tests/test-harness-expect-functions.html:
Standardize the naming for these two suites.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225810 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/dom/highlightNode-expected.txt
LayoutTests/inspector/dom/highlightNode.html
LayoutTests/inspector/unit-tests/test-harness-expect-functions-async-expected.txt [new file with mode: 0644]
LayoutTests/inspector/unit-tests/test-harness-expect-functions-async.html [new file with mode: 0644]
LayoutTests/inspector/unit-tests/test-harness-expect-functions-expected.txt
LayoutTests/inspector/unit-tests/test-harness-expect-functions.html
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Test/TestHarness.js

index d2b9e7c..06deda6 100644 (file)
@@ -1,3 +1,22 @@
+2017-12-12  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: add InspectorTest.expectException() and use it
+        https://bugs.webkit.org/show_bug.cgi?id=180719
+
+        Reviewed by Matt Baker.
+
+        * inspector/dom/highlightNode-expected.txt:
+        * inspector/dom/highlightNode.html:
+        Adopt the new helper method.
+
+        * inspector/unit-tests/test-harness-expect-functions-async-expected.txt: Added.
+        * inspector/unit-tests/test-harness-expect-functions-async.html: Added.
+        Add a separate async test suite for async expect* functions.
+
+        * inspector/unit-tests/test-harness-expect-functions-expected.txt:
+        * inspector/unit-tests/test-harness-expect-functions.html:
+        Standardize the naming for these two suites.
+
 2017-12-12  Myles C. Maxfield  <mmaxfield@apple.com>
 
         REGRESSION (Safari 11): custom <font-face> tag crashes a page
index d6f7e05..f4171cc 100644 (file)
@@ -24,14 +24,14 @@ PASS: Should be one highlighted node.
 Highlighted Element Data: {"tagName":"div","idValue":"id-one","size":{"width":150,"height":250},"role":""}
 
 -- Running test case: MissingNodeAndObjectIdShouldError
-PASS: Should produce an error.
+PASS: Should produce an exception.
 Error: Either nodeId or objectId must be specified
 
 -- Running test case: BadNodeId
-PASS: Should produce an error.
+PASS: Should produce an exception.
 Error: Could not find node with given id
 
 -- Running test case: BadObjectId
-PASS: Should produce an error.
+PASS: Should produce an exception.
 Error: Node for given objectId not found
 
index 06fb6d3..e8c8496 100644 (file)
@@ -111,11 +111,9 @@ function test()
     suite.addTestCase({
         name: "MissingNodeAndObjectIdShouldError",
         description: "Missing identifiers should cause an error.",
-        test(resolve, reject) {
-            DOMAgent.highlightNode(highlightConfig, undefined, undefined, (error) => {
-                InspectorTest.expectThat(error, "Should produce an error.");
-                InspectorTest.log("Error: " + error);
-                resolve();
+        async test() {
+            await InspectorTest.expectException(async () => {
+                await DOMAgent.highlightNode(highlightConfig, undefined, undefined);
             });
         }
     });
@@ -123,11 +121,9 @@ function test()
     suite.addTestCase({
         name: "BadNodeId",
         description: "Bad node id should cause an error.",
-        test(resolve, reject) {
-            DOMAgent.highlightNode(highlightConfig, 9999999, undefined, (error) => {
-                InspectorTest.expectThat(error, "Should produce an error.");
-                InspectorTest.log("Error: " + error);
-                resolve();
+        async test() {
+            await InspectorTest.expectException(async () => {
+                await DOMAgent.highlightNode(highlightConfig, 9999999, undefined);
             });
         }
     });
@@ -135,11 +131,9 @@ function test()
     suite.addTestCase({
         name: "BadObjectId",
         description: "Bad object id should cause an error.",
-        test(resolve, reject) {
-            DOMAgent.highlightNode(highlightConfig, undefined, "bad-object-id", (error) => {
-                InspectorTest.expectThat(error, "Should produce an error.");
-                InspectorTest.log("Error: " + error);
-                resolve();
+        async test() {
+            await InspectorTest.expectException(async () => {
+                await DOMAgent.highlightNode(highlightConfig, undefined, "bad-object-id");
             });
         }
     });
diff --git a/LayoutTests/inspector/unit-tests/test-harness-expect-functions-async-expected.txt b/LayoutTests/inspector/unit-tests/test-harness-expect-functions-async-expected.txt
new file mode 100644 (file)
index 0000000..8215982
--- /dev/null
@@ -0,0 +1,36 @@
+Testing asynchronous TestHarness.expect* functions.
+
+
+== Running test suite: InspectorTest.ExpectFunctions.Async
+-- Running test case: expectException.WorkIsNotAFunction
+PASS: Should produce an exception.
+Error: Invalid argument to catchException: work must be a function.
+
+-- Running test case: expectException.SyncWorkThatThrowsException
+PASS: Should produce an exception.
+Error: A fake exception
+PASS: Returned promise should be resolved with the expected exception.
+PASS: Exception should not be altered.
+
+-- Running test case: expectException.SyncWorkThatDoesNotThrowException
+The following assertion is expected to fail.
+FAIL: Should produce an exception.
+    Expected: not null
+    Actual: null
+PASS: Rejected value should be the returned value.
+
+-- Running test case: expectException.AsyncWorkThatRejects
+PASS: Should produce an exception.
+Error: A fake rejection
+PASS: Returned promise should be resolved with the expected exception.
+PASS: Exception should not be altered.
+
+-- Running test case: expectException.AsyncWorkThatResolves
+The following assertion is expected to fail.
+FAIL: Should produce an exception.
+    Expected: not null
+    Actual: null
+PASS: Should produce an exception.
+42
+FAIL: The returned promise should not resolve since an expected exception was not raised.
+
diff --git a/LayoutTests/inspector/unit-tests/test-harness-expect-functions-async.html b/LayoutTests/inspector/unit-tests/test-harness-expect-functions-async.html
new file mode 100644 (file)
index 0000000..e7df676
--- /dev/null
@@ -0,0 +1,90 @@
+<!doctype html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("InspectorTest.ExpectFunctions.Async");
+    suite.addTestCase({
+        name: "expectException.WorkIsNotAFunction",
+        async test() {
+            InspectorTest.expectException(async () => {
+                await InspectorTest.expectException(42);
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "expectException.SyncWorkThatThrowsException",
+        async test() {
+            let error;
+            await InspectorTest.expectException(() => {
+                error = new Error("A fake exception");
+                throw error;
+            }).then((e) => {
+                InspectorTest.expectNotNull(error, "Returned promise should be resolved with the expected exception.");
+                InspectorTest.expectEqual(error, e, "Exception should not be altered.");
+            }).catch(() => {
+                InspectorTest.fail("The returned promise should not reject since an expected exception was raised.");
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "expectException.SyncWorkThatDoesNotThrowException",
+        async test() {
+            InspectorTest.log("The following assertion is expected to fail.");
+
+            let returnValue = 42;
+            await InspectorTest.expectException(() => {
+                return returnValue;
+            }).then((e) => {
+                InspectorTest.fail("The returned promise should reject since an expected exception was not raised.");
+            }).catch((resolvedValue) => {
+                InspectorTest.expectEqual(resolvedValue, returnValue, "Rejected value should be the returned value.");
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "expectException.AsyncWorkThatRejects",
+        async test() {
+            let error;
+
+            await InspectorTest.expectException(() => {
+                error = new Error("A fake rejection");
+                return Promise.reject(error);
+            }).then((e) => {
+                InspectorTest.expectNotNull(error, "Returned promise should be resolved with the expected exception.");
+                InspectorTest.expectEqual(error, e, "Exception should not be altered.");
+            }).catch(() => {
+                InspectorTest.fail("The returned promise should not reject since an expected exception was raised.");
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "expectException.AsyncWorkThatResolves",
+        async test() {
+            InspectorTest.log("The following assertion is expected to fail.");
+
+            let returnValue = 42;
+            await InspectorTest.expectException(() => {
+                return Promise.resolve(returnValue);
+            }).then(() => {
+                InspectorTest.fail("The returned promise should not resolve since an expected exception was not raised.");
+            }).catch((resolvedValue) => {
+                InspectorTest.expectEqual(resolvedValue, returnValue, "Rejected value should be the returned value.");
+            });
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+    <p>Testing asynchronous TestHarness.expect* functions.</p>
+</body>
+</html>
index f9b20ab..5055d16 100644 (file)
@@ -1,7 +1,7 @@
 Testing TestHarness.expect* family of functions.
 
 
-== Running test suite: InspectorTestExpectFunctions
+== Running test suite: InspectorTest.ExpectFunctions.Sync
 -- Running test case: InspectorTest.expectThat
 Expected to PASS
 PASS: expectThat(true)
index 124305b..cbe66ff 100644 (file)
@@ -5,7 +5,7 @@
 <script>
 function test()
 {
-    let suite = InspectorTest.createSyncSuite("InspectorTestExpectFunctions");
+    let suite = InspectorTest.createSyncSuite("InspectorTest.ExpectFunctions.Sync");
 
     function toArray(a) {
         return a instanceof Array && a.length ? a : [a];
index 3dd2e1d..d8e5fd5 100644 (file)
@@ -1,3 +1,24 @@
+2017-12-12  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: add InspectorTest.expectException() and use it
+        https://bugs.webkit.org/show_bug.cgi?id=180719
+
+        Reviewed by Matt Baker.
+
+        This helps straighten out async test() functions that need
+        to test exceptional cases. Without this, every such method
+        needs try/catch boilerplate to leak the error outside the
+        scope of the catch-block and then assert that it exists.
+
+        * UserInterface/Test/TestHarness.js:
+        (TestHarness.prototype.expectException):
+        Added. This method takes a callback which is expected to raise
+        an exception either by throwing an Error instance or returning
+        a Promise that rejects with an Error instance. The method returns
+        a Promise that is either resolved with the expected exception
+        from one of the above sources, or is rejected with the non-exception
+        result that was returned or resolved by the callback.
+
 2017-12-12  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         Don't require perl(File::Copy::Recursive)
index 7ad586c..920bef1 100644 (file)
@@ -184,6 +184,46 @@ TestHarness = class TestHarness extends WI.Object
         this.log("FAIL: " + stringifiedMessage);
     }
 
+    // Use this to expect an exception. To further examine the exception,
+    // chain onto the result with .then() and add your own test assertions.
+    // The returned promise is rejected if an exception was not thrown.
+    expectException(work)
+    {
+        if (typeof work !== "function")
+            throw new Error("Invalid argument to catchException: work must be a function.");
+
+        let expectAndDumpError = (e) => {
+            this.expectNotNull(e, "Should produce an exception.");
+            if (e)
+                this.log(e.toString());
+        }
+
+        let error = null;
+        let result = null;
+        try {
+            result = work();
+        } catch (caughtError) {
+            error = caughtError;
+        } finally {
+            // If 'work' returns a promise, it will settle (resolve or reject) by itself.
+            // Invert the promise's settled state to match the expectation of the caller.
+            if (result instanceof Promise) {
+                return result.then((resolvedValue) => {
+                    expectAndDumpError(null);
+                    return Promise.reject(resolvedValue);
+                }).catch((e) => {
+                    expectAndDumpError(e);
+                    return Promise.resolve(e);
+                });
+            }
+
+            // If a promise is not produced, turn the exception into a resolved promise, and a
+            // resolved value into a rejected value (since an exception was expected).
+            expectAndDumpError(error);
+            return error ? Promise.resolve(error) : Promise.reject(result);
+        }
+    }
+
     // Protected
 
     static messageAsString(message)