-
Notifications
You must be signed in to change notification settings - Fork 283
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
test(connector-fabric): fix module requires Go 1.17 #914 #915
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
petermetz
added
bug
Something isn't working
dependencies
Pull requests that update a dependency file
labels
May 4, 2021
This was referenced May 4, 2021
kikoncuo
approved these changes
May 6, 2021
takeutak
approved these changes
May 10, 2021
takeutak
approved these changes
May 10, 2021
petermetz
force-pushed
the
fix-914
branch
4 times, most recently
from
May 27, 2021 23:37
c5798be
to
8f6b98a
Compare
Figured out from the additional logs we recently added that the reason why the Fabric 1.4.x AIO image fails to start for the 1.4.x tests is because a previously ran test (2.x) crashes and never stops it's container which then ends up hogging the ports which cannot be randomized for the Fabric AIO images yet. So, adding more logging here and also ensuring that the test finish hooks that are responsible for shutting down the containers are registered ***prior*** to calling the start method on the test container classes so that if the start method hangs the container will still go away as it should. This is what gave me the clue from the CI logs: # BEFORE runs tx on a Fabric v2.2.0 ledger Detected current process to be running inside a Github Action. Pruning all docker resources... [2021-05-28T01:23:31.310Z] DEBUG (Containers#pruneDockerResources()): Finished pruning all docker resources. Outcome: { containers: { ContainersDeleted: null, SpaceReclaimed: 0 }, images: { ImagesDeleted: null, SpaceReclaimed: 0 }, networks: { NetworksDeleted: null }, volumes: { VolumesDeleted: [ '10afa6c6071a4e362324ec7eaabedbbee088dedc0b0e182880ca4ccc6430b998', [length]: 1 ], SpaceReclaimed: 1915254089 } } ok 1 Pruning didn't throw OK # runs tx on a Fabric v2.2.0 ledger # test count(1) != plan(null) # failed 1 test not ok 62 - packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/run-transaction-endpoint-v1.test.ts # time=3600276.779ms Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
This was initially a candidate for resolving the issue of hyperledger-cacti#914 but later that idea turned out to not work. At the end of the day we always thrive to work with the latest and greatest of all of our dependencies CVEs permitting so the upgrade can stay in as a valid change. Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
…#914 Primary change: ------------------ Pinned the buggy dependency to yesterday's version (the bug was introduced in this morning's build). This prevents today's version from being used and fixes the problem. Secondary change: -------------------- Upgraded the container image that's being used for the test to the one that has the fabric images pre-cached. This leads to faster test execution and lower probability of developers getting hit by the dreaded DockerHub rate limiting issue. Fixes hyperledger-cacti#914 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
🎉 Great news! Looks like all the dependencies have been resolved: 💡 To add or remove a dependency please update this issue/PR description. Brought to you by Dependent Issues (:robot: ). Happy coding! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #993
Author: Peter Somogyvari peter.somogyvari@accenture.com
Committer: Peter Somogyvari peter.somogyvari@accenture.com
Date: Tue May 04 2021 14:32:33 GMT-0700 (Pacific Daylight Time)
test(connector-fabric): fix module requires Go 1.17 #914
Primary change:
Pinned the buggy dependency to yesterday's version
(the bug was introduced in this morning's build).
This prevents today's version from being used and
fixes the problem.
Secondary change:
Upgraded the container image that's being used for the test to
the one that has the fabric images pre-cached.
This leads to faster test execution and lower probability
of developers getting hit by the dreaded DockerHub rate limiting issue.
Fixes #914
Signed-off-by: Peter Somogyvari peter.somogyvari@accenture.com