Adds the e2e test suite framework and a bit over 80 test scenarios #1514
Conversation
adds the step "the user waits for n blocks", makes blocks shorter in the configuration (45 seconds now), makes server verbosity optional
spacing consistency Co-authored-by: Daniel Olshansky <[email protected]>
|
@iajrz I'll continue reviewing once the existing comments are resolved or Once the existing comments are resolved or tended |
I am pretty uncomfortable about the semantics of this code; I end up declaring a ton of `err` variables for no reason other than they are the second parameter, and the first one needs to be created. The tendency towards being terse is fighting the common code structuring patterns and the semantics in a royal rumble where the winner gets to still not be satisfactory, so I'll just keep this in a recognizable style and call it quits until a guiding principle can show up and help us never face such mixed solutions of sometimes creating and sometimes reusing a variable.
opted for it over bytes.Buffer since it seems to be the intended upgrade path anyhow (see https://groups.google.com/g/golang-codereviews/c/6S3_B4N197E?pli=1); they seem similar in the intended use, but it never hurts to use the intended tool. It also has some checks around escape analysis that were missing from the custom implementation, so we're winning.
|
@iajrz I saw that you pushed some updates. Please re-request a review (see screenshot) when it's ready for another round
|
| And the user should be able to see standard output containing "0011" | ||
| And the pocket client should have exited without error | ||
|
|
||
| Scenario: To get existing supported networks, Returns an error code with invalid height |
There was a problem hiding this comment.
Confusing scenario. What is the acceptance criterion here?
There was a problem hiding this comment.
@AneudyM I completely with you that this scenario, and it's title are confusing. When reviewing code, I usually try to use Github's "suggestion" feature so the reviewee can just accept/commit it or modify it if need be.; would appreciate your teams help in distributing the workload on this.
For example (I have no clue if this is clearer):
| Scenario: To get existing supported networks, Returns an error code with invalid height | |
| Scenario: To verify that an invalid height cannot be quried from the list of supported-networks |
Re the scenario -
- Are we verifying we can't query an invalid height OR we can't query an unsupported network?
Re the Then - error loading store makes me think the network is not supported.
2. Is that correct?
3. If so, is there any way to make the source code (without comments) make that more explicit?
Re the And
4. Does this exit successfully because the client worked but the backing data is invalid?
There was a problem hiding this comment.
Yeah. Those are good points. Mmm..., looking at the validation of the scenario it might be trying to query a network with an invalid height. In that case, I think I would rewrite it as follows:
Scenario: Return error when querying supported networks with invalid height number
Given a “single_node_network”
And a valid pocket client
When querying a supported network with invalid height “99999” number
Then an “error loading store” message should be returned
But the pocket client should not exit with an error status
Re point 4:
That's why I am unsure of including the exit status steps here. What does the specification say about these exit statuses.
Co-authored-by: Daniel Olshansky <[email protected]>
|
@iajrz I updated the documentation, added a few comments and refactored a bit of the code. I'm in favour of adopting a similar pattern (with a simpler initial implementation) in V1 using gherkin. Lmk if you see issue with any of my changes. If not, I can go ahead and merge it in. cc @jessicadaugherty for visibility |
This PR has both Go code and a cucumber test suite in gherkin; I'd expect @Olshansk to look at the go side of the matter and @emedrano2112 and @AneudyM to look at the gherkin side of the matter.
Fixes #1482
Co-authored-by: Daniel Olshansky [email protected]