Changeset 202609 in webkit

Timestamp:

Jun 28, 2016 9:17:36 PM (8 years ago)

Author:

ddkilzer@apple.com

Message:

Enhance shouldNotThrow()/shouldThrow() to accept functions and a descriptive message
<​https://webkit.org/b/159232>

Reviewed by Brent Fulgham.

Based on a Blink change (patch by <​hongchan@chromium.org>):
<​https://src.chromium.org/viewvc/blink?view=revision&revision=192204>

Currently, shouldNotThrow() and shouldThrow() take the following
arguments:

shouldNotThrow(evalString)
shouldThrow(evalString, expectedExceptionString)

The challenges with this design are:

1) The 'evalString' must capture every variable that it

needs, which means the code can be long, and concatenated
into a single line. It would be really nice to be able
to use an (anonymous) function to capture variables
instead.

2) The 'evalString' is literally printed out in the test

results, which isn't always the most descriptive. A
descriptive message would make it clearer what failed.

3) When changing a shouldThrow() into a shouldNotThrow()

or copying/pasting code, it's possible to forget to
remove 'expectedExceptionString' from the function call.

This patch changes the methods to take the following arguments:

shouldNotThrow(evalString|function [, message])
shouldThrow(evalString|function, expectedExceptionString [, message])

If 'function' is passed in, then it is invoked instead of
evaluated, and 'message' replaces the literal code in the
pass/fail output.

This patch also adds the global 'didFailSomeTests' variable to
js-test.js, which already exists in js-test-pre.js. This was
added to js-test-pre.js in r153203 by Oliver Hunt to
LayoutTests/fast/js/resources/js-test-pre.js.

• fast/canvas/webgl/canvas-supports-context-expected.txt:
• fast/canvas/webgl/gl-bind-attrib-location-before-compile-test-expected.txt:
• fast/css-grid-layout/grid-element-auto-repeat-get-set-expected.txt:
• fast/dom/getElementsByClassName/ASCII-case-insensitive-expected.txt:
• storage/indexeddb/cursor-basics-expected.txt:
• storage/indexeddb/cursor-basics-private-expected.txt:
• Update expected results to include "Some tests fail." since some subtests actually do fail during these tests.

• fast/css/parsing-css-lang.html:
• fast/css/parsing-css-matches-1.html:
• fast/css/parsing-css-matches-2.html:
• fast/css/parsing-css-matches-3.html:
• fast/css/parsing-css-matches-4.html:
• fast/css/parsing-css-not-1.html:
• fast/css/parsing-css-not-2.html:
• fast/css/parsing-css-not-3.html:
• fast/css/parsing-css-not-4.html:
• fast/css/parsing-css-nth-child-of-1.html:
• fast/css/parsing-css-nth-child-of-2.html:
• fast/css/parsing-css-nth-last-child-of-1.html:
• fast/css/parsing-css-nth-last-child-of-2.html:
• js/script-tests/arrowfunction-supercall.js:
• Remove expectedExceptionString from shouldNotThrow() calls after they were changed from shouldThrow() calls.

• resources/js-test-pre.js:

(shouldNotThrow): Change to invoke first argument if it is a
function, else use eval() as before. Use second argurment in
place of first argument (if set) when printing results. NOTE:
Care was taken not to add any lines of code to prevent changes
to test results.
(shouldThrow): Ditto. Reformat code.

• resources/js-test.js: Declare 'didFailSomeTests'.

(testFailed): Set 'didFailSomeTests' to true when a test fails.
(shouldNotThrow): Same changes as js-test-pre.js.
(shouldThrow): Ditto.
(isSuccessfullyParsed): Output a message if 'didFailSomeTests'
is true.

Location:

trunk/LayoutTests

Files:

• ChangeLog (modified) (1 diff)
• fast/canvas/webgl/canvas-supports-context-expected.txt (modified) (1 diff)
• fast/canvas/webgl/gl-bind-attrib-location-before-compile-test-expected.txt (modified) (1 diff)
• fast/css-grid-layout/grid-element-auto-repeat-get-set-expected.txt (modified) (1 diff)
• fast/css/parsing-css-lang.html (modified) (1 diff)
• fast/css/parsing-css-matches-1.html (modified) (1 diff)
• fast/css/parsing-css-matches-2.html (modified) (1 diff)
• fast/css/parsing-css-matches-3.html (modified) (1 diff)
• fast/css/parsing-css-matches-4.html (modified) (1 diff)
• fast/css/parsing-css-not-1.html (modified) (1 diff)
• fast/css/parsing-css-not-2.html (modified) (1 diff)
• fast/css/parsing-css-not-3.html (modified) (1 diff)
• fast/css/parsing-css-not-4.html (modified) (1 diff)
• fast/css/parsing-css-nth-child-of-1.html (modified) (1 diff)
• fast/css/parsing-css-nth-child-of-2.html (modified) (1 diff)
• fast/css/parsing-css-nth-last-child-of-1.html (modified) (1 diff)
• fast/css/parsing-css-nth-last-child-of-2.html (modified) (1 diff)
• fast/dom/getElementsByClassName/ASCII-case-insensitive-expected.txt (modified) (1 diff)
• js/script-tests/arrowfunction-supercall.js (modified) (1 diff)
• resources/js-test-pre.js (modified) (1 diff)
• resources/js-test.js (modified) (4 diffs)
• storage/indexeddb/cursor-basics-expected.txt (modified) (1 diff)
• storage/indexeddb/cursor-basics-private-expected.txt (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-06-28  David Kilzer  <ddkilzer@apple.com>
+
+        Enhance shouldNotThrow()/shouldThrow() to accept functions and a descriptive message
+        <https://webkit.org/b/159232>
+
+        Reviewed by Brent Fulgham.
+
+        Based on a Blink change (patch by <hongchan@chromium.org>):
+        <https://src.chromium.org/viewvc/blink?view=revision&revision=192204>
+
+        Currently, shouldNotThrow() and shouldThrow() take the following
+        arguments:
+
+            shouldNotThrow(evalString)
+            shouldThrow(evalString, expectedExceptionString)
+
+        The challenges with this design are:
+
+            1) The 'evalString' must capture every variable that it
+               needs, which means the code can be long, and concatenated
+               into a single line.  It would be really nice to be able
+               to use an (anonymous) function to capture variables
+               instead.
+            2) The 'evalString' is literally printed out in the test
+               results, which isn't always the most descriptive.  A
+               descriptive message would make it clearer what failed.
+            3) When changing a shouldThrow() into a shouldNotThrow()
+               or copying/pasting code, it's possible to forget to
+               remove 'expectedExceptionString' from the function call.
+
+        This patch changes the methods to take the following arguments:
+
+            shouldNotThrow(evalString|function [, message])
+            shouldThrow(evalString|function, expectedExceptionString [, message])
+
+        If 'function' is passed in, then it is invoked instead of
+        evaluated, and 'message' replaces the literal code in the
+        pass/fail output.
+
+        This patch also adds the global 'didFailSomeTests' variable to
+        js-test.js, which already exists in js-test-pre.js.  This was
+        added to js-test-pre.js in r153203 by Oliver Hunt to
+        LayoutTests/fast/js/resources/js-test-pre.js.
+
+        * fast/canvas/webgl/canvas-supports-context-expected.txt:
+        * fast/canvas/webgl/gl-bind-attrib-location-before-compile-test-expected.txt:
+        * fast/css-grid-layout/grid-element-auto-repeat-get-set-expected.txt:
+        * fast/dom/getElementsByClassName/ASCII-case-insensitive-expected.txt:
+        * storage/indexeddb/cursor-basics-expected.txt:
+        * storage/indexeddb/cursor-basics-private-expected.txt:
+        - Update expected results to include "Some tests fail." since
+          some subtests actually do fail during these tests.
+
+        * fast/css/parsing-css-lang.html:
+        * fast/css/parsing-css-matches-1.html:
+        * fast/css/parsing-css-matches-2.html:
+        * fast/css/parsing-css-matches-3.html:
+        * fast/css/parsing-css-matches-4.html:
+        * fast/css/parsing-css-not-1.html:
+        * fast/css/parsing-css-not-2.html:
+        * fast/css/parsing-css-not-3.html:
+        * fast/css/parsing-css-not-4.html:
+        * fast/css/parsing-css-nth-child-of-1.html:
+        * fast/css/parsing-css-nth-child-of-2.html:
+        * fast/css/parsing-css-nth-last-child-of-1.html:
+        * fast/css/parsing-css-nth-last-child-of-2.html:
+        * js/script-tests/arrowfunction-supercall.js:
+        - Remove expectedExceptionString from shouldNotThrow() calls
+          after they were changed from shouldThrow() calls.
+
+        * resources/js-test-pre.js:
+        (shouldNotThrow): Change to invoke first argument if it is a
+        function, else use eval() as before.  Use second argurment in
+        place of first argument (if set) when printing results.  NOTE:
+        Care was taken not to add any lines of code to prevent changes
+        to test results.
+        (shouldThrow): Ditto.  Reformat code.
+        * resources/js-test.js: Declare 'didFailSomeTests'.
+        (testFailed): Set 'didFailSomeTests' to true when a test fails.
+        (shouldNotThrow): Same changes as js-test-pre.js.
+        (shouldThrow): Ditto.
+        (isSuccessfullyParsed): Output a message if 'didFailSomeTests'
+        is true.
+
 2016-06-28  Jiewen Tan  <jiewen_tan@apple.com>
 

trunk/LayoutTests/fast/canvas/webgl/canvas-supports-context-expected.txt

@@ -116 +116 @@
 
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE

trunk/LayoutTests/fast/canvas/webgl/gl-bind-attrib-location-before-compile-test-expected.txt

@@ -24 +24 @@
 
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE

trunk/LayoutTests/fast/css-grid-layout/grid-element-auto-repeat-get-set-expected.txt

@@ -58 +58 @@
 PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-columns') is "none"
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE

trunk/LayoutTests/fast/css/parsing-css-lang.html

@@ -12 +12 @@
 
 function testValidLanguageRange(languageRangeString, expectedSerializedLanguageRange) {
-    shouldNotThrow('document.querySelector(\':lang(' + languageRangeString.replace(/\\/g, '\\\\') + ')\')', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(\':lang(' + languageRangeString.replace(/\\/g, '\\\\') + ')\')');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-matches-1.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-matches-2.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-matches-3.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-matches-4.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":matches(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-not-1.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-not-2.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-not-3.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-not-4.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":not(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-nth-child-of-1.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":nth-child(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":nth-child(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-nth-child-of-2.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":nth-child(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":nth-child(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-nth-last-child-of-1.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":nth-last-child(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":nth-last-child(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/css/parsing-css-nth-last-child-of-2.html

@@ -12 +12 @@
 
 function testValidSelector(selectorString, expectedSerializedSelector) {
-    shouldNotThrow('document.querySelector(":nth-last-child(' + selectorString.replace(/\\/g, '\\\\') + ')")', '"Error: SyntaxError: DOM Exception 12"');
+    shouldNotThrow('document.querySelector(":nth-last-child(' + selectorString.replace(/\\/g, '\\\\') + ')")');
 
     var styleContainer = document.getElementById('style-container');

trunk/LayoutTests/fast/dom/getElementsByClassName/ASCII-case-insensitive-expected.txt

@@ -4 +4 @@
 PASS getByClassName('Ä') is "Ä"
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE

trunk/LayoutTests/js/script-tests/arrowfunction-supercall.js

@@ -152 +152 @@
 shouldThrow('(new class extends A { constructor() { ((a = this)=>{ return a; })() } })', '"ReferenceError: Cannot access uninitialized variable."');
 shouldThrow('(new class extends A { constructor() { ((a = this, b=super())=>{ return a; })() } })', '"ReferenceError: Cannot access uninitialized variable."');
-shouldNotThrow('(new class extends A { constructor() { ((a = new.target)=>{ return a; })(); super(); } })', '"ReferenceError: Cannot access uninitialized variable."');
-shouldNotThrow('(new class extends A { constructor() { ((a = new.target, b=super())=>{ return a; })() } })', '"ReferenceError: Cannot access uninitialized variable."');
+shouldNotThrow('(new class extends A { constructor() { ((a = new.target)=>{ return a; })(); super(); } })');
+shouldNotThrow('(new class extends A { constructor() { ((a = new.target, b=super())=>{ return a; })() } })');
 
 var successfullyParsed = true;

trunk/LayoutTests/resources/js-test-pre.js

@@ -584 +584 @@
 }
 
-function shouldNotThrow(_a) {
+function shouldNotThrow(_a, _message) {
     try {
-        eval(_a);
-        testPassed(_a + " did not throw exception.");
+        typeof _a == "function" ? _a() : eval(_a);
+        testPassed((_message ? _message : _a) + " did not throw exception.");
     } catch (e) {
-        testFailed(_a + " should not throw exception. Threw exception " + e + ".");
-    }
-}
-
-function shouldThrow(_a, _e)
-{
-  var exception;
-  var _av;
-  try {
-     _av = eval(_a);
-  } catch (e) {
-     exception = e;
-  }
-
-  var _ev;
-  if (_e)
-      _ev =  eval(_e);
-
-  if (exception) {
-    if (typeof _e == "undefined" || exception == _ev)
-      testPassed(_a + " threw exception " + exception + ".");
+        testFailed((_message ? _message : _a) + " should not throw exception. Threw exception " + e + ".");
+    }
+}
+
+function shouldThrow(_a, _e, _message)
+{
+    var _exception;
+    var _av;
+    try {
+        _av = typeof _a == "function" ? _a() : eval(_a);
+    } catch (e) {
+        _exception = e;
+    }
+
+    var _ev;
+    if (_e)
+        _ev = eval(_e);
+
+    if (_exception) {
+        if (typeof _e == "undefined" || _exception == _ev)
+            testPassed((_message ? _message : _a) + " threw exception " + _exception + ".");
+        else
+            testFailed((_message ? _message : _a) + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Threw exception " + _exception + ".");
+    } else if (typeof _av == "undefined")
+        testFailed((_message ? _message : _a) + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was undefined.");
     else
-      testFailed(_a + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Threw exception " + exception + ".");
-  } else if (typeof _av == "undefined")
-    testFailed(_a + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was undefined.");
-  else
-    testFailed(_a + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was " + _av + ".");
+        testFailed((_message ? _message : _a) + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was " + _av + ".");
 }
 

trunk/LayoutTests/resources/js-test.js

@@ -11 +11 @@
 }
 
-var description, debug, successfullyParsed;
+var description, debug, didFailSomeTests, successfullyParsed;
+
+didFailSomeTests = false;
 
 var expectingError; // set by shouldHaveError()
@@ -154 +156 @@
 function testFailed(msg)
 {
+    didFailSomeTests = true;
     debug('<span><span class="fail">FAIL</span> ' + escapeHTML(msg) + '</span>');
 }
@@ -560 +563 @@
 }
 
-function shouldNotThrow(_a) {
+function shouldNotThrow(_a, _message) {
     try {
-        eval(_a);
-        testPassed(_a + " did not throw exception.");
+        typeof _a == "function" ?  _a() : eval(_a);
+        testPassed((_message ? _message : _a) + " did not throw exception.");
     } catch (e) {
-        testFailed(_a + " should not throw exception. Threw exception " + e + ".");
-    }
-}
-
-function shouldThrow(_a, _e)
-{
-  var _exception;
-  var _av;
-  try {
-     _av = eval(_a);
-  } catch (e) {
-     _exception = e;
-  }
-
-  var _ev;
-  if (_e)
-      _ev = eval(_e);
-
-  if (_exception) {
-    if (typeof _e == "undefined" || _exception == _ev)
-      testPassed(_a + " threw exception " + _exception + ".");
+        testFailed((_message ? _message : _a) + " should not throw exception. Threw exception " + e + ".");
+    }
+}
+
+function shouldThrow(_a, _e, _message)
+{
+    var _exception;
+    var _av;
+    try {
+        _av = typeof _a == "function" ? _a() : eval(_a);
+    } catch (e) {
+        _exception = e;
+    }
+
+    var _ev;
+    if (_e)
+        _ev = eval(_e);
+
+    if (_exception) {
+        if (typeof _e == "undefined" || _exception == _ev)
+            testPassed((_message ? _message : _a) + " threw exception " + _exception + ".");
+        else
+            testFailed((_message ? _message : _a) + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Threw exception " + _exception + ".");
+    } else if (typeof _av == "undefined")
+        testFailed((_message ? _message : _a) + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was undefined.");
     else
-      testFailed(_a + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Threw exception " + _exception + ".");
-  } else if (typeof _av == "undefined")
-    testFailed(_a + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was undefined.");
-  else
-    testFailed(_a + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was " + _av + ".");
+        testFailed((_message ? _message : _a) + " should throw " + (typeof _e == "undefined" ? "an exception" : _ev) + ". Was " + _av + ".");
 }
 
@@ -692 +695 @@
     successfullyParsed = !unexpectedErrorMessage;
     shouldBeTrue("successfullyParsed");
+    if (didFailSomeTests)
+        debug("Some tests failed.");
     debug('<br /><span class="pass">TEST COMPLETE</span>');
 }

trunk/LayoutTests/storage/indexeddb/cursor-basics-expected.txt

@@ -83 +83 @@
 PASS 'value' in cursor is false
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE

trunk/LayoutTests/storage/indexeddb/cursor-basics-private-expected.txt

@@ -83 +83 @@
 PASS 'value' in cursor is false
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE