itest: prepare itest for major refactor #6052

yyforyongyu · 2021-12-04T07:34:43Z

This PR has commits pulled out from #5939 for easier reviewing, only the last 6 commits are new.

Depends on #5756.

guggero

Nice refactor, LGTM 🎉

lntest/harness_node.go

lightninglabs-deploy · 2021-12-12T20:04:59Z

@positiveblue: review reminder

positiveblue

Thank you @yyforyongyu, test suites improvements keep adding up 🍾

lntest/harness_node.go

guggero · 2021-12-16T09:26:49Z

The RPC middleware itest seems to fail consistently now. Should I take a look or do you already have a hunch what's causing the failure?

yyforyongyu · 2021-12-16T16:53:27Z

The RPC middleware itest seems to fail consistently now. Should I take a look or do you already have a hunch what's causing the failure?

If you are talking about this one,

lnd_rpc_middleware_interceptor_test.go:407: 
        	Error Trace:	lnd_rpc_middleware_interceptor_test.go:407
        	            				lnd_rpc_middleware_interceptor_test.go:125
        	Error:      	Received unexpected error:
        	            	failed to subscribe to state: rpc error: code = Canceled desc = grpc: the client connection is closing

Yeah it's been fixed in the last PR.

guggero · 2022-01-03T12:40:41Z

The RPC interceptor test still seems to fail with failed to subscribe to state: rpc error: code = Canceled desc = grpc: the client connection is closing.
I'm not sure what you mean with "Yeah it's been fixed in the last PR.". Do you mean last commit? Or another PR and this needs to be rebased?

yyforyongyu · 2022-01-04T00:51:32Z

The RPC interceptor test still seems to fail with failed to subscribe to state: rpc error: code = Canceled desc = grpc: the client connection is closing. I'm not sure what you mean with "Yeah it's been fixed in the last PR.". Do you mean last commit? Or another PR and this needs to be rebased?

Sorry, I meant in the final PR #5940, in particular this commit.

This PR contains some commits taken from #5939. I'd say it's an "intermediate" PR that exists only for easier reviewing.

guggero · 2022-01-04T09:07:48Z

Ah I see. So should we wait with merging this PR? Or add that commit to it so we don't fully break the itests?

yyforyongyu · 2022-01-04T20:38:11Z

Ah I see. So should we wait with merging this PR? Or add that commit to it so we don't fully break the itests?

Yeah let's wait. Actually I'm kinda stuck here😂 and opinions are appreciated. These itest-related fix has gone quite far than I originally anticipated, and it's challenging to, say, cherry-pick some commits from later PRs so we have a clean build in each PR because the changes are chained. I'm thinking we could focus the itest build results in the final PR just to make sure the flakes are fixed. Meanwhile, we could somehow loosen checking the itest build results in the intermediate PRs? And for the following PR #5939, do you think it's too large? Maybe I should break it down even more?

This commit refactors the long function start() into smaller pieces and moves commonly used functions into test_common.go.

This commit adds a new struct RPCClients to better handle rpc clients. A private field, rpc, is added to HarnessNode to prevent direct access to its clients. Inside RPCClients, all clients are exported in case a test case need access to a specific client.

This commit adds a new component, harness miner, to the itest. This newly added component is responsible for checking the mempool and blocks for the itest.

This commit replaces the old miner with the new HarnessMiner and cleans harness_node.go by moving methods into the test_common.go.

guggero · 2022-01-05T12:50:26Z

I see, it's getting tricky indeed. But I'd rather not keep the itests in a state where they all fail reliably, that would render us completely blind to new bugs being introduced. Instead, I was able to extract the fix into a single commit that you can add to this PR so we can merge it (if nothing else pops up): guggero@dba9ade

yyforyongyu · 2022-01-06T07:02:37Z

I was able to extract the fix into a single commit that you can add to this PR so we can merge it

Cool cherry-picked.

yyforyongyu changed the title ~~Itest 2 refactor 1~~ itest: prepare itest for major refactor Dec 4, 2021

yyforyongyu added this to the flake hunting szn milestone Dec 4, 2021

yyforyongyu added the flake fix label Dec 4, 2021

yyforyongyu requested review from guggero and positiveblue December 4, 2021 08:33

yyforyongyu mentioned this pull request Dec 4, 2021

itest: fix previously known test flakes #5940

Closed

guggero approved these changes Dec 6, 2021

View reviewed changes

lntest/harness_node.go Show resolved Hide resolved

lntest/harness_node.go Show resolved Hide resolved

lntest/harness_node.go Outdated Show resolved Hide resolved

lntest/harness_node.go Outdated Show resolved Hide resolved

yyforyongyu force-pushed the itest-2-refactor-1 branch 2 times, most recently from 7bfc2fe to a4541b4 Compare December 9, 2021 19:08

yyforyongyu force-pushed the itest-2-refactor-1 branch from a4541b4 to 0bcddb4 Compare December 15, 2021 06:05

positiveblue approved these changes Dec 15, 2021

View reviewed changes

lntest/harness_node.go Show resolved Hide resolved

yyforyongyu force-pushed the itest-2-refactor-1 branch from 0bcddb4 to 5d9c606 Compare December 28, 2021 11:34

yyforyongyu force-pushed the itest-2-refactor-1 branch from 5d9c606 to fc211fb Compare January 3, 2022 23:33

yyforyongyu force-pushed the itest-2-refactor-1 branch from fc211fb to e8b09a6 Compare January 4, 2022 20:38

yyforyongyu added 6 commits January 5, 2022 13:10

itest: shorten functions inside harness node

f8cf7c8

This commit refactors the long function start() into smaller pieces and moves commonly used functions into test_common.go.

itest: add method waitTillServerState in harness node

75da5ca

itest: refactor initClientWhenReady to clean up init node

61d6f5d

itest: introduce harness miner

337aa66

This commit adds a new component, harness miner, to the itest. This newly added component is responsible for checking the mempool and blocks for the itest.

itest: start using harness miner in harness net

5872731

This commit replaces the old miner with the new HarnessMiner and cleans harness_node.go by moving methods into the test_common.go.

itest: fix RPC middleware itest

10d876a

yyforyongyu force-pushed the itest-2-refactor-1 branch from e8b09a6 to 10d876a Compare January 6, 2022 07:01

guggero merged commit 61bffa7 into lightningnetwork:master Jan 6, 2022

guggero modified the milestones: flake hunting szn, v0.15.0 Jan 6, 2022

yyforyongyu deleted the itest-2-refactor-1 branch January 7, 2022 02:13

guggero mentioned this pull request Jan 26, 2022

multi: create v0.14.2-beta-rc1 branch #6201

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

itest: prepare itest for major refactor #6052

itest: prepare itest for major refactor #6052

yyforyongyu commented Dec 4, 2021

guggero left a comment

lightninglabs-deploy commented Dec 12, 2021

positiveblue left a comment

guggero commented Dec 16, 2021

yyforyongyu commented Dec 16, 2021

guggero commented Jan 3, 2022

yyforyongyu commented Jan 4, 2022 •

edited

Loading

guggero commented Jan 4, 2022

yyforyongyu commented Jan 4, 2022

guggero commented Jan 5, 2022

yyforyongyu commented Jan 6, 2022

itest: prepare itest for major refactor #6052

itest: prepare itest for major refactor #6052

Conversation

yyforyongyu commented Dec 4, 2021

guggero left a comment

Choose a reason for hiding this comment

lightninglabs-deploy commented Dec 12, 2021

positiveblue left a comment

Choose a reason for hiding this comment

guggero commented Dec 16, 2021

yyforyongyu commented Dec 16, 2021

guggero commented Jan 3, 2022

yyforyongyu commented Jan 4, 2022 • edited Loading

guggero commented Jan 4, 2022

yyforyongyu commented Jan 4, 2022

guggero commented Jan 5, 2022

yyforyongyu commented Jan 6, 2022

yyforyongyu commented Jan 4, 2022 •

edited

Loading