From 96891669c38e3268b93123fed822fd10d710e9c1 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 21 Aug 2020 21:53:54 -0400 Subject: [PATCH] test-js: Sometimes include more details for failures LibJS doesn't store stacks for exception objects, so this only amends test-common.js's __expect() with an optional `details` function that can produce a more detailed error message, and it lets test-js.cpp read and print that error message. I added the optional details parameter to a few matchers, most notably toBe() where it now prints expected and actual value. It'd be nice to have line numbers of failures, but that seems hard to do with the current design, and this is already much better than the current state. --- Libraries/LibJS/Tests/test-common.js | 30 +++++++++++++++++----------- Userland/test-js.cpp | 10 ++++++++-- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/Libraries/LibJS/Tests/test-common.js b/Libraries/LibJS/Tests/test-common.js index 4f827b6179..7143984d37 100644 --- a/Libraries/LibJS/Tests/test-common.js +++ b/Libraries/LibJS/Tests/test-common.js @@ -63,14 +63,15 @@ class ExpectationError extends Error { toBe(value) { this.__doMatcher(() => { - this.__expect(Object.is(this.target, value)); + this.__expect(Object.is(this.target, value), + () => ("toBe: expected _" + String(value) + "_, got _" + String(this.target) + "_")); }); } // FIXME: Take a precision argument like jest's toBeCloseTo matcher toBeCloseTo(value) { - this.__expect(typeof this.target === "number"); - this.__expect(typeof value === "number"); + this.__expect(typeof this.target === "number", () => "toBeCloseTo: target not of type number"); + this.__expect(typeof value === "number", () => "toBeCloseTo: argument not of type number"); this.__doMatcher(() => { this.__expect(Math.abs(this.target - value) < 0.000001); @@ -78,7 +79,7 @@ class ExpectationError extends Error { } toHaveLength(length) { - this.__expect(typeof this.target.length === "number"); + this.__expect(typeof this.target.length === "number", () => "toHaveLength: target.length not of type number"); this.__doMatcher(() => { this.__expect(Object.is(this.target.length, length)); @@ -120,7 +121,7 @@ class ExpectationError extends Error { toBeDefined() { this.__doMatcher(() => { - this.__expect(this.target !== undefined); + this.__expect(this.target !== undefined, () => "toBeDefined: target was undefined"); }); } @@ -138,13 +139,13 @@ class ExpectationError extends Error { toBeUndefined() { this.__doMatcher(() => { - this.__expect(this.target === undefined); + this.__expect(this.target === undefined, () => "toBeUndefined: target was not undefined"); }); } toBeNaN() { this.__doMatcher(() => { - this.__expect(isNaN(this.target)); + this.__expect(isNaN(this.target), () => ("toBeNaN: target was _" + String(this.target) + "_, not NaN")); }); } @@ -258,9 +259,8 @@ class ExpectationError extends Error { // jest-extended fail(message) { - // FIXME: message is currently ignored this.__doMatcher(() => { - this.__expect(false); + this.__expect(false, message); }); } @@ -391,12 +391,17 @@ class ExpectationError extends Error { } catch (e) { if (e.name === "ExpectationError") threw = true; } - if (!threw) throw new ExpectationError(); + if (!threw) throw new ExpectationError("not: test didn't fail"); } } - __expect(value) { - if (value !== true) throw new ExpectationError(); + __expect(value, details) { + if (value !== true) { + if (details !== undefined) + throw new ExpectationError(details()); + else + throw new ExpectationError(); + } } } @@ -432,6 +437,7 @@ class ExpectationError extends Error { } catch (e) { suite[message] = { result: "fail", + details: String(e), }; } }; diff --git a/Userland/test-js.cpp b/Userland/test-js.cpp index 2a1dadd6b3..f3821d4c97 100644 --- a/Userland/test-js.cpp +++ b/Userland/test-js.cpp @@ -52,6 +52,7 @@ enum class TestResult { struct JSTest { String name; TestResult result; + String details; }; struct JSSuite { @@ -281,7 +282,7 @@ JSFileResult TestRunner::run_file_test(const String& test_path) ASSERT(suite_value.is_object()); suite_value.as_object().for_each_member([&](const String& test_name, const JsonValue& test_value) { - JSTest test { test_name, TestResult::Fail }; + JSTest test { test_name, TestResult::Fail, "" }; ASSERT(test_value.is_object()); ASSERT(test_value.as_object().has("result")); @@ -296,6 +297,10 @@ JSFileResult TestRunner::run_file_test(const String& test_path) test.result = TestResult::Fail; m_counts.tests_failed++; suite.most_severe_test_result = TestResult::Fail; + ASSERT(test_value.as_object().has("details")); + auto details = test_value.as_object().get("details"); + ASSERT(result.is_string()); + test.details = details.as_string(); } else { test.result = TestResult::Skip; if (suite.most_severe_test_result == TestResult::Pass) @@ -476,7 +481,8 @@ void TestRunner::print_file_result(const JSFileResult& file_result) const printf(" Test: "); if (test.result == TestResult::Fail) { print_modifiers({ CLEAR, FG_RED }); - printf("%s (failed)\n", test.name.characters()); + printf("%s (failed):\n", test.name.characters()); + printf(" %s\n", test.details.characters()); } else { print_modifiers({ CLEAR, FG_ORANGE }); printf("%s (skipped)\n", test.name.characters());