core

core copied to clipboard

We now have tests that exercise most of the behavior that MetaMask provides around various RPC methods via middleware — these are the "network client tests" — but some of this behavior lacks tests. Before we can backfill the remaining tests, however, we need to address some issues with the existing ones.

The tests are currently organized first by whether they are supported by Infura and/or whether they are contained in the Ethereum spec, and then by whether we assume that an RPC method takes a block parameter or not. The first level isn't necessary from a test perspective; it's just a factoid, but it doesn't affect behavior. The second level is important from a behavior perspective, but the behavior in question comes from the block cache middleware, and although that's the middleware that is most impactful, it is not the only way to categorize RPC methods given that there are other middleware that affect other RPC methods.

All this to say a better organizational scheme would be to use a flat structure. I propose the following guidelines:

• There should be one describe per RPC method, with the behavior for that RPC method tested within that describe.
• All tests should be named properly to reflect the behavior that they are testing. This means if we have a test where expectations differ slightly based on the RPC method being used, the type of network client being tested, or some other condition, that test should be split into multiple instances.
• It should be easy to find the tests for a particular network client / RPC method combo by starting at the top of a file and scrolling down. With so many tests, it is confusing to have to jump between multiple files to be able to read a whole test or even a whole suite of tests. This means that if statements are allowed to specify differences in behavior but only if it does not impact readability.