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

Electrum integration tests hang for 10 minutes in case wrong host is provided #3586

Closed
1 task done
lukasz-zimnoch opened this issue May 23, 2023 · 2 comments
Closed
1 task done
Assignees
Milestone

Comments

@lukasz-zimnoch
Copy link
Member

lukasz-zimnoch commented May 23, 2023

Observed in:

Steps to recreate:

  1. Go to pkg/bitcoin/electrum/electrum_integration_test.go
  2. Replace "blackie.c3-soft.com:57006" at line 72 with something that can't be resolved, e.g. "blackie.c3-softtt.com:57006"
  3. Run go test github.com/keep-network/keep-core/pkg/bitcoin/electrum -v -tags=integration ./...

In some cases, one of the integration tests hangs for 10 minutes and fail with:

panic: test timed out after 10m0s

goroutine 429 [running]:
testing.(*M).startAlarm.func1()
        /opt/homebrew/Cellar/[email protected]/1.18.5/libexec/src/testing/testing.go:2029 +0x8c
created by time.goFunc
        /opt/homebrew/Cellar/[email protected]/1.18.5/libexec/src/time/sleep.go:176 +0x3c

goroutine 1 [chan receive, 2 minutes]:
testing.(*T).Run(0x14000555520, {0x102e1e49b?, 0x1c114841bbb5b?}, 0x103160588)

while in this case, all tests should always fail with:

failed to initialize electrum client: [retry timeout [1m0s] exceeded; most recent error: [dial tcp: lookup blackie.c3-softfff.com: no such host]]

The above problem may indicate some problems with the Electrum implementation connection retry logic.

Tasks

Preview Give feedback
  1. 📟 client
    nkuba
@nkuba
Copy link
Member

nkuba commented Jun 28, 2023

Go tests are executed with the default timeout of 10m. If the suite doesn't finish execution within this time the tests are aborted with panic.

In case of electrum connection problems for any of the servers, the executor has 1m for retries before it fails the particular test. If one of the servers doesn't work, given the number of tests the suite defines the execution won't be completed within 10 minutes timeout (it requires ~1061s). So we see the panics.

I'll increase the timeout for integration tests to not panic if any of the servers is non-responsive.
I'll add parallel test execution to reduce the time required for the whole suite to execute.

@nkuba
Copy link
Member

nkuba commented Jun 28, 2023

Actually, we already increased the timeout for the integration test to 20m in #3599.

To run the tests locally add -timeout 20m flag to the test execution command or execute the tests with ./script/run-integration-test.sh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants