FABGW-6 Enchance scenario tests for discovery #31
Conversation
.gitignore
Outdated
| @@ -5,3 +5,6 @@ cover.out | |||
| fabric-protos/ | |||
| protos/gateway.pb.go | |||
| protos/gateway_grpc.pb.go | |||
| node/src/protos/protos.js | |||
There was a problem hiding this comment.
Everything in node/src/protos should already be covered by node/.gitignore so no need for it here
java/pom.xml
Outdated
| @@ -239,10 +239,11 @@ | |||
| <show>public</show> | |||
| <doctitle>Hyperledger Fabric Gateway SDK for Java</doctitle> | |||
| <nohelp>true</nohelp> | |||
| <failOnError>true</failOnError> | |||
| <failOnError>false</failOnError> | |||
There was a problem hiding this comment.
We can't leave it allowing Javadoc build failures
There was a problem hiding this comment.
oops, didn't mean to commit this!
pkg/server/api.go
Outdated
| endorsers := gs.registry.GetEndorsers(proposedTransaction.ChannelId) | ||
| channel, chaincodeID, err := getChannelAndChaincodeFromSignedProposal(proposedTransaction.Proposal) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "Failed to unpack channel header: ") |
There was a problem hiding this comment.
I'm not sure we should be continuing to use the github.com/pkg/errors package. It seems to be effectively deprecated and the wrapping of errors was introduced into the core Go libraries in v1.13: https://blog.golang.org/go1.13-errors
| } | ||
|
|
||
| func getChannelHeaderFromSignedProposal(signedProposal *peer.SignedProposal) (*common.ChannelHeader, error) { | ||
| func getChannelAndChaincodeFromSignedProposal(signedProposal *peer.SignedProposal) (string, string, error) { |
There was a problem hiding this comment.
The channel name is a top-level property of the ProposedTransaction so we don't need to deserialise and dig into the proposal to get it. I guess we should either:
- Also move the chaincode name to the top-level of the ProposedTransaction, which means we can save a whole bunch of expensive deserialisation on the proposals within the Gateway; or
- Just take the hit on deserialising the proposal and remove any ProposedTransaction properties that are duplicates of content in the proposal for simplicity.
But we could do that work outside of this PR, just as long as we actually do it.
There was a problem hiding this comment.
ok, this will all need revisiting anyway when we decide exactly what goes into the top-level message
|
|
||
| require ( | ||
| github.com/hyperledger/fabric-contract-api-go v1.1.1 | ||
| github.com/pkg/errors v0.9.1 |
There was a problem hiding this comment.
As per previous comments, I'm not sure it's a good idea to use this package when the core libraries give us similar functionality
| exec("docker", "start", peer); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| return; |
There was a problem hiding this comment.
If this fails the tests are broken so I think we'd be better throwing an Exception and failing fast instead of continuing on for the tests to behave unexpectedly later
| "--sequence", "1", | ||
| "--waitForEvent", | ||
| "--peerAddresses", "peer0.org1.example.com:7051", | ||
| "--peerAddresses", |
There was a problem hiding this comment.
Do we also need org3 here now we're a 3 org network?
There was a problem hiding this comment.
We could add it, but don't need to. It just needs a majority of endorsers
scenario/node/lib/scenario_steps.js
Outdated
| const transaction = await proposal.endorse(); | ||
| this.txn.result = await transaction.submit(); | ||
| } else { | ||
| //throw new Error(`Unknown transaction type: ${this.txn.type}`); |
There was a problem hiding this comment.
The tests are broken if we get here so we should throw
There was a problem hiding this comment.
Actually, most of this function is a duplicate of the implementation of When('I invoke the transaction'). Perhaps we could factor out some common code
There was a problem hiding this comment.
The tests are broken if we get here so we should throw
It will throw, but admittedly with a misleading message - I'll fix that.
|
There is some overlap with #32. Your call on which one looks easier to get in first and rebase the other one on. |
Enhance the cucumber test framework to allow discovery testing. Test network now comprises 5 peers across 3 orgs Peers can be stopped and started to test different signature policy scenarios Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com>
Enhance the cucumber test framework to allow discovery testing.
Test network now comprises 5 peers across 3 orgs
Peers can be stopped and started to test different signature policy scenarios
Signed-off-by: andrew-coleman andrew_coleman@uk.ibm.com