From b7312bb0cd920f7f10abcc70878b222c973d8dc1 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Mon, 10 Oct 2022 22:41:15 -0700 Subject: [PATCH] fix: better split errors --- .../test/unitTests/test-inputValidation.js | 15 +++++---- .../ERTP/test/unitTests/test-issuerObj.js | 6 ++-- packages/ERTP/test/unitTests/test-mintObj.js | 2 +- .../test/unitTests/test-paramGovernance.js | 2 +- .../store/src/patterns/patternMatchers.js | 31 ++++++++++++++++--- packages/store/test/test-heap-classes.js | 15 ++++----- packages/store/test/test-patterns.js | 18 +++++------ .../vat-data/test/test-durable-classes.js | 10 +++--- .../vat-data/test/test-virtual-classes.js | 10 +++--- packages/vat-data/test/test-vivify.js | 15 ++++----- .../zoe/test/unitTests/test-cleanProposal.js | 16 +++++----- 11 files changed, 75 insertions(+), 65 deletions(-) diff --git a/packages/ERTP/test/unitTests/test-inputValidation.js b/packages/ERTP/test/unitTests/test-inputValidation.js index c192eb598939..d6e271247297 100644 --- a/packages/ERTP/test/unitTests/test-inputValidation.js +++ b/packages/ERTP/test/unitTests/test-inputValidation.js @@ -33,8 +33,7 @@ test('makeIssuerKit bad displayInfo.decimalPlaces', async t => { harden({ decimalPlaces: 'hello' }), ), { - message: - 'displayInfo: optional: decimalPlaces: "hello" - Must be >= -100', + message: 'displayInfo: decimalPlaces?: "hello" - Must be >= -100', }, ); @@ -62,7 +61,7 @@ test('makeIssuerKit bad displayInfo.decimalPlaces', async t => { () => makeIssuerKit('myTokens', AssetKind.NAT, harden({ decimalPlaces: 101 })), { - message: 'displayInfo: optional: decimalPlaces: 101 - Must be <= 100', + message: 'displayInfo: decimalPlaces?: 101 - Must be <= 100', }, ); @@ -70,7 +69,7 @@ test('makeIssuerKit bad displayInfo.decimalPlaces', async t => { () => makeIssuerKit('myTokens', AssetKind.NAT, harden({ decimalPlaces: -101 })), { - message: 'displayInfo: optional: decimalPlaces: -101 - Must be >= -100', + message: 'displayInfo: decimalPlaces?: -101 - Must be >= -100', }, ); }); @@ -88,7 +87,7 @@ test('makeIssuerKit bad displayInfo.assetKind', async t => { ), { message: - 'displayInfo: optional: assetKind: "something" - Must match one of ["nat","set","copySet","copyBag"]', + 'displayInfo: assetKind?: "something" - Must match one of ["nat","set","copySet","copyBag"]', }, ); }); @@ -105,7 +104,7 @@ test('makeIssuerKit bad displayInfo.whatever', async t => { }), ), { - message: 'displayInfo: rest: {"whatever":"something"} - Must be: {}', + message: 'displayInfo: ...rest: {"whatever":"something"} - Must be: {}', }, ); }); @@ -142,7 +141,7 @@ test('brand.isMyIssuer bad issuer', async t => { // @ts-expect-error Intentional wrong type for testing t.throwsAsync(() => brand.isMyIssuer('not an issuer'), { message: - 'In "isMyIssuer" method of (myTokens brand): args: [0]: string "not an issuer" - Must be a remotable (Issuer)', + 'In "isMyIssuer" method of (myTokens brand): arg 0: string "not an issuer" - Must be a remotable (Issuer)', }); const fakeIssuer = /** @type {Issuer} */ ( /** @type {unknown} */ (Far('myTokens issuer', {})) @@ -195,7 +194,7 @@ test('issuer.combine bad payments array', async t => { // @ts-expect-error Intentional wrong type for testing await t.throwsAsync(() => E(issuer).combine(notAnArray2), { message: - 'In "combine" method of (fungible issuer): args: [0]: remotable "[Alleged: notAnArray2]" - Must be a copyArray', + 'In "combine" method of (fungible issuer): arg 0: remotable "[Alleged: notAnArray2]" - Must be a copyArray', }); }); diff --git a/packages/ERTP/test/unitTests/test-issuerObj.js b/packages/ERTP/test/unitTests/test-issuerObj.js index f5e924f3d8d1..ec255b60fa4a 100644 --- a/packages/ERTP/test/unitTests/test-issuerObj.js +++ b/packages/ERTP/test/unitTests/test-issuerObj.js @@ -44,7 +44,7 @@ test('bad display info', t => { const displayInfo = harden({ somethingUnexpected: 3 }); // @ts-expect-error deliberate invalid arguments for testing t.throws(() => makeIssuerKit('fungible', AssetKind.NAT, displayInfo), { - message: 'displayInfo: rest: {"somethingUnexpected":3} - Must be: {}', + message: 'displayInfo: ...rest: {"somethingUnexpected":3} - Must be: {}', }); }); @@ -200,7 +200,7 @@ test('purse.deposit promise', async t => { () => E(purse).deposit(exclusivePaymentP, fungible25), { message: - 'In "deposit" method of (fungible Purse purse): args: [0]: promise "[Promise]" - Must be a remotable (Payment)', + 'In "deposit" method of (fungible Purse purse): arg 0: promise "[Promise]" - Must be a remotable (Payment)', }, 'failed to reject a promise for a payment', ); @@ -335,7 +335,7 @@ test('issuer.split bad amount', async t => { _ => E(issuer).split(payment, AmountMath.make(otherBrand, 10n)), { message: - 'In "split" method of (fungible issuer): args: [1]: brand: "[Alleged: other fungible brand]" - Must be: "[Alleged: fungible brand]"', + 'In "split" method of (fungible issuer): arg 1: brand: "[Alleged: other fungible brand]" - Must be: "[Alleged: fungible brand]"', }, 'throws for bad amount', ); diff --git a/packages/ERTP/test/unitTests/test-mintObj.js b/packages/ERTP/test/unitTests/test-mintObj.js index 2ad45a334433..d4c91684e129 100644 --- a/packages/ERTP/test/unitTests/test-mintObj.js +++ b/packages/ERTP/test/unitTests/test-mintObj.js @@ -46,7 +46,7 @@ test('mint.mintPayment set w strings AssetKind', async t => { const badAmount = AmountMath.make(brand, harden([['badElement']])); t.throws(() => mint.mintPayment(badAmount), { message: - 'In "mintPayment" method of (items mint): args: [0]: value: [0]: copyArray ["badElement"] - Must be a string', + 'In "mintPayment" method of (items mint): arg 0: value: [0]: copyArray ["badElement"] - Must be a string', }); }); diff --git a/packages/governance/test/unitTests/test-paramGovernance.js b/packages/governance/test/unitTests/test-paramGovernance.js index 38744faaa52d..2c2f6b3c5341 100644 --- a/packages/governance/test/unitTests/test-paramGovernance.js +++ b/packages/governance/test/unitTests/test-paramGovernance.js @@ -220,7 +220,7 @@ test('multiple params bad change', async t => { ), { message: - 'In "getAmountOf" method of (Zoe Invitation issuer): args: [0]: bigint "[13n]" - Must be a remotable (Payment)', + 'In "getAmountOf" method of (Zoe Invitation issuer): arg 0: bigint "[13n]" - Must be a remotable (Payment)', }, ); }); diff --git a/packages/store/src/patterns/patternMatchers.js b/packages/store/src/patterns/patternMatchers.js index 9534426c1283..fd8302f707ec 100644 --- a/packages/store/src/patterns/patternMatchers.js +++ b/packages/store/src/patterns/patternMatchers.js @@ -1579,10 +1579,24 @@ const makePatternKit = () => { optionalPatt, optionalSpecimen.length, ); + let argNum = 0; return ( - checkMatches(requiredSpecimen, requiredPatt, check, 'args') && - checkMatches(optionalSpecimen, partialPatt, check, 'optional args') && - checkMatches(restSpecimen, restPatt, check, 'rest args') + (requiredSpecimen.length === requiredPatt.length || + check( + false, + X`Expected at least ${q( + requiredPatt.length, + )} arguments: ${specimen}`, + )) && + requiredPatt.every((p, i) => + // eslint-disable-next-line no-plusplus + checkMatches(requiredSpecimen[i], p, check, `arg ${argNum++}`), + ) && + partialPatt.every((p, i) => + // eslint-disable-next-line no-plusplus + checkMatches(optionalSpecimen[i], p, check, `arg ${argNum++}?`), + ) && + checkMatches(restSpecimen, restPatt, check, '...rest') ); }, @@ -1728,8 +1742,15 @@ const makePatternKit = () => { const partialPatt = adaptRecordPattern(optionalPatt, partialNames); return ( checkMatches(requiredSpecimen, requiredPatt, check) && - checkMatches(optionalSpecimen, partialPatt, check, 'optional') && - checkMatches(restSpecimen, restPatt, check, 'rest') + partialNames.every(name => + checkMatches( + optionalSpecimen[name], + partialPatt[name], + check, + `${name}?`, + ), + ) && + checkMatches(restSpecimen, restPatt, check, '...rest') ); }, diff --git a/packages/store/test/test-heap-classes.js b/packages/store/test/test-heap-classes.js index 4bffd29cc0de..c019843463cb 100644 --- a/packages/store/test/test-heap-classes.js +++ b/packages/store/test/test-heap-classes.js @@ -40,13 +40,12 @@ test('test defineHeapFarClass', t => { t.is(upCounter.incr(5), 8); t.is(upCounter.incr(1), 9); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (UpCounter): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (UpCounter): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error bad arg t.throws(() => upCounter.incr('foo'), { message: - 'In "incr" method of (UpCounter): optional args: [0]: string "foo" - Must be a number', + 'In "incr" method of (UpCounter): arg 0?: string "foo" - Must be a number', }); }); @@ -79,13 +78,12 @@ test('test defineHeapFarClassKit', t => { t.is(downCounter.decr(), 7); t.is(upCounter.incr(3), 10); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (Counter up): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (Counter up): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error the type violation is what we're testing t.throws(() => downCounter.decr('foo'), { message: - 'In "decr" method of (Counter down): optional args: [0]: string "foo" - Must be a number', + 'In "decr" method of (Counter down): arg 0?: string "foo" - Must be a number', }); // @ts-expect-error bad arg t.throws(() => upCounter.decr(3), { @@ -104,12 +102,11 @@ test('test makeHeapFarInstance', t => { t.is(upCounter.incr(5), 8); t.is(upCounter.incr(1), 9); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (upCounter): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (upCounter): arg 0?: -3 - Must be >= 0', }); t.throws(() => upCounter.incr('foo'), { message: - 'In "incr" method of (upCounter): optional args: [0]: string "foo" - Must be a number', + 'In "incr" method of (upCounter): arg 0?: string "foo" - Must be a number', }); }); diff --git a/packages/store/test/test-patterns.js b/packages/store/test/test-patterns.js index 6321fc5c7caf..be4eff2cff18 100644 --- a/packages/store/test/test-patterns.js +++ b/packages/store/test/test-patterns.js @@ -84,13 +84,13 @@ const runTests = (successCase, failCase) => { failCase( specimen, M.split([3, 4, 5, 6]), - 'args: [3,4] - Must be: [3,4,5,6]', + 'Expected at least 4 arguments: [3,4]', ); - failCase(specimen, M.split([5]), 'args: [3] - Must be: [5]'); + failCase(specimen, M.split([5]), 'arg 0: 3 - Must be: 5'); failCase(specimen, M.split({}), 'copyArray [3,4] - Must be a copyRecord'); - failCase(specimen, M.split([3], 'x'), 'rest args: [4] - Must be: "x"'); + failCase(specimen, M.split([3], 'x'), '...rest: [4] - Must be: "x"'); - failCase(specimen, M.partial([5]), 'optional args: [0]: 3 - Must be: 5'); + failCase(specimen, M.partial([5]), 'arg 0?: 3 - Must be: 5'); failCase( specimen, @@ -199,7 +199,7 @@ const runTests = (successCase, failCase) => { failCase( specimen, M.splitRecord({ foo: M.number() }, { bar: M.string(), baz: M.number() }), - 'optional: bar: number 4 - Must be a string', + 'bar?: number 4 - Must be a string', ); failCase( @@ -208,7 +208,7 @@ const runTests = (successCase, failCase) => { { foo: M.number() }, M.and(M.partial({ bar: M.string() }), M.partial({ baz: M.number() })), ), - 'rest: optional: bar: number 4 - Must be a string', + '...rest: bar?: number 4 - Must be a string', ); failCase( @@ -224,17 +224,17 @@ const runTests = (successCase, failCase) => { failCase( specimen, M.split({ foo: 3 }, { foo: 3, bar: 4 }), - 'rest: {"bar":4} - Must be: {"foo":3,"bar":4}', + '...rest: {"bar":4} - Must be: {"foo":3,"bar":4}', ); failCase( specimen, M.split({ foo: 3 }, { foo: M.any(), bar: 4 }), - 'rest: {"bar":4} - Must have missing properties ["foo"]', + '...rest: {"bar":4} - Must have missing properties ["foo"]', ); failCase( specimen, M.partial({ foo: 7, zip: 5 }, { bar: 4 }), - 'optional: foo: 3 - Must be: 7', + 'foo?: 3 - Must be: 7', ); failCase( diff --git a/packages/vat-data/test/test-durable-classes.js b/packages/vat-data/test/test-durable-classes.js index c4e2f77ba1a9..1dc969370aa2 100644 --- a/packages/vat-data/test/test-durable-classes.js +++ b/packages/vat-data/test/test-durable-classes.js @@ -44,13 +44,12 @@ test('test defineDurableFarClass', t => { t.is(upCounter.incr(5), 8); t.is(upCounter.incr(1), 9); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (UpCounter): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (UpCounter): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error the type violation is what we're testing t.throws(() => upCounter.incr('foo'), { message: - 'In "incr" method of (UpCounter): optional args: [0]: string "foo" - Must be a number', + 'In "incr" method of (UpCounter): arg 0?: string "foo" - Must be a number', }); }); @@ -83,13 +82,12 @@ test('test defineDurableFarClassKit', t => { t.is(downCounter.decr(), 7); t.is(upCounter.incr(3), 10); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (Counter up): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (Counter up): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error the type violation is what we're testing t.throws(() => downCounter.decr('foo'), { message: - 'In "decr" method of (Counter down): optional args: [0]: string "foo" - Must be a number', + 'In "decr" method of (Counter down): arg 0?: string "foo" - Must be a number', }); t.throws(() => upCounter.decr(3), { message: 'upCounter.decr is not a function', diff --git a/packages/vat-data/test/test-virtual-classes.js b/packages/vat-data/test/test-virtual-classes.js index e23d12239ac7..77b57765d1b6 100644 --- a/packages/vat-data/test/test-virtual-classes.js +++ b/packages/vat-data/test/test-virtual-classes.js @@ -41,13 +41,12 @@ test('test defineVirtualFarClass', t => { t.is(upCounter.incr(5), 8); t.is(upCounter.incr(1), 9); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (UpCounter): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (UpCounter): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error the type violation is what we're testing t.throws(() => upCounter.incr('foo'), { message: - 'In "incr" method of (UpCounter): optional args: [0]: string "foo" - Must be a number', + 'In "incr" method of (UpCounter): arg 0?: string "foo" - Must be a number', }); }); @@ -78,13 +77,12 @@ test('test defineVirtualFarClassKit', t => { t.is(downCounter.decr(), 7); t.is(upCounter.incr(3), 10); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (Counter up): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (Counter up): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error the type violation is what we're testing t.throws(() => downCounter.decr('foo'), { message: - 'In "decr" method of (Counter down): optional args: [0]: string "foo" - Must be a number', + 'In "decr" method of (Counter down): arg 0?: string "foo" - Must be a number', }); t.throws(() => upCounter.decr(3), { message: 'upCounter.decr is not a function', diff --git a/packages/vat-data/test/test-vivify.js b/packages/vat-data/test/test-vivify.js index 8b75a66b4dd8..a7d31d081e56 100644 --- a/packages/vat-data/test/test-vivify.js +++ b/packages/vat-data/test/test-vivify.js @@ -46,13 +46,12 @@ test('test vivifyFarClass', t => { t.is(upCounter.incr(5), 8); t.is(upCounter.incr(1), 9); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (UpCounter): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (UpCounter): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error the type violation is what we're testing t.throws(() => upCounter.incr('foo'), { message: - 'In "incr" method of (UpCounter): optional args: [0]: string "foo" - Must be a number', + 'In "incr" method of (UpCounter): arg 0?: string "foo" - Must be a number', }); }); @@ -86,13 +85,12 @@ test('test vivifyFarClassKit', t => { t.is(downCounter.decr(), 7); t.is(upCounter.incr(3), 10); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (Counter up): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (Counter up): arg 0?: -3 - Must be >= 0', }); // @ts-expect-error the type violation is what we're testing t.throws(() => downCounter.decr('foo'), { message: - 'In "decr" method of (Counter down): optional args: [0]: string "foo" - Must be a number', + 'In "decr" method of (Counter down): arg 0?: string "foo" - Must be a number', }); t.throws(() => upCounter.decr(3), { message: 'upCounter.decr is not a function', @@ -112,12 +110,11 @@ test('test vivifyFarInstance', t => { t.is(upCounter.incr(5), 8); t.is(upCounter.incr(1), 9); t.throws(() => upCounter.incr(-3), { - message: - 'In "incr" method of (upCounter): optional args: [0]: -3 - Must be >= 0', + message: 'In "incr" method of (upCounter): arg 0?: -3 - Must be >= 0', }); t.throws(() => upCounter.incr('foo'), { message: - 'In "incr" method of (upCounter): optional args: [0]: string "foo" - Must be a number', + 'In "incr" method of (upCounter): arg 0?: string "foo" - Must be a number', }); }); diff --git a/packages/zoe/test/unitTests/test-cleanProposal.js b/packages/zoe/test/unitTests/test-cleanProposal.js index 843e1b679030..3daf18978a33 100644 --- a/packages/zoe/test/unitTests/test-cleanProposal.js +++ b/packages/zoe/test/unitTests/test-cleanProposal.js @@ -228,7 +228,7 @@ test('cleanProposal - other wrong stuff', t => { t, { exit: { onDemand: 'foo' } }, 'nat', - 'proposal: exit: optional: onDemand: "foo" - Must be: null', + 'proposal: exit: onDemand?: "foo" - Must be: null', ); proposeBad( t, @@ -240,43 +240,43 @@ test('cleanProposal - other wrong stuff', t => { t, { exit: { afterDeadline: { timer: 'foo', deadline: 3n } } }, 'nat', - 'proposal: exit: optional: afterDeadline: timer: "foo" - Must match one of ["[match:remotable]","[match:kind]"]', + 'proposal: exit: afterDeadline?: timer: "foo" - Must match one of ["[match:remotable]","[match:kind]"]', ); proposeBad( t, { exit: { afterDeadline: { timer, deadline: 'foo' } } }, 'nat', - 'proposal: exit: optional: afterDeadline: deadline: string "foo" - Must be a bigint', + 'proposal: exit: afterDeadline?: deadline: string "foo" - Must be a bigint', ); proposeBad( t, { exit: { afterDeadline: { timer, deadline: 3n, extra: 'foo' } } }, 'nat', - 'proposal: exit: optional: afterDeadline: {"timer":"[Alleged: ManualTimer]","deadline":"[3n]","extra":"foo"} - Must not have unexpected properties: ["extra"]', + 'proposal: exit: afterDeadline?: {"timer":"[Alleged: ManualTimer]","deadline":"[3n]","extra":"foo"} - Must not have unexpected properties: ["extra"]', ); proposeBad( t, { exit: { afterDeadline: { timer } } }, 'nat', - 'proposal: exit: optional: afterDeadline: {"timer":"[Alleged: ManualTimer]"} - Must have missing properties ["deadline"]', + 'proposal: exit: afterDeadline?: {"timer":"[Alleged: ManualTimer]"} - Must have missing properties ["deadline"]', ); proposeBad( t, { exit: { afterDeadline: { deadline: 3n } } }, 'nat', - 'proposal: exit: optional: afterDeadline: {"deadline":"[3n]"} - Must have missing properties ["timer"]', + 'proposal: exit: afterDeadline?: {"deadline":"[3n]"} - Must have missing properties ["timer"]', ); proposeBad( t, { exit: { afterDeadline: { timer, deadline: 3 } } }, 'nat', - 'proposal: exit: optional: afterDeadline: deadline: number 3 - Must be a bigint', + 'proposal: exit: afterDeadline?: deadline: number 3 - Must be a bigint', ); proposeBad( t, { exit: { afterDeadline: { timer, deadline: -3n } } }, 'nat', - 'proposal: exit: optional: afterDeadline: deadline: "[-3n]" - Must be non-negative', + 'proposal: exit: afterDeadline?: deadline: "[-3n]" - Must be non-negative', ); proposeBad(t, { exit: {} }, 'nat', /exit {} should only have one key/); proposeBad(