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.

Sign up for GitHub

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

Jump to bottom

upgrade to Ginkgo v2 #3498

Merged
merged 1 commit into from
Jun 29, 2022
Merged

upgrade to Ginkgo v2 #3498

merged 1 commit into from
Jun 29, 2022

Conversation

SamYuan1990
Copy link
Contributor

Type of change

  • Improvement (improvement to code, performance, etc)
  • Test update

Description

upgrade ginkgo to v2

Additional details

Related issues

@SamYuan1990 SamYuan1990 requested a review from a team as a code owner June 21, 2022 11:43
@SamYuan1990 SamYuan1990 marked this pull request as draft June 21, 2022 11:43
@SamYuan1990 SamYuan1990 marked this pull request as ready for review June 21, 2022 13:47
@SamYuan1990
Copy link
Contributor Author

as reopen for #3202

@SamYuan1990 SamYuan1990 mentioned this pull request Jun 21, 2022
Merged
@SamYuan1990
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 3498 in repo hyperledger/fabric

@SamYuan1990
Copy link
Contributor Author 

2022-06-21T14:14:05.4200851Z coverage: 95.3% of statements
2022-06-21T14:14:05.4201233Z FAIL	github.com/hyperledger/fabric/orderer/common/cluster	67.052s

fails from CI but passed locally, can any one help?
image

@SamYuan1990
Copy link
Contributor Author 

--- FAIL: TestWithStaticDeliverClientLeader (90.03s)
    gossip_service_test.go:630: Failed to establish full channel membership. Only 0 out of 2 peers have full membership
FAIL
coverage: 45.0% of statements
FAIL	github.com/hyperledger/fabric/gossip/service	104.114s
?   	github.com/hyperledger/fabric/gossip/service/mocks	[no test files]
ok  	github.com/hyperledger/fabric/gossip/state	88.116s	coverage: 79.7% of statements
ok  	github.com/hyperledger/fabric/gossip/state/mocks	0.034s	coverage: 55.6% of statements
ok  	github.com/hyperledger/fabric/gossip/util	4.054s	coverage: 73.6% of statements
FAIL

from ci https://dev.azure.com/Hyperledger/Fabric/_build/results?buildId=53950&view=logs&j=e306c17a-d139-54bf-a475-f5a11259cee7&t=1e3023a5-584f-52f3-49bc-66bd27d27b6d&l=138
but seems this test case passed at local?
image

@SamYuan1990
Copy link
Contributor Author

hope anyone can help re run the CI to see if it works.
btw I use https://marketplace.visualstudio.com/items?itemName=joselitofilho.ginkgotestexplorer this plugin at local to trigger unit test.

denyeart
denyeart reviewed Jun 24, 2022
View reviewed changes
go.mod Outdated
@@ -87,7 +86,7 @@ require (
github.com/moby/sys/mountinfo v0.4.0 // indirect
github.com/moby/term v0.0.0-20201216013528-df9cb8a40635 // indirect
github.com/morikuni/aec v1.0.0 // indirect
github.com/nxadm/tail v1.4.8 // indirect
github.com/onsi/ginkgo/v2 v2.1.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Since ginkgo is a direct dependency, shouldn't it be in the upper 'require' section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

@SamYuan1990
by this change make ginkgo following go mod for package management.
3e98612 
upgrade related go files to ginkgo v2
refer: https://onsi.github.io/ginkgo/MIGRATING_TO_V2

Signed-off-by: Sam Yuan <yy19902439@126.com>
denyeart
denyeart approved these changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@denyeart denyeart left a comment

Choose a reason for hiding this comment

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

Thanks @SamYuan1990 , this looks good to me. I'm re-running unit test to see if we can get it to pass.

@denyeart denyeart merged commit d2f0adb into hyperledger:main Jun 29, 2022
@SamYuan1990 SamYuan1990 mentioned this pull request Oct 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denyeart denyeart denyeart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SamYuan1990 @denyeart