From 116e84dc59686cb11b260c4c5ce4408a451c2f99 Mon Sep 17 00:00:00 2001
From: "Mark S. Miller" <erights@gmail.com>
Date: Fri, 7 Oct 2022 22:01:22 -0700
Subject: [PATCH] fix: review suggestions

---
 packages/store/test/test-rankOrder.js |  3 +-
 packages/store/tools/arb-passable.js  | 53 ++++++++++++++++++---------
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/packages/store/test/test-rankOrder.js b/packages/store/test/test-rankOrder.js
index 90b89974ee1..e1d194828bb 100644
--- a/packages/store/test/test-rankOrder.js
+++ b/packages/store/test/test-rankOrder.js
@@ -48,7 +48,8 @@ test('compareRank totally orders ranks', async t => {
 test('compareRank is transitive', async t => {
   await fc.assert(
     fc.property(
-      // operate on a set of three passables covering at least two ranks
+      // operate on a set of three distinct passables covering
+      // at least two ranks
       fc
         .uniqueArray(arbPassable, { minLength: 3, maxLength: 3 })
         .filter(
diff --git a/packages/store/tools/arb-passable.js b/packages/store/tools/arb-passable.js
index ad3382ed9fd..49b8e554358 100644
--- a/packages/store/tools/arb-passable.js
+++ b/packages/store/tools/arb-passable.js
@@ -14,7 +14,7 @@ export const exampleCarol = Far('carol', {});
 export const arbString = fc.oneof(fc.string(), fc.fullUnicodeString());
 
 export const arbLeaf = fc.oneof(
-  fc.record({}),
+  fc.constantFrom(null, undefined, false, true),
   arbString,
   arbString.map(s => Symbol.for(s)),
   // primordial symbols and registered lookalikes
@@ -28,7 +28,7 @@ export const arbLeaf = fc.oneof(
   fc.bigInt(),
   fc.integer(),
   fc.constantFrom(-0, NaN, Infinity, -Infinity),
-  fc.constantFrom(null, undefined, false, true),
+  fc.record({}),
   fc.constantFrom(exampleAlice, exampleBob, exampleCarol),
   arbString.map(s => new Error(s)),
   // unresolved promise
@@ -38,45 +38,62 @@ export const arbLeaf = fc.oneof(
 const { arbDag } = fc.letrec(tie => {
   return {
     arbDag: fc.oneof(
-      {
-        // depthFactor: 0.5, // TODO Replace with what from new API?
-        withCrossShrink: true,
-      },
+      { withCrossShrink: true },
       arbLeaf,
       tie('arbDag').map(v => Promise.resolve(v)),
-      fc.array(tie('arbDag'), { maxLength: 3 }),
+      fc.array(tie('arbDag')),
       fc.dictionary(
         arbString.filter(s => s !== 'then'),
         tie('arbDag'),
-        { maxKeys: 3 },
       ),
-      // a tagged value whose payload is an array of [key, leaf] pairs
-      // where each key is unique within the payload
-      // XXX can the payload be generalized further?
+      // A tagged value, either of arbitrary type with arbitrary payload
+      // or of known type with arbitrary or explicitly valid payload.
+      // Ordered by increasing complexity.
       fc
         .oneof(
+          fc.record({ type: arbString, payload: tie('arbDag') }),
           fc.record({
             type: fc.constantFrom('copySet'),
-            payload: fc.oneof(fc.array(tie('arbDag')), tie('arbDag')),
+            payload: fc.oneof(
+              tie('arbDag'),
+              // copySet valid payload is an array of unique passables.
+              // TODO: it must be a reverse sorted array, so we should
+              // generate some of those
+              fc.uniqueArray(tie('arbDag')),
+            ),
           }),
           fc.record({
             type: fc.constantFrom('copyBag'),
             payload: fc.oneof(
-              fc.array(fc.tuple(tie('arbDag'), fc.bigInt())),
               tie('arbDag'),
+              // copyBag valid payload is an array of [passable, count] tuples
+              // in which each passable is unique.
+              // TODO: it must be a reverse sorted array, so we should
+              // generate some of those
+              fc.uniqueArray(fc.tuple(tie('arbDag'), fc.bigInt()), {
+                selector: entry => entry[0],
+              }),
             ),
           }),
           fc.record({
             type: fc.constantFrom('copyMap'),
             payload: fc.oneof(
-              fc.record({
-                keys: fc.array(tie('arbDag')),
-                values: fc.array(tie('arbDag')),
-              }),
               tie('arbDag'),
+              // copyMap valid payload is a {keys: Array<Passable>, values: Array<Passable>} record
+              // in which keys are unique and both arrays have the same length.
+              // TODO: keys must be a reverse sorted array, so we should
+              // generate some of those
+              fc
+                .uniqueArray(
+                  fc.record({ key: tie('arbDag'), value: tie('arbDag') }),
+                  { selector: entry => entry.key },
+                )
+                .map(entries => ({
+                  keys: entries.map(({ key }) => key),
+                  values: entries.map(({ value }) => value),
+                })),
             ),
           }),
-          fc.record({ type: arbString, payload: tie('arbDag') }),
         )
         .map(({ type, payload }) => makeTagged(type, payload)),
     ),