design.md: edits from Frisby's review

[nfrisby]

• Fixed some typos
• Made a few editorial edits.
• Sought to fill a minor apparent gap, but by the end of the document I'm not sure. But I'll leave my efforts in for you to review.

[nfrisby]

Other notes:

• Determinism instead of Congruence? Broader jargon, I think.
• "We will create an additional localRoot peer whose job is to record all messages diffused from the NUT." --- "all messages" sounds a bit confused; all mini protocol messages are sent only peer-to-peer, not broadcast. But perhaps you mean "protocol messages", in which case having ones simulated downtream peer could provide additional information (all current simulated peers are upstream peers). This might even supersede the GetChainPoint LocalStateQuery message we discussed on Slack yesterday.
• "Once all of the peers have been connected to, the point schedule begins running." This one suggests that perhaps the test harness should control ticking, since presumably nothing else can observe with the node-under-test has finished connecting to all the simulated peers.
• The "apparent gap" that I mentioned in the commit/PR message is that it the CLI overview of the runner didn't make it apparent to me how the shrinking loop would know which "test seed" is the root of the shrinking. EG, in QuickCheck, a test case is conceptually determined by the tuple (original seed, shrink-path) and I only see shrink-path being passed around in the proposed CLI/stdout loop.
• For this reason, I was thinking that the test-file itself needed to be maintained alongside the shrink-index; but I think my edits along those lines are probably ultimately wrong. Left them as food for thought. For example, the later sentence "We will also implement the shrinkview binary at this time, which accepts only a test file and shrink index, and outputs the shrunk test on stdout." presumes there's always a test file available to shrink, but it wasn't obvious in the CLI/stdout description where that would be when the shrunk test failed. Actually...
  • Is the problem merely that the "The test runner CLI tool supports the following optional flags:" list should also list that the current test file is to be received on stdin? And when runner emits a new shrink index, the same stdin passed to runner should be passed to shrinkview?
  • Another option apparently missing from runner's inputs is how runner is supposed to invoke the node-under-test.
• A related idea: I think we should let the user stop shrinking whenever they want to. For that reason, I made a frail attempt at altering the spec to ensure there's always at least one interesting "test file" available for them to use for debugging, even if the shrinker does not consider that test file to be minimized. I think the right logic is something like: everytime we see a failure during a shrinking loop, emit a new test file to a shrunk-failures/ directory or something like that.
• "make sure the simulated peers behave as expected" This seems like a good concern to raise, but it's difficult to appreciate it in this context since the specification of the simulated peers is so vague in this document (which is fine, it's out of this documents scope). So perhaps remove this one-off item.
• For Milestone 3, I think at the very least it should begin with simply a consultation with the Amaru architects. Explain the design to them and see what concerns they would have about having to satisfy the proposed interface. Incorporating their feedback into the design might suffice as the entirety of Milestone 3; leaving it to them to actually implement whichever changes the framework would require of Amaru, since they had their chance to object 😈.
• "We will provide a tool that will automatically run the shrinking loop" -- merely based on my experience as a long-time QuickCheck user: it'd be really nice if the user could interrupt this tool whenever they run out of patience and still see some useful subset of the most-shrunk-so-far nodes in the accumulated shrinking tree.

[nfrisby]

The document paints a nice picture, thanks for preparing it so nicely! I especially like the nod towards supporting parallelization use cases, since one risk is that each iteration might sadly have a significant duration.