From 73987a1772c1887d19ba023965a874dbe8870706 Mon Sep 17 00:00:00 2001 From: Ian Bull Date: Mon, 26 Aug 2024 01:22:16 -0400 Subject: [PATCH] refactor(testing): align additional error messages (#5810) --- testing/_test_suite.ts | 6 ++++-- testing/bdd.ts | 6 +++--- testing/bdd_test.ts | 2 +- testing/mock.ts | 38 ++++++++++++++++++++++++++++---------- testing/mock_test.ts | 34 +++++++++++++++++----------------- testing/time.ts | 38 ++++++++++++++++++++++++++++---------- testing/time_test.ts | 38 +++++++++++++++++++++++++++++--------- 7 files changed, 110 insertions(+), 52 deletions(-) diff --git a/testing/_test_suite.ts b/testing/_test_suite.ts index 9a0696a6f..805a8b093 100644 --- a/testing/_test_suite.ts +++ b/testing/_test_suite.ts @@ -84,7 +84,9 @@ export class TestSuiteInternal implements TestSuite { const { suite } = describe; if (suite && !TestSuiteInternal.suites.has(suite.symbol)) { - throw new Error("suite does not represent a registered test suite"); + throw new Error( + "Cannot construct Test Suite: suite does not represent a registered test suite", + ); } const testSuite = suite ? TestSuiteInternal.suites.get(suite.symbol) @@ -101,7 +103,7 @@ export class TestSuiteInternal implements TestSuite { const value = fn() as any; if (value instanceof Promise) { throw new Error( - 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.', + 'Returning a Promise from "describe" is not supported: tests must be defined synchronously', ); } } finally { diff --git a/testing/bdd.ts b/testing/bdd.ts index 8b1a884cc..81a8e7f71 100644 --- a/testing/bdd.ts +++ b/testing/bdd.ts @@ -562,7 +562,7 @@ export interface it { export function it(...args: ItArgs) { if (TestSuiteInternal.runningCount > 0) { throw new Error( - "cannot register new test cases after already registered test cases start running", + "Cannot register new test cases after already registered test cases start running", ); } const options = itDefinition(...args); @@ -717,7 +717,7 @@ function addHook( if (!TestSuiteInternal.current) { if (TestSuiteInternal.started) { throw new Error( - "cannot add global hooks after a global test is registered", + "Cannot add global hooks after a global test is registered", ); } TestSuiteInternal.current = new TestSuiteInternal({ @@ -1070,7 +1070,7 @@ export function describe( ): TestSuite { if (TestSuiteInternal.runningCount > 0) { throw new Error( - "cannot register new test suites after already registered test cases start running", + "Cannot register new test suites after already registered test cases start running", ); } const options = describeDefinition(...args); diff --git a/testing/bdd_test.ts b/testing/bdd_test.ts index dd133dc30..2ab8c4725 100644 --- a/testing/bdd_test.ts +++ b/testing/bdd_test.ts @@ -2055,7 +2055,7 @@ Deno.test("describe()", async (t) => { // deno-lint-ignore no-explicit-any () => describe("async describe", (async () => {}) as any), Error, - 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.', + 'Returning a Promise from "describe" is not supported: tests must be defined synchronously', ); }, ); diff --git a/testing/mock.ts b/testing/mock.ts index 8cc1ce533..b542f218e 100644 --- a/testing/mock.ts +++ b/testing/mock.ts @@ -435,7 +435,9 @@ function functionSpy< restore: { enumerable: true, value: () => { - throw new MockError("Function cannot be restored"); + throw new MockError( + "Cannot restore: function cannot be restored", + ); }, }, }); @@ -643,15 +645,21 @@ function methodSpy< Return, >(self: Self, property: keyof Self): MethodSpy { if (typeof self[property] !== "function") { - throw new MockError("Property is not an instance method"); + throw new MockError( + "Cannot spy: property is not an instance method", + ); } if (isSpy(self[property])) { - throw new MockError("Already spying on instance method"); + throw new MockError( + "Cannot spy: already spying on instance method", + ); } const propertyDescriptor = Object.getOwnPropertyDescriptor(self, property); if (propertyDescriptor && !propertyDescriptor.configurable) { - throw new MockError("Cannot spy on non-configurable instance method"); + throw new MockError( + "Cannot spy: non-configurable instance method", + ); } const original = self[property] as unknown as ( @@ -690,7 +698,9 @@ function methodSpy< enumerable: true, value: () => { if (restored) { - throw new MockError("Instance method already restored"); + throw new MockError( + "Cannot restore: instance method already restored", + ); } if (propertyDescriptor) { Object.defineProperty(self, property, propertyDescriptor); @@ -767,7 +777,9 @@ function constructorSpy< static readonly calls = calls; static readonly restored = false; static restore() { - throw new MockError("Constructor cannot be restored"); + throw new MockError( + "Cannot restore: constructor cannot be restored", + ); } } as ConstructorSpy; return spy; @@ -1082,15 +1094,19 @@ export function stub< func?: (this: Self, ...args: Args) => Return, ): Stub { if (self[property] !== undefined && typeof self[property] !== "function") { - throw new MockError("Property is not an instance method"); + throw new MockError( + "Cannot stub: property is not an instance method", + ); } if (isSpy(self[property])) { - throw new MockError("Already spying on instance method"); + throw new MockError( + "Cannot stub: already spying on instance method", + ); } const propertyDescriptor = Object.getOwnPropertyDescriptor(self, property); if (propertyDescriptor && !propertyDescriptor.configurable) { - throw new MockError("Cannot stub non-configurable instance method"); + throw new MockError("Cannot stub: non-configurable instance method"); } const fake = func ?? (() => {}) as (this: Self, ...args: Args) => Return; @@ -1135,7 +1151,9 @@ export function stub< enumerable: true, value: () => { if (restored) { - throw new MockError("Instance method already restored"); + throw new MockError( + "Cannot restore: instance method already restored", + ); } if (propertyDescriptor) { Object.defineProperty(self, property, propertyDescriptor); diff --git a/testing/mock_test.ts b/testing/mock_test.ts index de83b38e1..51326b2cd 100644 --- a/testing/mock_test.ts +++ b/testing/mock_test.ts @@ -79,7 +79,7 @@ Deno.test("spy()", () => { assertThrows( () => func.restore(), MockError, - "Function cannot be restore", + "Cannot restore: function cannot be restored", ); assertEquals(func.restored, false); }); @@ -125,7 +125,7 @@ Deno.test("spy() works on function", () => { assertThrows( () => func.restore(), MockError, - "Function cannot be restored", + "Cannot restore: function cannot be restored", ); assertEquals(func.restored, false); @@ -239,7 +239,7 @@ Deno.test("spy() works on instance method", () => { assertThrows( () => func.restore(), MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(func.restored, true); }); @@ -277,7 +277,7 @@ Deno.test("spy() works on instance method symbol", () => { assertThrows( () => func.restore(), MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(func.restored, true); }); @@ -342,7 +342,7 @@ Deno.test("spy() works on instance method property descriptor", () => { assertThrows( () => action.restore(), MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(action.restored, true); }); @@ -444,7 +444,7 @@ Deno.test("spy() supports explicit resource management", () => { if (funcRef) funcRef.restore(); }, MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(funcRef.restored, true); } @@ -478,7 +478,7 @@ Deno.test("spy() works on constructor", () => { assertThrows( () => PointSpy.restore(), MockError, - "Constructor cannot be restored", + "Cannot restore: constructor cannot be restored", ); }); @@ -546,7 +546,7 @@ Deno.test("spy() throws when try spying already spied method", () => { assertThrows( () => spy(obj, "fn"), MockError, - "Already spying on instance method", + "Cannot spy: already spying on instance method", ); }); @@ -556,7 +556,7 @@ Deno.test("spy() throws when the property is not a method", () => { // deno-lint-ignore no-explicit-any () => spy(obj as any, "fn"), MockError, - "Property is not an instance method", + "Cannot spy: property is not an instance method", ); }); @@ -566,7 +566,7 @@ Deno.test("spy() throws when the property is not configurable", () => { assertThrows( () => spy(obj, "fn"), MockError, - "Cannot spy on non-configurable instance method", + "Cannot spy: non-configurable instance method", ); }); @@ -602,7 +602,7 @@ Deno.test("stub()", () => { assertThrows( () => func.restore(), MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(func.restored, true); }); @@ -640,7 +640,7 @@ Deno.test("stub() works on function", () => { assertThrows( () => func.restore(), MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(func.restored, true); }); @@ -684,7 +684,7 @@ Deno.test("stub() supports explicit resource management", () => { if (funcRef) funcRef.restore(); }, MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(funcRef.restored, true); } @@ -727,7 +727,7 @@ Deno.test("stub() handles non existent function", () => { assertThrows( () => func.restore(), MockError, - "Instance method already restored", + "Cannot restore: instance method already restored", ); assertEquals(func.restored, true); }); @@ -797,7 +797,7 @@ Deno.test("stub() throws when the property is not a method", () => { // deno-lint-ignore no-explicit-any () => stub(obj as any, "fn"), MockError, - "Property is not an instance method", + "Cannot stub: property is not an instance method", ); }); @@ -807,7 +807,7 @@ Deno.test("stub() throws when try stubbing already stubbed method", () => { assertThrows( () => stub(obj, "fn"), MockError, - "Already spying on instance method", + "Cannot stub: already spying on instance method", ); }); @@ -817,7 +817,7 @@ Deno.test("stub() throws then the property is not configurable", () => { assertThrows( () => stub(obj, "fn"), MockError, - "Cannot stub non-configurable instance method", + "Cannot stub: non-configurable instance method", ); }); diff --git a/testing/time.ts b/testing/time.ts index 150b7182c..59ba6a44c 100644 --- a/testing/time.ts +++ b/testing/time.ts @@ -134,12 +134,12 @@ function fakeSetTimeout( // deno-lint-ignore no-explicit-any ...args: any[] ): number { - if (!time) throw new TimeError("Time is not faked"); + if (!time) throw new TimeError("Cannot set timeout: time is not faked"); return setTimer(callback, delay, args, false); } function fakeClearTimeout(id?: unknown) { - if (!time) throw new TimeError("Time is not faked"); + if (!time) throw new TimeError("Cannot clear timeout: time is not faked"); if (typeof id === "number" && dueNodes.has(id)) { dueNodes.delete(id); } @@ -152,12 +152,12 @@ function fakeSetInterval( // deno-lint-ignore no-explicit-any ...args: any[] ): number { - if (!time) throw new TimeError("Time is not faked"); + if (!time) throw new TimeError("Cannot set interval: time is not faked"); return setTimer(callback, delay, args, true); } function fakeClearInterval(id?: unknown) { - if (!time) throw new TimeError("Time is not faked"); + if (!time) throw new TimeError("Cannot clear interval: time is not faked"); if (typeof id === "number" && dueNodes.has(id)) { dueNodes.delete(id); } @@ -294,7 +294,9 @@ export class FakeTime { start?: number | string | Date | null, options?: FakeTimeOptions, ) { - if (time) throw new TimeError("Time is already faked"); + if (time) { + throw new TimeError("Cannot construct FakeTime: time is already faked"); + } initializedAt = _internals.Date.now(); startedAt = start instanceof Date ? start.valueOf() @@ -303,7 +305,11 @@ export class FakeTime { : typeof start === "string" ? (new Date(start)).valueOf() : initializedAt; - if (Number.isNaN(startedAt)) throw new TypeError("Invalid start time"); + if (Number.isNaN(startedAt)) { + throw new TypeError( + `Cannot construct FakeTime: invalid start time ${startedAt}`, + ); + } now = startedAt; timerId = timerIdGen(); @@ -381,7 +387,9 @@ export class FakeTime { * ``` */ static restore() { - if (!time) throw new TimeError("Time is already restored"); + if (!time) { + throw new TimeError("Cannot restore time: time is already restored"); + } time.restore(); } @@ -417,7 +425,11 @@ export class FakeTime { // deno-lint-ignore no-explicit-any ...args: any[] ): Promise { - if (!time) return Promise.reject(new TimeError("Time is not faked")); + if (!time) { + return Promise.reject( + new TimeError("Cannot restore time: time is not faked"), + ); + } restoreGlobals(); try { const result = callback.apply(null, args); @@ -478,7 +490,11 @@ export class FakeTime { * @param value The current time (in milliseconds) */ set now(value: number) { - if (value < now) throw new RangeError("Time cannot go backwards"); + if (value < now) { + throw new RangeError( + `Cannot set current time in the past, time must be >= ${now}: received ${value}`, + ); + } let dueNode: DueNode | null = dueTree.min(); while (dueNode && dueNode.due <= value) { const timer: Timer | undefined = dueNode.timers.shift(); @@ -803,7 +819,9 @@ export class FakeTime { * ``` */ restore() { - if (!time) throw new TimeError("Time is already restored"); + if (!time) { + throw new TimeError("Cannot restore time: time is already restored"); + } time = undefined; restoreGlobals(); if (advanceIntervalId) clearInterval(advanceIntervalId); diff --git a/testing/time_test.ts b/testing/time_test.ts index 632b1f6fe..33b8c5973 100644 --- a/testing/time_test.ts +++ b/testing/time_test.ts @@ -339,7 +339,7 @@ Deno.test("FakeTime.restoreFor() returns promise that rejected to TimeError if F await assertRejects( () => FakeTime.restoreFor(() => {}), TimeError, - "Time is not faked", + "Cannot restore time: time is not faked", ); }); @@ -662,15 +662,23 @@ Deno.test("Faked timer functions throws when called after FakeTime is restored", assertThrows( () => fakeSetTimeout(() => {}, 0), TimeError, - "Time is not faked", + "Cannot set timeout: time is not faked", + ); + assertThrows( + () => fakeClearTimeout(0), + TimeError, + "Cannot clear timeout: time is not faked", ); - assertThrows(() => fakeClearTimeout(0), TimeError, "Time is not faked"); assertThrows( () => fakeSetInterval(() => {}, 0), TimeError, - "Time is not faked", + "Cannot set interval: time is not faked", + ); + assertThrows( + () => fakeClearInterval(0), + TimeError, + "Cannot clear interval: time is not faked", ); - assertThrows(() => fakeClearInterval(0), TimeError, "Time is not faked"); }); Deno.test("Faked Date.now returns real time after FakeTime is restored", () => { @@ -700,19 +708,31 @@ Deno.test("FakeTime can be constructed with number, Date, or string", () => { }); Deno.test("FakeTime throws when NaN is provided", () => { - assertThrows(() => new FakeTime(NaN), TypeError, "Invalid start time"); + assertThrows( + () => new FakeTime(NaN), + TypeError, + "Cannot construct FakeTime: invalid start time NaN", + ); }); Deno.test("FakeTime.restore() throws when the time is already restored", () => { const _time = new FakeTime(); FakeTime.restore(); - assertThrows(() => FakeTime.restore(), TimeError, "Time is already restored"); + assertThrows( + () => FakeTime.restore(), + TimeError, + "Cannot restore time: time is already restored", + ); }); Deno.test("time.restore() throws when the time is already restored", () => { const time = new FakeTime(); time.restore(); - assertThrows(() => time.restore(), TimeError, "Time is already restored"); + assertThrows( + () => time.restore(), + TimeError, + "Cannot restore time: time is already restored", + ); }); Deno.test("time.now = N throws when N < time.now", () => { @@ -722,7 +742,7 @@ Deno.test("time.now = N throws when N < time.now", () => { time.now = 999; }, RangeError, - "Time cannot go backwards", + "Cannot set current time in the past, time must be >= 1000: received 999", ); });