From d6c2ee1bbfa5ee0dd78f1e929eb5bd60469480ca Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 29 Oct 2020 15:35:02 +0100 Subject: [PATCH 1/4] itest: use require.NoError in TestLightningNetworkDaemon --- lntest/itest/lnd_test.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 0639b7cf..1f77ff80 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -14278,16 +14278,13 @@ func TestLightningNetworkDaemon(t *testing.T) { err := lndHarness.EnsureConnected( context.Background(), lndHarness.Alice, lndHarness.Bob, ) - if err != nil { - t.Fatalf("unable to connect alice to bob: %v", err) - } + require.NoError(t, err, "unable to connect alice to bob") - if err := lndHarness.Alice.AddToLog(logLine); err != nil { - t.Fatalf("unable to add to log: %v", err) - } - if err := lndHarness.Bob.AddToLog(logLine); err != nil { - t.Fatalf("unable to add to log: %v", err) - } + err = lndHarness.Alice.AddToLog(logLine) + require.NoError(t, err, "unable to add to Alice's log") + + err = lndHarness.Bob.AddToLog(logLine) + require.NoError(t, err, "unable to add to Bob's log") // Start every test with the default static fee estimate. lndHarness.SetFeeEstimate(12500) From 979c8497b2267b84ae7f3b66d3b12c888a4b1f49 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 29 Oct 2020 15:37:17 +0100 Subject: [PATCH 2/4] itest: add icase to node log filename + restart nodes before each icase This commit adds the icase name to the log filename, to make it simpler to find problematic tests. Additionally after this commit we'll restart Alice and Bob (the base harness nodes) before each icase to start with a clean state. --- .gitignore | 5 ++-- lntest/harness.go | 37 +++++++++++++++---------- lntest/itest/lnd_test.go | 60 ++++++++++++++++++++++++++-------------- lntest/node.go | 33 ++++++++++++++-------- 4 files changed, 85 insertions(+), 50 deletions(-) diff --git a/.gitignore b/.gitignore index 371b57b6..daa3e220 100644 --- a/.gitignore +++ b/.gitignore @@ -32,8 +32,7 @@ _testmain.go /lncli-itest # Integration test log files -lntest/itest/output*.log -lntest/itest/pprof*.log +lntest/itest/*.log lntest/itest/.backendlogs lntest/itest/.minerlogs lntest/itest/lnd-itest @@ -73,4 +72,4 @@ profile.tmp .vscode # Coverage test -coverage.txt \ No newline at end of file +coverage.txt diff --git a/lntest/harness.go b/lntest/harness.go index bd459f4b..91aa88b0 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -36,6 +36,9 @@ const DefaultCSV = 4 type NetworkHarness struct { netParams *chaincfg.Params + // currentTestCase holds the name for the currently run test case. + currentTestCase string + // lndBinary is the full path to the lnd binary that was specifically // compiled with all required itest flags. lndBinary string @@ -131,10 +134,11 @@ func (f *fakeLogger) Println(args ...interface{}) {} // rpc clients capable of communicating with the initial seeder nodes are // created. Nodes are initialized with the given extra command line flags, which // should be formatted properly - "--arg=value". -func (n *NetworkHarness) SetUp(lndArgs []string) error { +func (n *NetworkHarness) SetUp(testCase string, lndArgs []string) error { // Swap out grpc's default logger with out fake logger which drops the // statements on the floor. grpclog.SetLogger(&fakeLogger{}) + n.currentTestCase = testCase // Start the initial seeder nodes within the test network, then connect // their respective RPC clients. @@ -241,21 +245,23 @@ out: return nil } -// TearDownAll tears down all active nodes within the test lightning network. -func (n *NetworkHarness) TearDownAll() error { - +// TearDown tears down all active nodes within the test lightning network. +func (n *NetworkHarness) TearDown() error { for _, node := range n.activeNodes { if err := n.ShutdownNode(node); err != nil { return err } } + return nil +} + +// Stop stops the test harness. +func (n *NetworkHarness) Stop() { close(n.lndErrorChan) close(n.quit) n.feeService.stop() - - return nil } // NewNode fully initializes a returns a new HarnessNode bound to the @@ -358,17 +364,18 @@ func (n *NetworkHarness) RestoreNodeWithSeed(name string, extraArgs []string, // wallet with or without a seed. If hasSeed is false, the returned harness node // can be used immediately. Otherwise, the node will require an additional // initialization phase where the wallet is either created or restored. -func (n *NetworkHarness) newNode(name string, extraArgs []string, - hasSeed bool, password []byte) (*HarnessNode, error) { +func (n *NetworkHarness) newNode(name string, extraArgs []string, hasSeed bool, + password []byte) (*HarnessNode, error) { node, err := newNode(NodeConfig{ - Name: name, - HasSeed: hasSeed, - Password: password, - BackendCfg: n.BackendCfg, - NetParams: n.netParams, - ExtraArgs: extraArgs, - FeeURL: n.feeService.url, + Name: name, + LogFilenamePrefix: n.currentTestCase, + HasSeed: hasSeed, + Password: password, + BackendCfg: n.BackendCfg, + NetParams: n.netParams, + ExtraArgs: extraArgs, + FeeURL: n.feeService.url, }) if err != nil { return nil, err diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 1f77ff80..18b1cfca 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -14232,7 +14232,7 @@ func TestLightningNetworkDaemon(t *testing.T) { if err != nil { ht.Fatalf("unable to create lightning network harness: %v", err) } - defer lndHarness.TearDownAll() + defer lndHarness.Stop() // Spawn a new goroutine to watch for any fatal errors that any of the // running lnd processes encounter. If an error occurs, then the test @@ -14265,34 +14265,52 @@ func TestLightningNetworkDaemon(t *testing.T) { aliceBobArgs := []string{ "--default-remote-max-htlcs=483", } - if err = lndHarness.SetUp(aliceBobArgs); err != nil { - ht.Fatalf("unable to set up test lightning network: %v", err) - } // Run the subset of the test cases selected in this tranche. for idx, testCase := range testCases { testCase := testCase - logLine := fmt.Sprintf("STARTING ============ %v ============\n", - testCase.name) - - err := lndHarness.EnsureConnected( - context.Background(), lndHarness.Alice, lndHarness.Bob, - ) - require.NoError(t, err, "unable to connect alice to bob") - - err = lndHarness.Alice.AddToLog(logLine) - require.NoError(t, err, "unable to add to Alice's log") - - err = lndHarness.Bob.AddToLog(logLine) - require.NoError(t, err, "unable to add to Bob's log") - - // Start every test with the default static fee estimate. - lndHarness.SetFeeEstimate(12500) - name := fmt.Sprintf("%02d-of-%d/%s/%s", trancheOffset+uint(idx)+1, len(allTestCases), chainBackend.Name(), testCase.name) + success := t.Run(name, func(t1 *testing.T) { + cleanTestCaseName := strings.ReplaceAll( + testCase.name, " ", "_", + ) + + err = lndHarness.SetUp(cleanTestCaseName, aliceBobArgs) + require.NoError(t1, + err, "unable to set up test lightning network", + ) + defer func() { + require.NoError(t1, lndHarness.TearDown()) + }() + + err = lndHarness.EnsureConnected( + context.Background(), lndHarness.Alice, + lndHarness.Bob, + ) + require.NoError(t1, + err, "unable to connect alice to bob", + ) + + logLine := fmt.Sprintf( + "STARTING ============ %v ============\n", + testCase.name, + ) + + err = lndHarness.Alice.AddToLog(logLine) + require.NoError(t1, err, "unable to add to log") + + err = lndHarness.Bob.AddToLog(logLine) + require.NoError(t1, err, "unable to add to log") + + // Start every test with the default static fee estimate. + lndHarness.SetFeeEstimate(12500) + + // Create a separate harness test for the testcase to + // avoid overwriting the external harness test that is + // tied to the parent test. ht := newHarnessTest(t1, lndHarness) ht.RunTestCase(testCase) }) diff --git a/lntest/node.go b/lntest/node.go index 32d1cadb..a437adb4 100644 --- a/lntest/node.go +++ b/lntest/node.go @@ -153,7 +153,12 @@ type BackendConfig interface { } type NodeConfig struct { - Name string + Name string + + // LogFilenamePrefix is is used to prefix node log files. Can be used + // to store the current test case for simpler postmortem debugging. + LogFilenamePrefix string + BackendCfg BackendConfig NetParams *chaincfg.Params BaseDir string @@ -493,17 +498,21 @@ func (hn *HarnessNode) start(lndBinary string, lndError chan<- error) error { // log files. if *logOutput { dir := GetLogDir() - fileName := fmt.Sprintf("%s/output-%d-%s-%s.log", dir, hn.NodeID, - hn.Cfg.Name, hex.EncodeToString(hn.PubKey[:logPubKeyBytes])) + fileName := fmt.Sprintf("%s/%d-%s-%s-%s.log", dir, hn.NodeID, + hn.Cfg.LogFilenamePrefix, hn.Cfg.Name, + hex.EncodeToString(hn.PubKey[:logPubKeyBytes])) - // If the node's PubKey is not yet initialized, create a temporary - // file name. Later, after the PubKey has been initialized, the - // file can be moved to its final name with the PubKey included. + // If the node's PubKey is not yet initialized, create a + // temporary file name. Later, after the PubKey has been + // initialized, the file can be moved to its final name with + // the PubKey included. if bytes.Equal(hn.PubKey[:4], []byte{0, 0, 0, 0}) { - fileName = fmt.Sprintf("%s/output-%d-%s-tmp__.log", - dir, hn.NodeID, hn.Cfg.Name) + fileName = fmt.Sprintf("%s/%d-%s-%s-tmp__.log", dir, + hn.NodeID, hn.Cfg.LogFilenamePrefix, + hn.Cfg.Name) - // Once the node has done its work, the log file can be renamed. + // Once the node has done its work, the log file can be + // renamed. finalizeLogfile = func() { if hn.logFile != nil { hn.logFile.Close() @@ -511,8 +520,10 @@ func (hn *HarnessNode) start(lndBinary string, lndError chan<- error) error { pubKeyHex := hex.EncodeToString( hn.PubKey[:logPubKeyBytes], ) - newFileName := fmt.Sprintf("%s/output"+ - "-%d-%s-%s.log", dir, hn.NodeID, + newFileName := fmt.Sprintf("%s/"+ + "%d-%s-%s-%s.log", + dir, hn.NodeID, + hn.Cfg.LogFilenamePrefix, hn.Cfg.Name, pubKeyHex) err := os.Rename(fileName, newFileName) if err != nil { From 9eac0dd3c95758f8553f14e347e2fd04bb26b0bb Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Fri, 30 Oct 2020 17:54:29 +0100 Subject: [PATCH 3/4] itest: fix crash in parallel macaroon tests --- lntest/itest/lnd_macaroons_test.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/lntest/itest/lnd_macaroons_test.go b/lntest/itest/lnd_macaroons_test.go index d9136019..343c1492 100644 --- a/lntest/itest/lnd_macaroons_test.go +++ b/lntest/itest/lnd_macaroons_test.go @@ -22,7 +22,7 @@ import ( // enabled on the gRPC interface, no requests with missing or invalid // macaroons are allowed. Further, the specific access rights (read/write, // entity based) and first-party caveats are tested as well. -func testMacaroonAuthentication(net *lntest.NetworkHarness, t *harnessTest) { +func testMacaroonAuthentication(net *lntest.NetworkHarness, ht *harnessTest) { var ( infoReq = &lnrpc.GetInfoRequest{} newAddrReq = &lnrpc.NewAddressRequest{ @@ -200,15 +200,13 @@ func testMacaroonAuthentication(net *lntest.NetworkHarness, t *harnessTest) { for _, tc := range testCases { tc := tc - t.t.Run(tc.name, func(t *testing.T) { - t.Parallel() - + ht.t.Run(tc.name, func(tt *testing.T) { ctxt, cancel := context.WithTimeout( context.Background(), defaultTimeout, ) defer cancel() - tc.run(ctxt, t) + tc.run(ctxt, tt) }) } } @@ -377,9 +375,7 @@ func testBakeMacaroon(net *lntest.NetworkHarness, t *harnessTest) { for _, tc := range testCases { tc := tc - t.t.Run(tc.name, func(t *testing.T) { - t.Parallel() - + t.t.Run(tc.name, func(tt *testing.T) { ctxt, cancel := context.WithTimeout( context.Background(), defaultTimeout, ) @@ -388,11 +384,11 @@ func testBakeMacaroon(net *lntest.NetworkHarness, t *harnessTest) { adminMac, err := testNode.ReadMacaroon( testNode.AdminMacPath(), defaultTimeout, ) - require.NoError(t, err) - cleanup, client := macaroonClient(t, testNode, adminMac) + require.NoError(tt, err) + cleanup, client := macaroonClient(tt, testNode, adminMac) defer cleanup() - tc.run(ctxt, t, client) + tc.run(ctxt, tt, client) }) } } From 38e8184926af0fe796c3b3bbc9d7ab2090ab4e2f Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Mon, 2 Nov 2020 16:37:11 +0100 Subject: [PATCH 4/4] itest: update error log whitelist --- lntest/itest/log_error_whitelist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/lntest/itest/log_error_whitelist.txt b/lntest/itest/log_error_whitelist.txt index 064158d5..79a5cb4a 100644 --- a/lntest/itest/log_error_whitelist.txt +++ b/lntest/itest/log_error_whitelist.txt @@ -49,6 +49,7 @@