diff --git a/CHANGELOG.md b/CHANGELOG.md index f42d49972..1accb1eb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,12 +17,18 @@ Other notable changes: * general: * Allow node version 23. * `loggy-intf` / `loggy`: - * Minor tweaks to "human" (non-JSON) log rendering. + * Made several improvements to "human" (non-JSON) log rendering, including + fixing it to be able to log values with reference cycles. * `structy`: * Started allowing any object (plain or not) to be used as the argument to the `BaseStruct` constructor. * Added the option to allow undeclared properties to be allowed and dynamically vetted, via two additional `_impl*` methods. +* `valvis`: + * `BaseValueVisitor`: + * Simplified detection of reference cycles. + * Added argument `isCycleHead` to `_impl_shouldRef()`, so client code can + choose to be more cycle-aware. * `webapp-builtins`: * Simplified naming scheme for preserved log files: Names now always include a `-` suffix after the date. diff --git a/src/loggy-intf/export/LogPayload.js b/src/loggy-intf/export/LogPayload.js index 421f3a34d..e829ab867 100644 --- a/src/loggy-intf/export/LogPayload.js +++ b/src/loggy-intf/export/LogPayload.js @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import { EventPayload, EventSource } from '@this/async'; -import { IntfDeconstructable, Sexp } from '@this/sexp'; import { Moment } from '@this/quant'; +import { IntfDeconstructable, Sexp } from '@this/sexp'; import { MustBe } from '@this/typey'; import { StackTrace } from '@this/valvis'; diff --git a/src/loggy-intf/export/LoggedValueEncoder.js b/src/loggy-intf/export/LoggedValueEncoder.js index 0fde6bd98..2d91a0dcd 100644 --- a/src/loggy-intf/export/LoggedValueEncoder.js +++ b/src/loggy-intf/export/LoggedValueEncoder.js @@ -33,17 +33,27 @@ export class LoggedValueEncoder extends BaseValueVisitor { } /** @override */ - _impl_shouldRef(value) { - if (typeof value === 'object') { - if (Array.isArray(value)) { - return (value.length > 10); - } else if (AskIf.plainObject(value)) { - return (Object.entries(value).length > 10); - } else { + _impl_shouldRef(value, isCycleHead) { + switch (typeof value) { + case 'function': { return true; } - } else { - return false; + + case 'object': { + if (isCycleHead) { + return true; + } else if (Array.isArray(value)) { + return (value.length > 10); + } else if (AskIf.plainObject(value)) { + return (Object.entries(value).length > 10); + } else { + return true; + } + } + + default: { + return false; + } } } @@ -99,7 +109,10 @@ export class LoggedValueEncoder extends BaseValueVisitor { const visitedArray = this._prot_visitProperties(sexpArray); return new Sexp(...visitedArray); } else { - return this._prot_labelFromValue(node); + const constructor = Reflect.getPrototypeOf(node).constructor; + return constructor + ? new Sexp(this._prot_nameFromValue(constructor), '...') + : new Sexp('Object', this._prot_labelFromValue(node), '...'); } } @@ -110,7 +123,7 @@ export class LoggedValueEncoder extends BaseValueVisitor { /** @override */ _impl_visitProxy(node, isFunction_unused) { - return this._prot_labelFromValue(node); + return new Sexp('Proxy', this._prot_nameFromValue(node)); } /** @override */ @@ -186,12 +199,22 @@ export class LoggedValueEncoder extends BaseValueVisitor { return this.#makeDefIfAppropriate(node, result); } + /** @override */ + _impl_visitInstance(node) { + return this.#makeDefIfAppropriate(node, node); + } + /** @override */ _impl_visitPlainObject(node) { const result = this._prot_visitProperties(node); return this.#makeDefIfAppropriate(node, result); } + /** @override */ + _impl_visitString(node) { + return this.#makeDefIfAppropriate(node, node); + } + /** * Wraps a result in a "def" if it is in fact a defining value occurrence. * diff --git a/src/loggy-intf/tests/LogPayload.test.js b/src/loggy-intf/tests/LogPayload.test.js index b4147ffd7..7a77a6468 100644 --- a/src/loggy-intf/tests/LogPayload.test.js +++ b/src/loggy-intf/tests/LogPayload.test.js @@ -3,9 +3,9 @@ import stripAnsi from 'strip-ansi'; -import { Sexp } from '@this/sexp'; import { LogPayload, LogTag } from '@this/loggy-intf'; import { Moment } from '@this/quant'; +import { Sexp } from '@this/sexp'; import { StackTrace } from '@this/valvis'; diff --git a/src/loggy-intf/tests/LogTag.test.js b/src/loggy-intf/tests/LogTag.test.js index 73231d42b..8c62224d6 100644 --- a/src/loggy-intf/tests/LogTag.test.js +++ b/src/loggy-intf/tests/LogTag.test.js @@ -3,8 +3,8 @@ import stripAnsi from 'strip-ansi'; -import { Sexp } from '@this/sexp'; import { LogTag } from '@this/loggy-intf'; +import { Sexp } from '@this/sexp'; import { StyledText } from '@this/texty'; diff --git a/src/loggy-intf/tests/LoggedValueEncoder.test.js b/src/loggy-intf/tests/LoggedValueEncoder.test.js new file mode 100644 index 000000000..6b0a05b45 --- /dev/null +++ b/src/loggy-intf/tests/LoggedValueEncoder.test.js @@ -0,0 +1,147 @@ +// Copyright 2022-2024 the Lactoserv Authors (Dan Bornstein et alia). +// SPDX-License-Identifier: Apache-2.0 + +import { LoggedValueEncoder } from '@this/loggy-intf'; +import { Duration } from '@this/quant'; +import { Sexp } from '@this/sexp'; +import { VisitDef, VisitRef } from '@this/valvis'; + + +function withNullObjectProtos(value) { + if (!(value && (typeof value === 'object'))) { + return value; + } else if (Array.isArray(value)) { + const result = []; + for (const v of value) { + result.push(withNullObjectProtos(v)); + } + return result; + } else if (Reflect.getPrototypeOf(value) === Object.prototype) { + const result = Object.create(null); + for (const [k, v] of Object.entries(value)) { + result[k] = withNullObjectProtos(v); + } + return result; + } else { + return value; + } +} + +function sexp(type, ...args) { + return new Sexp(type, ...args); +} + +describe('encode()', () => { + // Cases where the result should be equal to the input. + test.each` + value + ${null} + ${false} + ${true} + ${'florp'} + ${123.456} + ${[]} + ${[1, 2, 3, 'four']} + ${[[null], [false], [[55]]]} + `('($#) self-encodes $value', ({ value }) => { + const got = LoggedValueEncoder.encode(value); + expect(got).toStrictEqual(value); + }); + + // Plain objects are expected to get converted to null-prototype objects. + test.each` + value + ${{}} + ${{ a: 10, b: 'twenty' }} + ${{ abc: { x: [{ d: 'efg' }] } }} + `('($#) self-encodes $value except with a `null` object prototypes', ({ value }) => { + const got = LoggedValueEncoder.encode(value); + const expected = withNullObjectProtos(value); + expect(got).toStrictEqual(expected); + }); + + class SomeClass { + // @defaultConstructor + } + + const someFunc = () => null; + + // Stuff that isn't JSON-encodable should end up in the form of a sexp. + test.each` + value | expected + ${undefined} | ${sexp('Undefined')}} + ${[undefined]} | ${[sexp('Undefined')]}} + ${321123n} | ${sexp('BigInt', '321123')}} + ${Symbol('xyz')} | ${sexp('Symbol', 'xyz')}} + ${Symbol.for('blorp')} | ${sexp('Symbol', 'blorp', true)}} + ${new Duration(12.34)} | ${sexp('Duration', 12.34, '12.340 sec')} + ${new Map()} | ${sexp('Map', '...')} + ${new Proxy({}, {})} | ${sexp('Proxy', '')} + ${new Proxy([], {})} | ${sexp('Proxy', '')} + ${new Proxy(new SomeClass(), {})} | ${sexp('Proxy', '')} + ${new Proxy(someFunc, {})} | ${sexp('Proxy', 'someFunc')} + `('($#) correctly encodes $value', ({ value, expected }) => { + const got = LoggedValueEncoder.encode(value); + expect(got).toStrictEqual(expected); + }); + + test('does not def-ref a small-enough array', () => { + const value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + const got = LoggedValueEncoder.encode([value, value]); + expect(got).toStrictEqual([value, value]); + }); + + test('does not def-ref a small-enough plain object', () => { + const value = { a: 1, b: 2, c: 3, d: 4, e: 5, f: 6, g: 7, h: 8, i: 9, j: 10 }; + const expected = withNullObjectProtos(value); + const got = LoggedValueEncoder.encode([value, value]); + expect(got).toStrictEqual([expected, expected]); + }); + + test('def-refs a large-enough array', () => { + const value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]; + const def = new VisitDef(0, value); + const expected = [def, new VisitRef(def)]; + const got = LoggedValueEncoder.encode([value, value]); + expect(got).toStrictEqual(expected); + }); + + test('def-refs a large-enough plain object', () => { + const value = { a: 1, b: 2, c: 3, d: 4, e: 5, f: 6, g: 7, h: 8, i: 9, j: 10, k: 11 }; + const def = new VisitDef(0, withNullObjectProtos(value)); + const expected = [def, new VisitRef(def)]; + const got = LoggedValueEncoder.encode([value, value]); + + expect(got).toBeArrayOfSize(2); + expect(got[0]).toStrictEqual(expected[0]); + expect(got[1]).toStrictEqual(expected[1]); + expect(got).toStrictEqual(expected); + }); + + test('def-refs the sexp from an instance', () => { + const value = new Map(); + const def = new VisitDef(0, sexp('Map', '...')); + const expected = [def, new VisitRef(def)]; + const got = LoggedValueEncoder.encode([value, value]); + expect(got).toStrictEqual(expected); + }); + + test('def-refs an array with a self-reference', () => { + const value = [123]; + value.push(value); + + const got = LoggedValueEncoder.encode(value); + + // Jest can't compare self-referential structures (it recurses forever), so + // we have to do it "manually." + + expect(got).toBeInstanceOf(VisitDef); + expect(got.index).toBe(0); + + const gotValue = got.value; + expect(gotValue).toBeArrayOfSize(2); + expect(gotValue[0]).toBe(123); + expect(gotValue[1]).toBeInstanceOf(VisitRef); + expect(gotValue[1].def).toBe(got); + }); +}); diff --git a/src/main-tester/index.js b/src/main-tester/index.js index b6d287bc0..6ee8ed723 100644 --- a/src/main-tester/index.js +++ b/src/main-tester/index.js @@ -1,6 +1,9 @@ // Copyright 2022-2024 the Lactoserv Authors (Dan Bornstein et alia). // SPDX-License-Identifier: Apache-2.0 +import { inspect } from 'node:util'; + + process.on('warning', (warning) => { if (warning.name === 'ExperimentalWarning') { if (/VM Modules/.test(warning.message)) { @@ -13,3 +16,51 @@ process.on('warning', (warning) => { // eslint-disable-next-line no-console console.log('%s: %s\n', warning.name, warning.message); }); + +// This extends Jest's equality checks to deal reasonably with a few more cases +// than it does by default, by leaning on common patterns in this codebase. + +function lactoservEquals(a, b, customTesters) { + // Note: `return undefined` means, "The comparison is not handled by this + // tester." + + if ((typeof a !== 'object') || (typeof b !== 'object')) { + return undefined; + } else if ((a === null) || (b === null)) { + return undefined; + } else if (Array.isArray(a) || Array.isArray(b)) { + return undefined; + } + + const aProto = Reflect.getPrototypeOf(a); + const bProto = Reflect.getPrototypeOf(b); + + if ((aProto === null) || (aProto !== bProto)) { + return undefined; + } + + // At this point, we're looking at two non-array objects of the same class + // (that is, with the same prototype). + + if (typeof aProto.deconstruct === 'function') { + // They (effectively) implement `IntfDeconstructable`. Note: There is no + // need to check the `functor` of the deconstructed result, since it's + // going to be `aProto` (which is also `bProto`). + const aDecon = a.deconstruct(true).args; + const bDecon = b.deconstruct(true).args; + return this.equals(aDecon, bDecon, customTesters); + } else if (typeof aProto[inspect.custom] === 'function') { + // They have a custom inspector. + return this.equals(inspect(a), inspect(b)); + } else if (typeof aProto.toJSON === 'function') { + // They have a custom JSON encoder. This is the least-preferable option + // because it's the most likely to end up losing information. + return this.equals(a.toJSON(), b.toJSON(), customTesters); + } + + return undefined; +} + +// The linter doesn't realize this file is loaded in the context of testing, so +// it would complain that `expect` is undefined without the disable directive. +expect.addEqualityTesters([lactoservEquals]); // eslint-disable-line no-undef diff --git a/src/net-util/export/DispatchInfo.js b/src/net-util/export/DispatchInfo.js index 8dfc037ce..b2c79fc6c 100644 --- a/src/net-util/export/DispatchInfo.js +++ b/src/net-util/export/DispatchInfo.js @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import { PathKey } from '@this/collections'; -import { IntfDeconstructable, Sexp } from '@this/sexp'; import { IntfLogger } from '@this/loggy-intf'; +import { IntfDeconstructable, Sexp } from '@this/sexp'; import { MustBe } from '@this/typey'; import { IncomingRequest } from '#x/IncomingRequest'; diff --git a/src/sexp/tests/Sexp.test.js b/src/sexp/tests/Sexp.test.js index dde7611cf..b52153ebb 100644 --- a/src/sexp/tests/Sexp.test.js +++ b/src/sexp/tests/Sexp.test.js @@ -181,3 +181,27 @@ describe('.toArray()', () => { expect(sexp.toArray()).toStrictEqual(expected); }); }); + +// This validates that it's safe to use `expect(sexp).toStrictEqual(otherSexp)` +// in test cases throughout the system. +describe('validating Jest usage', () => { + test('can use `expect().toStrictEqual()` to check `functor`', () => { + const sexp1a = new Sexp('x', 1, 2, 3); + const sexp1b = new Sexp('x', 1, 2, 3); + const sexp2 = new Sexp('y', 1, 2, 3); + + expect(sexp1a).toStrictEqual(sexp1a); + expect(sexp1a).toStrictEqual(sexp1b); + expect(sexp1a).not.toStrictEqual(sexp2); + }); + + test('can use `expect().toStrictEqual()` to check `args`', () => { + const sexp1a = new Sexp('x', 1, 2, 3); + const sexp1b = new Sexp('x', 1, 2, 3); + const sexp2 = new Sexp('x', 1, 2, 3, 'floop'); + + expect(sexp1a).toStrictEqual(sexp1a); + expect(sexp1a).toStrictEqual(sexp1b); + expect(sexp1a).not.toStrictEqual(sexp2); + }); +}); diff --git a/src/valvis/export/BaseDefRef.js b/src/valvis/export/BaseDefRef.js index d652ee369..231885041 100644 --- a/src/valvis/export/BaseDefRef.js +++ b/src/valvis/export/BaseDefRef.js @@ -1,8 +1,10 @@ // Copyright 2022-2024 the Lactoserv Authors (Dan Bornstein et alia). // SPDX-License-Identifier: Apache-2.0 +import { IntfDeconstructable } from '@this/sexp'; import { Methods } from '@this/typey'; + /** * Forward declaration of this class, because `import`ing it would cause a * circular dependency while loading. @@ -25,8 +27,9 @@ import { Methods } from '@this/typey'; * implementation details. * * @abstract + * @implements {IntfDeconstructable} */ -export class BaseDefRef { +export class BaseDefRef extends IntfDeconstructable { /** * The reference index number. * @@ -40,6 +43,7 @@ export class BaseDefRef { * @param {number} index The reference index number. */ constructor(index) { + super(); this.#index = index; } diff --git a/src/valvis/export/BaseValueVisitor.js b/src/valvis/export/BaseValueVisitor.js index 3a7d8998f..ed221014d 100644 --- a/src/valvis/export/BaseValueVisitor.js +++ b/src/valvis/export/BaseValueVisitor.js @@ -68,14 +68,6 @@ export class BaseValueVisitor { */ #allRefs = []; - /** - * The set of visit entries that are actively being visited. This is used to - * detect attempts to visit a value containing a circular reference. - * - * @type {Set} - */ - #activeVisits = new Set(); - /** * Constructs an instance whose purpose in life is to visit the indicated * value. @@ -335,10 +327,12 @@ export class BaseValueVisitor { * `false` for everything else. * * @param {*} value The value to check. + * @param {boolean} isCycleHead Was `value` just detected as the head of a + * reference cyle? * @returns {boolean} `true` if `value` should be converted into a reference. * or `false` if not. */ - _impl_shouldRef(value) { // eslint-disable-line no-unused-vars + _impl_shouldRef(value, isCycleHead) { // eslint-disable-line no-unused-vars return false; } @@ -826,13 +820,13 @@ export class BaseValueVisitor { const already = this.#visitEntries.get(node); if (already) { - let ref = already.ref; + const isCycleHead = !already.isFinished(); + let ref = already.ref; - if (ref || already.shouldRef()) { + if (ref || already.shouldRef(isCycleHead)) { // We either already have a ref, or we are supposed to make a ref. - const isCycleHead = !already.isFinished(); - const result = isCycleHead ? null : already.extractSync(false); + const result = isCycleHead ? null : already.extractSync(false); if (!ref) { already.setDefRef(this.#allRefs.length); @@ -843,7 +837,7 @@ export class BaseValueVisitor { this._impl_revisit(node, result, isCycleHead, ref); return this.#visitNode(ref); - } else if (this.#activeVisits.has(already)) { + } else if (isCycleHead) { // We have encountered the head of a reference cycle that was _not_ // handled by making a "ref" object for the back-reference. @@ -1181,10 +1175,12 @@ export class BaseValueVisitor { * {@link #_impl_shouldRef} is never called more than once per original * value. * + * @param {boolean} isCycleHead Was this entry's value just detected as the + * head of a reference cyle? * @returns {boolean} `true` iff repeat visits to this instance should * result in a ref to this instance's result. */ - shouldRef() { + shouldRef(isCycleHead) { if (this.#shouldRef === null) { const visitor = this.#visitor; const node = this.#node; @@ -1195,20 +1191,29 @@ export class BaseValueVisitor { case 'function': case 'string': case 'symbol': { - result = visitor._impl_shouldRef(node); // eslint-disable-line no-restricted-syntax + result = visitor._impl_shouldRef(node, isCycleHead); // eslint-disable-line no-restricted-syntax break; } case 'object': { if ((node !== null) && !visitor.#isAssociatedDefOrRef(node)) { - result = visitor._impl_shouldRef(node); // eslint-disable-line no-restricted-syntax + result = visitor._impl_shouldRef(node, isCycleHead); // eslint-disable-line no-restricted-syntax } break; } } this.#shouldRef = result; + } else if (isCycleHead) { + /* c8 ignore start */ + // Shouldn't happen: By construction, the moment a second reference to a + // cycle head is encountered, this method should end up getting called + // with `isCycleHead === true`, and if the `_impl` declined to make a + // ref, the whole visit should end up erroring out with an error along + // the lines of "can't visit circular reference." + throw new Error('Shouldn\'t happen: Cycle head detected too late.'); + /* c8 ignore stop */ } return this.#shouldRef; @@ -1224,7 +1229,6 @@ export class BaseValueVisitor { // about circular references. this.#promise = (async () => { const visitor = this.#visitor; - visitor.#activeVisits.add(this); try { let result = visitor.#visitNode0(this.#node); @@ -1244,8 +1248,6 @@ export class BaseValueVisitor { this.#finishWithError(e); } - visitor.#activeVisits.delete(this); - return this; })(); } diff --git a/src/valvis/export/VisitDef.js b/src/valvis/export/VisitDef.js index ff11404a9..03501e145 100644 --- a/src/valvis/export/VisitDef.js +++ b/src/valvis/export/VisitDef.js @@ -1,6 +1,8 @@ // Copyright 2022-2024 the Lactoserv Authors (Dan Bornstein et alia). // SPDX-License-Identifier: Apache-2.0 +import { Sexp } from '@this/sexp'; + import { BaseDefRef } from '#x/BaseDefRef'; import { VisitRef } from '#x/VisitRef'; @@ -86,6 +88,28 @@ export class VisitDef extends BaseDefRef { } } + /** @override */ + deconstruct(forLogging) { + let valueArgs; + + if (this.#finished) { + if (this.#error) { + if (!forLogging) { + // Can't deconstruct an errored def when _not_ logging, as there's no + // way to construct such a one. + this.value; // This will throw. + } + valueArgs = ['error', this.#error]; + } else { + valueArgs = [this.#value]; + } + } else { + valueArgs = []; + } + + return new Sexp(this.constructor, this.index, ...valueArgs); + } + /** * Indicates that this instance's visit has now finished unsuccessfully with * the given error. It is only ever valid to call this on an unfinished diff --git a/src/valvis/export/VisitRef.js b/src/valvis/export/VisitRef.js index d33da5c17..65bb787e1 100644 --- a/src/valvis/export/VisitRef.js +++ b/src/valvis/export/VisitRef.js @@ -1,6 +1,9 @@ // Copyright 2022-2024 the Lactoserv Authors (Dan Bornstein et alia). // SPDX-License-Identifier: Apache-2.0 +import { Sexp } from '@this/sexp'; +import { MustBe } from '@this/typey'; + import { BaseDefRef } from '#x/BaseDefRef'; import { VisitDef } from '#x/VisitDef'; @@ -28,6 +31,8 @@ export class VisitRef extends BaseDefRef { * @param {VisitDef} def The corresponding def. */ constructor(def) { + MustBe.instanceOf(def, VisitDef); + const valueArg = def.isFinished() ? [def.value] : []; super(def.index, ...valueArg); @@ -49,6 +54,11 @@ export class VisitRef extends BaseDefRef { return this.#def.value; } + /** @override */ + deconstruct(forLogging_unused) { + return new Sexp(this.constructor, this.#def); + } + /** @override */ isFinished() { return this.#def.isFinished(); diff --git a/src/valvis/tests/BaseValueVisitor.test.js b/src/valvis/tests/BaseValueVisitor.test.js index a90210bc9..18b8342b0 100644 --- a/src/valvis/tests/BaseValueVisitor.test.js +++ b/src/valvis/tests/BaseValueVisitor.test.js @@ -67,7 +67,7 @@ class ProxyAwareVisitor extends BaseValueVisitor { * Visitor subclass, which is set up to use refs for duplicate objects. */ class RefMakingVisitor extends BaseValueVisitor { - _impl_shouldRef(value) { + _impl_shouldRef(value, isCycleHead_unused) { return (typeof value === 'object'); } @@ -516,7 +516,7 @@ ${'visitAsyncWrap'} | ${true} | ${false} | ${true} const MSG = 'Not today, Satan!'; class TestVisitor extends BaseValueVisitor { - _impl_shouldRef(node_unused) { + _impl_shouldRef(node_unused, isCycleHead_unused) { return true; } @@ -699,7 +699,7 @@ ${'visitAsyncWrap'} | ${true} | ${false} | ${true} class TestVisitor extends BaseValueVisitor { calledOn = []; - _impl_shouldRef(node) { + _impl_shouldRef(node, isCycleHead_unused) { this.calledOn.push(node); return false; } @@ -747,7 +747,16 @@ ${'visitAsyncWrap'} | ${true} | ${false} | ${true} describe('when `_impl_shouldRef()` can return `true`', () => { class TestVisitor extends RefMakingVisitor { - refs = []; + cycleHeads = []; + refs = []; + + _impl_shouldRef(node, isCycleHead) { + if (isCycleHead) { + this.cycleHeads.push(node); + } + + return super._impl_shouldRef(node, isCycleHead); + } _impl_newRef(ref) { this.refs.push({ ref, wasFinished: ref.isFinished() }); @@ -768,6 +777,8 @@ ${'visitAsyncWrap'} | ${true} | ${false} | ${true} expect(ref.value).toBe(got[1][1]); expect(visitor.getVisitResult(shared)).toBe(ref.value); expect(wasFinished).toBeTrue(); + + expect(visitor.cycleHeads).toBeArrayOfSize(0); } }); }); @@ -789,6 +800,9 @@ ${'visitAsyncWrap'} | ${true} | ${false} | ${true} expect(visitor.getVisitResult(selfRef)).toBe(ref.value); expect(ref).toBe(ref.value[2]); expect(wasFinished).toBeFalse(); + + expect(visitor.cycleHeads).toBeArrayOfSize(1); + expect(visitor.cycleHeads[0]).toBe(selfRef); } }); }); @@ -819,7 +833,7 @@ describe('_impl_revisit()', () => { this.#doRefs = doRefs; } - _impl_shouldRef(node) { + _impl_shouldRef(node, isCycleHead_unused) { return this.#doRefs && (typeof node === 'object'); } diff --git a/src/valvis/tests/VisitDef.test.js b/src/valvis/tests/VisitDef.test.js index 50d612de6..9a89a78df 100644 --- a/src/valvis/tests/VisitDef.test.js +++ b/src/valvis/tests/VisitDef.test.js @@ -1,6 +1,7 @@ // Copyright 2022-2024 the Lactoserv Authors (Dan Bornstein et alia). // SPDX-License-Identifier: Apache-2.0 +import { Sexp } from '@this/sexp'; import { VisitDef, VisitRef } from '@this/valvis'; @@ -101,6 +102,59 @@ ${'finishWithValue'} | ${'florp'} }); }); +describe('deconstruct()', () => { + test('works on an unfinished instance', () => { + const def = new VisitDef(1); + const expected = new Sexp(VisitDef, 1); + expect(def.deconstruct()).toStrictEqual(expected); + }); + + test('works on a (non-error) finished instance', () => { + const def = new VisitDef(2, 'beep'); + const expected = new Sexp(VisitDef, 2, 'beep'); + expect(def.deconstruct()).toStrictEqual(expected); + }); + + test('works on an error-finished instance when given `forLogging === true`', () => { + const def = new VisitDef(3); + const error = new Error('eeeek!'); + const expected = new Sexp(VisitDef, 3, 'error', error); + def.finishWithError(error); + expect(def.deconstruct(true)).toStrictEqual(expected); + }); + + test('throws on an error-finished instance when given `forLogging === false`', () => { + const def = new VisitDef(4); + const error = new Error('eeeek!'); + def.finishWithError(error); + expect(() => def.deconstruct()).toThrow(); + }); +}); + +describe('isFinished()', () => { + test('is `false` on an instance constructed without a value', () => { + const def = new VisitDef(901); + expect(def.isFinished()).toBeFalse(); + }); + + test('is `true` on an instance constructed with a value', () => { + const def = new VisitDef(902, 'bloop'); + expect(def.isFinished()).toBeTrue(); + }); + + test('is `true` on an instance which became finished via `finishWithValue()`', () => { + const def = new VisitDef(903); + def.finishWithValue('bleep'); + expect(def.isFinished()).toBeTrue(); + }); + + test('is `true` on an instance which became finished via `finishWithError()`', () => { + const def = new VisitDef(904); + def.finishWithError(new Error('oy!')); + expect(def.isFinished()).toBeTrue(); + }); +}); + describe('.toJSON()', () => { test('returns the expected replacement for a value-bearing instance', () => { const def = new VisitDef(20, 'bongo'); @@ -119,3 +173,46 @@ describe('.toJSON()', () => { expect(def.toJSON()).toStrictEqual({ '@def': [22, null, 'Eek!'] }); }); }); + +// This validates that it's safe to use `expect(def).toStrictEqual(def)` +// in test cases throughout the system. +describe('validating Jest usage', () => { + test('can use `expect().toStrictEqual()` to check `index`es', () => { + const def1a = new VisitDef(1, 'boop'); + const def1b = new VisitDef(1, 'boop'); + const def2 = new VisitDef(2, 'boop'); + + expect(def1a).toStrictEqual(def1a); + expect(def1a).toStrictEqual(def1b); + expect(def1a).not.toStrictEqual(def2); + }); + + test('can use `expect().toStrictEqual()` to check finished `value`s', () => { + const def1a = new VisitDef(1, 'boop'); + const def1b = new VisitDef(1, 'boop'); + const def2 = new VisitDef(1, 'zonkers'); + const def3 = new VisitDef(1); + + expect(def1a).toStrictEqual(def1a); + expect(def1a).toStrictEqual(def1b); + expect(def1a).not.toStrictEqual(def2); + expect(def1a).not.toStrictEqual(def3); + }); + + test('can use `expect().toStrictEqual()` to check finished `error`s', () => { + const def1a = new VisitDef(1); + const def1b = new VisitDef(1); + const def2 = new VisitDef(1); + const def3 = new VisitDef(1, 'good'); + + const error1 = new Error('oy 1'); + def1a.finishWithError(error1); + def1b.finishWithError(error1); + def2.finishWithError(new Error('oy 2')); + + expect(def1a).toStrictEqual(def1a); + expect(def1a).toStrictEqual(def1b); + expect(def1a).not.toStrictEqual(def2); + expect(def1a).not.toStrictEqual(def3); + }); +}); diff --git a/src/valvis/tests/VisitRef.test.js b/src/valvis/tests/VisitRef.test.js index df30a937e..b1d31423d 100644 --- a/src/valvis/tests/VisitRef.test.js +++ b/src/valvis/tests/VisitRef.test.js @@ -1,6 +1,7 @@ // Copyright 2022-2024 the Lactoserv Authors (Dan Bornstein et alia). // SPDX-License-Identifier: Apache-2.0 +import { Sexp } from '@this/sexp'; import { VisitDef, VisitRef } from '@this/valvis'; @@ -33,9 +34,62 @@ describe('.ref', () => { }); }); +describe('deconstruct()', () => { + test('returns the `def` from the constructor', () => { + const def = new VisitDef(456, 'floop'); + const ref = new VisitRef(def); + const expected = new Sexp(VisitRef, def); + expect(ref.deconstruct()).toStrictEqual(expected); + }); +}); + +describe('isFinished()', () => { + test('is `false` when the associated `def` is unfinished', () => { + const def = new VisitDef(901); + const ref = new VisitRef(def); + expect(ref.isFinished()).toBeFalse(); + }); + + test('is `true` when the associated `def` is finished', () => { + const def = new VisitDef(902, 'bloop'); + const ref = new VisitRef(def); + expect(ref.isFinished()).toBeTrue(); + }); +}); + describe('.toJSON()', () => { test('returns the expected replacement', () => { const ref = new VisitRef(new VisitDef(2, null)); expect(ref.toJSON()).toStrictEqual({ '@ref': [2] }); }); }); + +// This validates that it's safe to use `expect(ref).toStrictEqual(ref)` +// in test cases throughout the system. +describe('validating Jest usage', () => { + test('can use `expect().toStrictEqual()` to check the defs\' `index`', () => { + const def1a = new VisitDef(1, 'boop'); + const def1b = new VisitDef(1, 'boop'); + const def2 = new VisitDef(2, 'boop'); + const ref1a = new VisitRef(def1a); + const ref1b = new VisitRef(def1b); + const ref2 = new VisitRef(def2); + + expect(ref1a).toStrictEqual(ref1a); + expect(ref1a).toStrictEqual(ref1b); + expect(ref1a).not.toStrictEqual(ref2); + }); + + test('can use `expect().toStrictEqual()` to check the defs\' `value`', () => { + const def1a = new VisitDef(1, 'boop'); + const def1b = new VisitDef(1, 'boop'); + const def2 = new VisitDef(1, 'zonkers'); + const ref1a = new VisitRef(def1a); + const ref1b = new VisitRef(def1b); + const ref2 = new VisitRef(def2); + + expect(ref1a).toStrictEqual(ref1a); + expect(ref1a).toStrictEqual(ref1b); + expect(ref1a).not.toStrictEqual(ref2); + }); +});