From 0de4bcdeae1858041a20ab082b6ef1c4ada39ce2 Mon Sep 17 00:00:00 2001
From: Kate Sills <kate@agoric.com>
Date: Wed, 16 Sep 2020 15:12:46 -0700
Subject: [PATCH] fix: add check so that expected values must be null in
 assertProposalShape (#1788)

* fix: add check so that expected values must be null in assert proposal shape. Add a few more assertions

* chore: remove 'failing' from test that should pass
---
 .../zoe/src/contractSupport/zoeHelpers.js     | 20 +++++++++++++-
 .../test/unitTests/zcf/test-zoeHelpersWZcf.js | 26 +++++++++----------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/packages/zoe/src/contractSupport/zoeHelpers.js b/packages/zoe/src/contractSupport/zoeHelpers.js
index 1e35fe64d0a..72ba265e721 100644
--- a/packages/zoe/src/contractSupport/zoeHelpers.js
+++ b/packages/zoe/src/contractSupport/zoeHelpers.js
@@ -290,8 +290,26 @@ export const swapExact = (
  * @param {ExpectedRecord} expected
  */
 export const assertProposalShape = (seat, expected) => {
+  assert.typeof(expected, 'object');
+  assert(!Array.isArray(expected), `Expected must be an non-array object`);
+  const assertValuesNull = e => {
+    if (e !== undefined) {
+      Object.values(e).forEach(value =>
+        assert(
+          value === null,
+          details`The value of the expected record must be null but was ${value}`,
+        ),
+      );
+    }
+  };
+
+  // Assert values of the expected record are all null. We do not
+  // check the values of the actual proposal.
+  assertValuesNull(expected.give);
+  assertValuesNull(expected.want);
+  assertValuesNull(expected.exit);
+
   const actual = seat.getProposal();
-  // Does not check values
   const assertKeys = (a, e) => {
     if (e !== undefined) {
       assert(
diff --git a/packages/zoe/test/unitTests/zcf/test-zoeHelpersWZcf.js b/packages/zoe/test/unitTests/zcf/test-zoeHelpersWZcf.js
index aab31fc6c2a..ed677b3c5b8 100644
--- a/packages/zoe/test/unitTests/zcf/test-zoeHelpersWZcf.js
+++ b/packages/zoe/test/unitTests/zcf/test-zoeHelpersWZcf.js
@@ -159,7 +159,7 @@ test(`zoeHelper with zcf - assertIssuerKeywords`, async t => {
     },
     'no expected keywordRecord gets an error',
   );
-  t.falsy(assertIssuerKeywords(zcf, ['A', 'B']));
+  t.notThrows(() => assertIssuerKeywords(zcf, ['A', 'B']));
 });
 
 test(`zoeHelper with zcf - assertProposalShape`, async t => {
@@ -180,27 +180,29 @@ test(`zoeHelper with zcf - assertProposalShape`, async t => {
     { B: simoleanMint.mintPayment(simoleans(3)) },
   );
 
-  t.falsy(assertProposalShape(zcfSeat, []), 'empty expectation matches');
+  t.throws(() => assertProposalShape(zcfSeat, []), {
+    message: 'Expected must be an non-array object',
+  });
   t.throws(
-    () => assertProposalShape(zcfSeat, { want: { C: undefined } }),
+    () => assertProposalShape(zcfSeat, { want: { C: null } }),
     {
       message:
         'actual (an object) did not match expected (an object)\nSee console for error data.',
     },
-    'empty keywordRecord does not  match',
+    'empty keywordRecord does not match',
   );
-  t.falsy(assertProposalShape(zcfSeat, { want: { A: null } }));
-  t.falsy(assertProposalShape(zcfSeat, { give: { B: null } }));
+  t.notThrows(() => assertProposalShape(zcfSeat, { want: { A: null } }));
+  t.notThrows(() => assertProposalShape(zcfSeat, { give: { B: null } }));
   t.throws(
-    () => assertProposalShape(zcfSeat, { give: { c: undefined } }),
+    () => assertProposalShape(zcfSeat, { give: { c: null } }),
     {
       message:
         'actual (an object) did not match expected (an object)\nSee console for error data.',
     },
-    'wrong key in keywordRecord does not  match',
+    'wrong key in keywordRecord does not match',
   );
   t.throws(
-    () => assertProposalShape(zcfSeat, { exit: { onDemaind: undefined } }),
+    () => assertProposalShape(zcfSeat, { exit: { onDemaind: null } }),
     {
       message:
         'actual (an object) did not match expected (an object)\nSee console for error data.',
@@ -403,7 +405,7 @@ test(`zoeHelper w/zcf - swapExact w/extra payments`, async t => {
   assertPayoutAmount(t, moolaIssuer, await userSeatB.getPayout('D'), moola(40));
 });
 
-test.failing(`zcf/zoeHelper - assertProposalShape w/bad Expected`, async t => {
+test(`zcf/zoeHelper - assertProposalShape w/bad Expected`, async t => {
   const {
     moolaIssuer,
     moola,
@@ -421,9 +423,7 @@ test.failing(`zcf/zoeHelper - assertProposalShape w/bad Expected`, async t => {
     { B: simoleanMint.mintPayment(simoleans(3)) },
   );
 
-  // TODO: providing an amount in the expected record should throw
-  // https://github.com/Agoric/agoric-sdk/issues/1769
   t.throws(() => assertProposalShape(zcfSeat, { give: { B: moola(3) } }), {
-    message: 'expected record should have null values',
+    message: `The value of the expected record must be null but was (an object)\nSee console for error data.`,
   });
 });