Add runTx API tests #352
Add runTx API tests #352
Conversation
|
Thanks, this already looks quite good. Would assume the same behavior as you for the no-cache case, think we have to look closer into that one. |
|
I had a quick look, and it seems I'd probably use It seems this might be the direction that was intended, as |
|
Can you comment out the current failing test with some commentary notes that this should pass (in a similar way) and then open a new issue where you describe what you wrote in the comment above? Then we have this cleanly separated and can move on merging this PR. Also, @mattdean-digicatapult should maybe have a comment on this, since he is the current state manager expert and is currently in the process of reworking this. |
s1na
force-pushed
the
test/api-runTx
branch
from
4995b73 to
407874e
Compare
|
@s1na, there is work in progress on this. I've got a do-not-merge PR (#309) that is gradually being broken up and merged which will clean-up all these direct cache writes. There's still an open question as to how we handle the |
|
Hey @mattdean-digicatapult :) great. I don't know the arguments for keeping |
| cb = opts | ||
| return cb(new Error('invalid input, opts must be provided')) | ||
| } | ||
|
|
| if (!tx) { | ||
| return cb(new Error('invalid input, tx is required')) | ||
| } | ||
|
|
| if (opts.populateCache === false) { | ||
| return cb() | ||
| } | ||
|
|
| ) | ||
|
|
||
| t.end() | ||
| }) |
There was a problem hiding this comment.
Nice cases. In addition to all the failure cases, can you add one which actually passes?
There was a problem hiding this comment.
Oh sorry, I was thinking the success case is thoroughly covered by the blockchain tests. You're right, there should be a success case.
Just added a simple one.
There was a problem hiding this comment.
Yeah, I think it is better to have this explicitly defined rather than have this implicitly tested somewhere else. Thanks for adding!
Signed-off-by: Sina Mahmoodi <[email protected]>
s1na
force-pushed
the
test/api-runTx
branch
from
407874e to
b5c4f97
Compare
This PR is part of the series for adding API test coverage. It:
runTx.jslib/runTx.jsto add two validation checks. Furthermore it moves the check forpopulateCacheflag to the beginning of function.populateCachefails. I'm not sure if the test case makes sense. I assumedrunTxshould run even whenpopulateCacheis false, the difference being that no cache is utilized. However,runTxfails because it tries to use cache nevertheless.createGenesisutility toutils.jsfile.