From 423de4d79a0c2c52506929604a0f62bc1740e3e6 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 8 Oct 2021 16:08:10 +0200 Subject: [PATCH 1/2] itest: defer close of done channel The testing framework uses runtime.Goexit() when a test fails which still allows the defer calls to execute. That's why we should use defer to close the done channel to allow all goroutines to exit properly. --- lntest/itest/lnd_rest_api_test.go | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/lntest/itest/lnd_rest_api_test.go b/lntest/itest/lnd_rest_api_test.go index eac605800..290b98171 100644 --- a/lntest/itest/lnd_rest_api_test.go +++ b/lntest/itest/lnd_rest_api_test.go @@ -438,8 +438,8 @@ func wsTestCaseBiDirectionalSubscription(ht *harnessTest, require.Nil(ht.t, err, "websocket") defer func() { err := conn.WriteMessage(websocket.CloseMessage, closeMsg) - require.NoError(ht.t, err) _ = conn.Close() + require.NoError(ht.t, err) }() // Buffer the message channel to make sure we're always blocking on @@ -576,21 +576,27 @@ func wsTestPingPongTimeout(ht *harnessTest, net *lntest.NetworkHarness) { require.Nil(ht.t, err, "websocket") defer func() { err := conn.WriteMessage(websocket.CloseMessage, closeMsg) - require.NoError(ht.t, err) _ = conn.Close() + require.NoError(ht.t, err) }() // We want to be able to read invoices for a long time, making sure we // can continue to read even after we've gone through several ping/pong // cycles. invoices := make(chan *lnrpc.Invoice, 1) - errors := make(chan error) + errChan := make(chan error) done := make(chan struct{}) + timeout := time.After(defaultTimeout) + + defer close(done) go func() { for { _, msg, err := conn.ReadMessage() if err != nil { - errors <- err + select { + case errChan <- err: + case <-done: + } return } @@ -599,7 +605,11 @@ func wsTestPingPongTimeout(ht *harnessTest, net *lntest.NetworkHarness) { // get rid of here. msgStr := string(msg) if !strings.Contains(msgStr, "\"result\":") { - errors <- fmt.Errorf("invalid msg: %s", msgStr) + select { + case errChan <- fmt.Errorf("invalid msg: %s", + msgStr): + case <-done: + } return } msgStr = resultPattern.ReplaceAllString(msgStr, "${1}") @@ -609,7 +619,10 @@ func wsTestPingPongTimeout(ht *harnessTest, net *lntest.NetworkHarness) { protoMsg := &lnrpc.Invoice{} err = jsonpb.UnmarshalString(msgStr, protoMsg) if err != nil { - errors <- err + select { + case errChan <- err: + case <-done: + } return } @@ -643,8 +656,11 @@ func wsTestPingPongTimeout(ht *harnessTest, net *lntest.NetworkHarness) { require.Equal(ht.t, int64(value), streamMsg.Value) require.Equal(ht.t, memo, streamMsg.Memo) - case err := <-errors: + case err := <-errChan: require.Fail(ht.t, "Error reading invoice: %v", err) + + case <-timeout: + require.Fail(ht.t, "No invoice msg received in time") } // Let's wait for at least a whole ping/pong cycle to happen, so @@ -652,7 +668,6 @@ func wsTestPingPongTimeout(ht *harnessTest, net *lntest.NetworkHarness) { // We double the pong wait just to add some extra margin. time.Sleep(pingInterval + 2*pongWait) } - close(done) } // invokeGET calls the given URL with the GET method and appropriate macaroon From 4e224fe0aa80dcf7d6b69dfabeec46ebdd6bf154 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 8 Oct 2021 16:32:17 +0200 Subject: [PATCH 2/2] GitHub: fix matrix, package log files before upload It turns out we were using the wrong matrix variable in the actual make command and ran the same itest 6 times with no arguments, all resulting in running the btcd test. To avoid uploading too many files in individual requests, we zip them first before uploading the zip itself. --- .github/workflows/main.yml | 44 ++++++++++++++-------- docs/release-notes/release-notes-0.14.0.md | 3 ++ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ec145f50e..6f35225ab 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -251,13 +251,19 @@ jobs: # Allow other tests in the matrix to continue if one fails. fail-fast: false matrix: - integration_type: - - backend=btcd - - backend=bitcoind - - backend="bitcoind notxindex" - - backend=bitcoind dbbackend=etcd - - backend=bitcoind dbbackend=postgres - - backend=neutrino + include: + - name: btcd + args: backend=btcd + - name: bitcoind + args: backend=bitcoind + - name: bitcoind-notxindex + args: backend="bitcoind notxindex" + - name: bitcoind-etcd + args: backend=bitcoind dbbackend=etcd + - name: bitcoind-postgres + args: backend=bitcoind dbbackend=postgres + - name: neutrino + args: backend=neutrino steps: - name: git checkout uses: actions/checkout@v2 @@ -281,15 +287,19 @@ jobs: - name: install bitcoind run: ./scripts/install_bitcoind.sh - - name: run ${{ matrix.unit_type }} - run: make itest-parallel ${{ matrix.unit_type }} + - name: run ${{ matrix.name }} + run: make itest-parallel ${{ matrix.args }} - - name: Upload Artifact - uses: actions/upload-artifact@v2 + - name: Zip log files on failure + if: ${{ failure() }} + run: 7z a logs-itest-${{ matrix.name }}.zip lntest/itest/**/*.log + + - name: Upload log files on failure + uses: actions/upload-artifact@v2.2.4 if: ${{ failure() }} with: - name: logs-itest-${{ job.id }} - path: lntest/itest/**/*.log + name: logs-itest-${{ matrix.name }} + path: logs-itest-${{ matrix.name }}.zip retention-days: 5 ######################## @@ -324,12 +334,16 @@ jobs: - name: run itest run: make itest-parallel windows=1 tranches=2 parallel=2 - - name: Upload Artifact + - name: Zip log files on failure + if: ${{ failure() }} + run: 7z a logs-itest-windows.zip lntest/itest/**/*.log + + - name: Upload log files on failure uses: actions/upload-artifact@v2 if: ${{ failure() }} with: name: logs-itest-windows - path: lntest/itest/**/*.log + path: logs-itest-windows.zip retention-days: 5 ######################## diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index 221051680..323aa7988 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -417,6 +417,9 @@ messages directly. There is no routing/path finding involved. * [Include htlc amount in bandwidth hints](https://github.com/lightningnetwork/lnd/pull/5512). +* [Fix REST/WebSocket API itest that lead to overall test + timeout](https://github.com/lightningnetwork/lnd/pull/5845). + ## Database * [Ensure single writer for legacy