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

fix: Fix bug in test harness for unstable network #4980

Merged

Conversation

SamHSmith
Copy link
Contributor

@SamHSmith SamHSmith commented Aug 19, 2024

Solution

The bug was that the node selection for the client was random. This meant that the node the client was using to observe the tests were being frozen. I fixed it so that the client always is connected to the genesis peer which never gets frozen.

@SamHSmith
Copy link
Contributor Author

Currently the github runners are too slow for all the integration tests to pass.

nxsaken
nxsaken previously approved these changes Aug 19, 2024
@SamHSmith SamHSmith force-pushed the fix_unstable_network_test branch 4 times, most recently from 95b7ac5 to cf13629 Compare August 25, 2024 21:19
@Erigara
Copy link
Contributor

Erigara commented Aug 26, 2024

Is increased timeouts necessary for a fix?

@SamHSmith SamHSmith force-pushed the fix_unstable_network_test branch from cf13629 to b53d043 Compare August 26, 2024 19:20
@SamHSmith
Copy link
Contributor Author

SamHSmith commented Aug 26, 2024

Is increased timeouts necessary for a fix?

Yes. Not locally, but in the CI it is necessary.

@SamHSmith SamHSmith force-pushed the fix_unstable_network_test branch 3 times, most recently from 5c95419 to 7facc48 Compare August 27, 2024 08:18
@nxsaken
Copy link
Contributor

nxsaken commented Aug 27, 2024

Note that #4996 switches to a faster runner, and adds retries for extra_functional tests

@SamHSmith SamHSmith force-pushed the fix_unstable_network_test branch 5 times, most recently from 3c9d72e to 82cd5f6 Compare August 27, 2024 16:13
@SamHSmith SamHSmith force-pushed the fix_unstable_network_test branch 4 times, most recently from 18330e7 to b5f136a Compare August 28, 2024 07:50
@SamHSmith
Copy link
Contributor Author

Note that #4996 switches to a faster runner, and adds retries for extra_functional tests

The faster runner would be a good addition. But I think retries is a very bad idea.

@SamHSmith SamHSmith enabled auto-merge (squash) August 28, 2024 09:14
client/src/config.rs Outdated Show resolved Hide resolved
@SamHSmith SamHSmith force-pushed the fix_unstable_network_test branch from b5f136a to ff9827b Compare August 28, 2024 11:54
client/src/config.rs Outdated Show resolved Hide resolved
@SamHSmith SamHSmith force-pushed the fix_unstable_network_test branch from ff9827b to 29fa97f Compare August 28, 2024 14:40
@s8sato s8sato self-assigned this Aug 28, 2024
@SamHSmith SamHSmith merged commit be22388 into hyperledger-iroha:main Aug 29, 2024
14 checks passed
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.

5 participants