Fix shared mocks in RPC network client test (#16856)

* Fix shared mocks in RPC network client test

Some of the provider api unit tests were inadvertently sharing test
mocks. A `params` object used for mock RPC calls was being shared
between tests and between calls within individual tests. They have been
updated to generate a fresh `params` object for each mock RPC call.

* Explicitly set blockParam to undefined

app/scripts/controllers/network/provider-api-tests/helpers.js

@@ -331,40 +331,27 @@ export async function withInfuraClient(...args) {
 }
 
 /**
- * Some JSON-RPC endpoints take a "block" param (example: `eth_blockNumber`)
- * which can optionally be left out. Additionally, the endpoint may support some
- * number of arguments, although the "block" param will always be last, even if
- * it is optional. Given this, this function builds a mock `params` array for
- * such an endpoint, filling it with arbitrary values, but with the "block"
- * param missing.
+ * Build mock parameters for a JSON-RPC call.
  *
- * @param {number} index - The index within the `params` array where the "block"
- * param *would* appear.
- * @returns {string[]} The mock params.
- */
-export function buildMockParamsWithoutBlockParamAt(index) {
-  const params = [];
-  for (let i = 0; i < index; i++) {
-    params.push('some value');
-  }
-  return params;
-}
-
-/**
- * Some JSON-RPC endpoints take a "block" param (example: `eth_blockNumber`)
- * which can optionally be left out. Additionally, the endpoint may support some
- * number of arguments, although the "block" param will always be last, even if
- * it is optional. Given this, this function builds a `params` array for such an
- * endpoint with the given "block" param added at the end.
+ * The string 'some value' is used as the default value for each entry. The
+ * block parameter index determines the number of parameters to generate.
  *
- * @param {number} index - The index within the `params` array to add the
- * "block" param.
- * @param {any} blockParam - The desired "block" param to add.
+ * The block parameter can be set to a custom value. If no value is given, it
+ * is set as undefined.
+ *
+ * @param {object} args - Arguments.
+ * @param {number} args.blockParamIndex - The index of the block parameter.
+ * @param {any} [args.blockParam] - The block parameter value to set.
  * @returns {any[]} The mock params.
  */
-export function buildMockParamsWithBlockParamAt(index, blockParam) {
-  const params = buildMockParamsWithoutBlockParamAt(index);
-  params.push(blockParam);
+export function buildMockParams({ blockParam, blockParamIndex }) {
+  if (blockParamIndex === undefined) {
+    throw new Error(`Missing 'blockParamIndex'`);
+  }
+
+  const params = new Array(blockParamIndex).fill('some value');
+  params[blockParamIndex] = blockParam;
+
   return params;
 }
 

app/scripts/controllers/network/provider-api-tests/shared-tests.js

@@ -4,8 +4,7 @@ import { fill } from 'lodash';
 import {
   withMockedInfuraCommunications,
   withInfuraClient,
-  buildMockParamsWithoutBlockParamAt,
-  buildMockParamsWithBlockParamAt,
+  buildMockParams,
   buildRequestWithReplacedBlockParam,
 } from './helpers';
 
@@ -639,18 +638,16 @@ export function testsForRpcMethodSupportingBlockParam(
   { blockParamIndex },
 ) {
   describe.each([
-    ['given no block tag', 'none'],
+    ['given no block tag', 'none', undefined],
     ['given a block tag of "latest"', 'latest', 'latest'],
   ])('%s', (_desc, blockParamType, blockParam) => {
-    const params =
-      blockParamType === 'none'
-        ? buildMockParamsWithoutBlockParamAt(blockParamIndex)
-        : buildMockParamsWithBlockParamAt(blockParamIndex, blockParam);
-
     it('does not hit Infura more than once for identical requests', async () => {
       const requests = [
-        { method, params },
-        { method, params },
+        {
+          method,
+          params: buildMockParams({ blockParamIndex, blockParam }),
+        },
+        { method, params: buildMockParams({ blockParamIndex, blockParam }) },
       ];
       const mockResults = ['first result', 'second result'];
 
@@ -684,8 +681,8 @@ export function testsForRpcMethodSupportingBlockParam(
 
     it('hits Infura and does not reuse the result of a previous request if the latest block number was updated since', async () => {
       const requests = [
-        { method, params },
-        { method, params },
+        { method, params: buildMockParams({ blockParamIndex, blockParam }) },
+        { method, params: buildMockParams({ blockParamIndex, blockParam }) },
       ];
       const mockResults = ['first result', 'second result'];
 
@@ -734,8 +731,8 @@ export function testsForRpcMethodSupportingBlockParam(
       'does not reuse the result of a previous request if it was `%s`',
       async (emptyValue) => {
         const requests = [
-          { method, params },
-          { method, params },
+          { method, params: buildMockParams({ blockParamIndex, blockParam }) },
+          { method, params: buildMockParams({ blockParamIndex, blockParam }) },
         ];
         const mockResults = [emptyValue, 'some result'];
 
@@ -1143,12 +1140,16 @@ export function testsForRpcMethodSupportingBlockParam(
     ['given a block tag of "earliest"', 'earliest', 'earliest'],
     ['given a block number', 'block number', '0x100'],
   ])('%s', (_desc, blockParamType, blockParam) => {
-    const params = buildMockParamsWithBlockParamAt(blockParamIndex, blockParam);
-
     it('does not hit Infura more than once for identical requests', async () => {
       const requests = [
-        { method, params },
-        { method, params },
+        {
+          method,
+          params: buildMockParams({ blockParamIndex, blockParam }),
+        },
+        {
+          method,
+          params: buildMockParams({ blockParamIndex, blockParam }),
+        },
       ];
       const mockResults = ['first result', 'second result'];
 
@@ -1173,8 +1174,14 @@ export function testsForRpcMethodSupportingBlockParam(
 
     it('reuses the result of a previous request even if the latest block number was updated since', async () => {
       const requests = [
-        { method, params },
-        { method, params },
+        {
+          method,
+          params: buildMockParams({ blockParamIndex, blockParam }),
+        },
+        {
+          method,
+          params: buildMockParams({ blockParamIndex, blockParam }),
+        },
       ];
       const mockResults = ['first result', 'second result'];
 
@@ -1212,8 +1219,14 @@ export function testsForRpcMethodSupportingBlockParam(
       'does not reuse the result of a previous request if it was `%s`',
       async (emptyValue) => {
         const requests = [
-          { method, params },
-          { method, params },
+          {
+            method,
+            params: buildMockParams({ blockParamIndex, blockParam }),
+          },
+          {
+            method,
+            params: buildMockParams({ blockParamIndex, blockParam }),
+          },
         ];
         const mockResults = [emptyValue, 'some result'];
 
@@ -1245,14 +1258,11 @@ export function testsForRpcMethodSupportingBlockParam(
         const requests = [
           {
             method,
-            params: buildMockParamsWithBlockParamAt(
-              blockParamIndex,
-              blockParam,
-            ),
+            params: buildMockParams({ blockParamIndex, blockParam }),
           },
           {
             method,
-            params: buildMockParamsWithBlockParamAt(blockParamIndex, '0x00'),
+            params: buildMockParams({ blockParamIndex, blockParam: '0x00' }),
           },
         ];
         const mockResults = ['first result', 'second result'];
@@ -1282,11 +1292,11 @@ export function testsForRpcMethodSupportingBlockParam(
           const requests = [
             {
               method,
-              params: buildMockParamsWithBlockParamAt(blockParamIndex, '0x100'),
+              params: buildMockParams({ blockParamIndex, blockParam: '0x100' }),
             },
             {
               method,
-              params: buildMockParamsWithBlockParamAt(blockParamIndex, '0x200'),
+              params: buildMockParams({ blockParamIndex, blockParam: '0x200' }),
             },
           ];
 
@@ -1315,7 +1325,7 @@ export function testsForRpcMethodSupportingBlockParam(
         await withMockedInfuraCommunications(async (comms) => {
           const request = {
             method,
-            params: buildMockParamsWithBlockParamAt(blockParamIndex, '0x100'),
+            params: buildMockParams({ blockParamIndex, blockParam: '0x100' }),
           };
 
           // The first time a block-cacheable request is made, the latest
@@ -1340,7 +1350,7 @@ export function testsForRpcMethodSupportingBlockParam(
         await withMockedInfuraCommunications(async (comms) => {
           const request = {
             method,
-            params: buildMockParamsWithBlockParamAt(blockParamIndex, '0x50'),
+            params: buildMockParams({ blockParamIndex, blockParam: '0x50' }),
           };
 
           // The first time a block-cacheable request is made, the latest
@@ -1364,7 +1374,7 @@ export function testsForRpcMethodSupportingBlockParam(
   });
 
   describe('given a block tag of "pending"', () => {
-    const params = buildMockParamsWithBlockParamAt(blockParamIndex, 'pending');
+    const params = buildMockParams({ blockParamIndex, blockParam: 'pending' });
 
     it('hits Infura on all calls and does not cache anything', async () => {
       const requests = [