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

itest: fix async payment benchmark timeout flakes #5579

Merged
merged 2 commits into from
Jul 27, 2021
Merged

itest: fix async payment benchmark timeout flakes #5579

merged 2 commits into from
Jul 27, 2021

Conversation

bhandras
Copy link
Collaborator

Fixes timeout flakes that rarely happen when dbbackend=etcd:

        *errors.errorString alice's remote balance is incorrect, got 771000, expected 1000000
        /home/travis/gopath/src/github.com/lightningnetwork/lnd/lntest/itest/lnd_payment_test.go:550 (0x13f6ea9)
        	testBidirectionalAsyncPayments: t.Fatalf("alice's remote balance is incorrect, got %v, "+
        /home/travis/gopath/src/github.com/lightningnetwork/lnd/lntest/itest/test_harness.go:112 (0x13a8ace)
        	(*harnessTest).RunTestCase: testCase.test(h.lndHarness, h)
        /home/travis/gopath/src/github.com/lightningnetwork/lnd/lntest/itest/lnd_test.go:247 (0x144c00f)
        	TestLightningNetworkDaemon.func4: ht.RunTestCase(testCase)
        /home/travis/.gimme/versions/go1.16.3.linux.amd64/src/testing/testing.go:1193 (0x52232f)
        	tRunner: fn(t)
        /home/travis/.gimme/versions/go1.16.3.linux.amd64/src/runtime/asm_amd64.s:1371 (0x472001)
        	goexit: BYTE	$0x90	// NOP```

@bhandras bhandras requested review from guggero and carlaKC July 27, 2021 10:43
@bhandras bhandras added itests Issues related to integration tests. flake fix labels Jul 27, 2021
@bhandras bhandras force-pushed the payment_bench_timeout_flake branch from efb86fb to 897a304 Compare July 27, 2021 10:47
@bhandras bhandras mentioned this pull request Jul 27, 2021
Merged
guggero
guggero approved these changes Jul 27, 2021
View reviewed changes
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice fix, LGTM 💯

lntest/itest/lnd_payment_test.go Outdated
if err != nil {
t.Fatalf("failed to assert alice's pending htlcs and/or remote/local balance")
}

// Wait for Bob to receive revocation from Alice.
time.Sleep(2 * time.Second)
var bobChan *lnrpc.Channel
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great to get rid of a static sleep!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like Travis gotten slower these days as this flake I've never seen before.

lntest/itest/lnd_payment_test.go Outdated Show resolved Hide resolved
@bhandras bhandras force-pushed the payment_bench_timeout_flake branch from 897a304 to d94e44e Compare July 27, 2021 11:48
@bhandras bhandras force-pushed the payment_bench_timeout_flake branch from d94e44e to 7da0b80 Compare July 27, 2021 11:57
carlaKC
carlaKC approved these changes Jul 27, 2021
View reviewed changes
Copy link
Collaborator

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

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

another flake bites the dust!

@guggero guggero merged commit 046d451 into lightningnetwork:master Jul 27, 2021
@bhandras bhandras mentioned this pull request Aug 6, 2021
Merged
@bhandras bhandras deleted the payment_bench_timeout_flake branch September 12, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guggero guggero guggero approved these changes

@carlaKC carlaKC carlaKC approved these changes

Assignees
No one assigned
Labels
flake fix itests Issues related to integration tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bhandras @guggero @carlaKC