Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FAB-16120 Adding IT for MSP inconsistence #305

Merged
merged 1 commit into from
Jan 6, 2020

Conversation

DereckLuo
Copy link
Contributor

@DereckLuo DereckLuo commented Nov 22, 2019

Signed-off-by: Chongxin Luo Chongxin.Luo@ibm.com

Type of change

  • Test update

Description

  • Adding integration test to replace behave test in fabric-test.
  • Testing correct endorsement behavior when a peer with member identity trying to endorse a chaincode with OR (Org1.peer, Org2.peer) endorsement policy.
  • Minor typo change in integration test.

Additional details

Related issues

https://jira.hyperledger.org/browse/FAB-16120

integration/msp/msp_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jyellick jyellick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As best as I can tell, this test is verifying that if you set an endorsement policy to require a peer role, for an org which does not have node OUs enabled, that that org won't be able to validly endorse. But it's not at all clear from the description of the test that this is the scenario being tested.

It actually seems like it's testing two scenarios, one, where the policy is not satisfied because the peer's signer cert does not have the peer node OU, and one where the policy is not satisfied because the peer's MSP does not define the peer node OU. It would be nice to explicitly call this out in the test, and to name which orgs/peers are testing which scenarios.

integration/msp/msp_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jyellick jyellick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bulk of it looks good, but a few issues with the template

integration/nwo/configtx_template.go Outdated Show resolved Hide resolved
integration/nwo/configtx_template.go Outdated Show resolved Hide resolved
integration/nwo/configtx_template.go Show resolved Hide resolved
Signed-off-by: Chongxin Luo <Chongxin.Luo@ibm.com>
@DereckLuo DereckLuo closed this Dec 20, 2019
@DereckLuo DereckLuo deleted the FAB-16120 branch December 20, 2019 14:57
@DereckLuo DereckLuo restored the FAB-16120 branch January 2, 2020 19:27
@DereckLuo DereckLuo reopened this Jan 2, 2020
@jyellick jyellick dismissed mastersingh24’s stale review January 6, 2020 16:13

The referenced typo has been fixed.

@jyellick jyellick merged commit d35acaa into hyperledger:master Jan 6, 2020
@DereckLuo DereckLuo deleted the FAB-16120 branch January 16, 2020 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants