diff --git a/docs/testing-api.md b/docs/testing-api.md index 6efac8e7a..689bfef8a 100644 --- a/docs/testing-api.md +++ b/docs/testing-api.md @@ -420,6 +420,8 @@ Both patterns work because GocciaScript's `await` is a synchronous drain --- the **Important:** If a test returns a Promise that is still pending after the microtask queue drains and all pending fetch completions have been pumped, the test **fails** with "Promise still pending after microtask drain". Since GocciaScript has no general event loop, a non-fetch pending Promise after drain will never settle --- this catches tests with missing assertions or broken async chains. This mirrors how Jest/Vitest fail tests with a timeout when the returned Promise never resolves. +When a returned Promise rejects, the failure line reports the reason as `Returned Promise rejected: `. An `Error` is named and described --- `Error: boom`, or the class name for a subclass that does not set its own `name`, such as `MyError: boom` --- because its `name` lives on the prototype and its `message` is non-enumerable, so serializing the object alone would render it as `{}`. Any other reason is serialized as a value. + **Testing intentionally-pending Promises:** When testing behavior around forever-pending Promises (e.g., verifying that `reject()` after `resolve(pendingPromise)` is ignored), never return the pending Promise. Instead, use a separate settled Promise chain to verify state after microtasks drain: ```javascript diff --git a/scripts/test-cli.ts b/scripts/test-cli.ts index 31dc4cf6e..f94f0087a 100644 --- a/scripts/test-cli.ts +++ b/scripts/test-cli.ts @@ -3174,9 +3174,10 @@ console.log("--log option..."); // -- Assertion failure text (TestRunner) --------------------------------------- // A failed assertion is recorded rather than thrown, so its message cannot be -// observed from inside a test. These two properties are load-bearing enough to -// pin from the outside: toBeInstanceOf naming what it compared, and the vitest -// shim keeping a named, actionable error for every member it does not provide. +// observed from inside a test. These properties are load-bearing enough to pin +// from the outside: toBeInstanceOf naming what it compared, a rejected returned +// Promise naming the error it rejected with, and the vitest shim keeping a +// named, actionable error for every member it does not provide. console.log("Assertion failure text..."); { const tmp = mkdtemp("goccia-assertion-text-"); @@ -3212,6 +3213,51 @@ console.log("Assertion failure text..."); throw new Error(`toBeInstanceOf should report ${expected}, got: ${instanceOfOut}`); } + // The reason a returned Promise rejected with is the whole failure report, + // and an Error keeps "name" on its prototype and "message" non-enumerable, + // so serializing the value reported `new Error("boom")` as "{}" — the one + // shape a debugging session most needs named. A class extending Error + // inherits Error.prototype.name, so its identity is read off the + // constructor, while an explicitly assigned name still wins. + const rejectionSrc = join(tmp, "rejection.test.js"); + writeFileSync( + rejectionSrc, + [ + "class MyErr extends Error {}", + "class NamedErr extends Error {", + " constructor(message) { super(message); this.name = 'ValidationFailure'; }", + "}", + "class ProtoNamed extends Error {}", + "ProtoNamed.prototype.name = 'ProtoAssigned';", + 'test("plain error", () => Promise.reject(new Error("boom")));', + 'test("subclass error", () => Promise.reject(new MyErr("boom")));', + 'test("named subclass error", () => Promise.reject(new NamedErr("boom")));', + 'test("prototype-named subclass error", () => Promise.reject(new ProtoNamed("boom")));', + 'test("native error", () => Promise.reject(new TypeError("bad")));', + 'test("plain object", () => Promise.reject({ code: 42 }));', + 'test("message only", () => Promise.reject({ message: "hi" }));', + "", + ].join("\n"), + ); + for (const mode of ["--mode=interpreted", "--mode=bytecode"]) { + const rejection = await $`${TESTRUNNER} ${rejectionSrc} ${mode} --no-progress 2>&1`.nothrow(); + const rejectionOut = rejection.text(); + for (const expected of [ + "Returned Promise rejected: Error: boom", + "Returned Promise rejected: MyErr: boom", + "Returned Promise rejected: ValidationFailure: boom", + "Returned Promise rejected: ProtoAssigned: boom", + "Returned Promise rejected: TypeError: bad", + "Returned Promise rejected: { code: 42 }", + "Returned Promise rejected: { message: 'hi' }", + ]) { + if (!rejectionOut.includes(expected)) + throw new Error( + `TestRunner (${mode}) should report "${expected}", got: ${rejectionOut}`, + ); + } + } + // Every member the shim does not implement must keep throwing by name. The // contract is what tells a suite author which member to work around, so // silently degrading one to a no-op is worse than not having it. diff --git a/source/units/Goccia.Builtins.TestingLibrary.pas b/source/units/Goccia.Builtins.TestingLibrary.pas index 8b3cb590e..fb332a929 100644 --- a/source/units/Goccia.Builtins.TestingLibrary.pas +++ b/source/units/Goccia.Builtins.TestingLibrary.pas @@ -759,6 +759,52 @@ function DescribeThrowValue(const AValue: TGocciaValue): string; Result := FormatForDisplay(AValue); end; +{ How the reason of a rejected returned Promise is reported. The reason is the + only evidence the failure line carries, and an Error keeps "name" on its + prototype and "message" non-enumerable, so serializing the value rendered + `new Error('boom')` as an empty object and named neither the error nor what + went wrong. The prototype-chain read DescribeThrowValue already uses for + assertion output recovers both. + + A user class extending Error inherits Error.prototype.name, so its own + identity -- the one thing that says which of a suite's error types rejected + -- lives only on the constructor; prefer that name while the resolved "name" + is still the default "Error", and keep any assigned name -- instance or + prototype -- when the author supplied one. } +function DescribeRejectionReason(const AValue: TGocciaValue): string; +var + NameValue: TGocciaValue; + MessageValue: TGocciaValue; + ConstructorValue: TGocciaValue; +begin + Result := DescribeThrowValue(AValue); + if not (AValue is TGocciaObjectValue) then + Exit; + + { Only the name DescribeThrowValue just read off the prototype chain is up + for replacement, and only while it is still the default "Error" that + Error.prototype supplies: any other name — assigned on the instance or on + a prototype — is already the author's answer to this question. } + NameValue := TGocciaObjectValue(AValue).GetProperty(PROP_NAME); + MessageValue := TGocciaObjectValue(AValue).GetProperty(PROP_MESSAGE); + if not ((NameValue is TGocciaStringLiteralValue) and + (MessageValue is TGocciaStringLiteralValue)) then + Exit; + if TGocciaStringLiteralValue(NameValue).Value <> 'Error' then + Exit; + if TGocciaObjectValue(AValue).HasOwnProperty(PROP_NAME) then + Exit; + + { Only a declared class narrows the name: a built-in error's constructor + already agrees with its "name", and a plain object's is Object, which would + report every object-literal reason as "Object". } + ConstructorValue := TGocciaObjectValue(AValue).GetProperty(PROP_CONSTRUCTOR); + if (ConstructorValue is TGocciaClassValue) and + (TGocciaClassValue(ConstructorValue).Name <> '') then + Result := TGocciaClassValue(ConstructorValue).Name + ': ' + + TGocciaStringLiteralValue(MessageValue).Value; +end; + { The name a value's own "name" property reports, or '' when it has none. } function OwnNamePropertyOf(const AValue: TGocciaValue): string; var @@ -4169,7 +4215,7 @@ procedure TGocciaTestAssertions.ExecuteSuite(const ASuite: TGocciaTestSuite; WaitForFetchPromise(TGocciaPromiseValue(TestResult)); if TGocciaPromiseValue(TestResult).State = gpsRejected then begin - RejectionReason := FormatForDisplay( + RejectionReason := DescribeRejectionReason( TGocciaPromiseValue(TestResult).PromiseResult); AssertionFailed('async test', 'Returned Promise rejected: ' + RejectionReason);