TestHarness should provide a family of assertions/expectThat methods, similar to that provided by XCTest and other test frameworks (JUnit, NUnit, etc). A quick audit of assert/expectThat usage in LayoutTests/inspector reveals the following usage patterns: InspectorTest.assert(!expr) 85 InspectorTest.assert(a === b) 315 InspectorTest.assert(a !== b) 1 InspectorTest.assert(a === null) 4 InspectorTest.assert(Object.shallowEqual(a, b)) 0 InspectorTest.expectThat(!expr) 60 InspectorTest.expectThat(a === b) 383 InspectorTest.expectThat(a !== b) 8 InspectorTest.expectThat(a === null) 11 InspectorTest.expectThat(Object.shallowEqual(a, b)) 14 Based on this, TestHarness should include the following family of assertions (and corresponding expect* methods): assert assertFalse assertNull assertNotNull assertEqual assertNotEqual assertShallowEqual
<rdar://problem/28039741>
Created attachment 288538 [details] [Patch] Proposed Fix
Comment on attachment 288538 [details] [Patch] Proposed Fix Attachment 288538 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2055983 New failing tests: inspector/unit-tests/test-harness-expect-functions.html
Created attachment 288541 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 288538 [details] [Patch] Proposed Fix Attachment 288538 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2056001 New failing tests: inspector/unit-tests/test-harness-expect-functions.html
Created attachment 288542 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 288538 [details] [Patch] Proposed Fix Attachment 288538 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2056019 New failing tests: inspector/unit-tests/test-harness-expect-functions.html
Created attachment 288543 [details] Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 288587 [details] [Patch] Proposed Fix
Comment on attachment 288587 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=288587&action=review r=me! > Source/WebInspectorUI/UserInterface/Test/TestHarness.js:140 > + console.assert(typeof expression1 === "number"); > + console.assert(typeof expression2 === "number"); Are console.assert's useful in LayoutTests? Should we make these `this.assert(...)`? > LayoutTests/inspector/unit-tests/test-harness-expect-functions.html:21 > + args = args.map((a) => { > + if (typeof a === "number") > + return a; > + // Append empty string so `undefined` is displayed correctly. > + return JSON.stringify(a) + ""; > + }); > + return args.join(", "); Nit: You can combine these lines: return args.map((a) => { ... }).join(", "); > LayoutTests/inspector/unit-tests/test-harness-expect-functions.html:30 > + test: () => { Style: `test: () => {` => `test() {` > LayoutTests/inspector/unit-tests/test-harness-expect-functions.html:33 > + if (!inputs || !inputs.length) > + return; Should probably output a fail message here, since this would be unexpected: InspectorTest.fail("Expected inputs."); > LayoutTests/inspector/unit-tests/test-harness-expect-functions.html:59 > + addInverseTestCase("expectFalse", expectThatTestCase); I wonder if we should call this expectFalsey, though maybe more descriptive it would probably worse in code. > LayoutTests/inspector/unit-tests/test-harness-expect-functions.html:72 > + let expectEqualTestCase = { > + functionName: "expectEqual", > + passingInputs: [ > + [true, true], Lets add a case here for object equivalence: [object1, object1]. > LayoutTests/inspector/unit-tests/test-harness-expect-functions.html:79 > + [true, false], Same here for two different objects [object1, object2]. > LayoutTests/inspector/unit-tests/test-harness-expect-functions.html:101 > + // Enable once <https://webkit.org/b/161852> is fixed. > + // [{}, []], You should be able to enable this before landing.
Comment on attachment 288587 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=288587&action=review >> Source/WebInspectorUI/UserInterface/Test/TestHarness.js:140 >> + console.assert(typeof expression2 === "number"); > > Are console.assert's useful in LayoutTests? Should we make these `this.assert(...)`? It left like TestHarness.assert was intended for use in test code, not internally. FrontendTestHarness also uses console.assert. However it looks like even after calling Inspector.debug(), console.assert (and debug) messages don't appear in test output. console.log/info/warn do.
Created attachment 288601 [details] [Patch] For Landing
Comment on attachment 288601 [details] [Patch] For Landing Clearing flags on attachment: 288601 Committed r205834: <http://trac.webkit.org/changeset/205834>
All reviewed patches have been landed. Closing bug.