Merge pull request #5845 from guggero/itest-fixes-rest

itest: fix build matrix, WebSocket test timeout and log file upload

.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:
+        include:
-          - backend=btcd
+          - name: btcd
-          - backend=bitcoind
+            args: backend=btcd
-          - backend="bitcoind notxindex"
+          - name: bitcoind
-          - backend=bitcoind dbbackend=etcd
+            args: backend=bitcoind
-          - backend=bitcoind dbbackend=postgres
+          - name: bitcoind-notxindex
-          - backend=neutrino
+            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 }}
+      - name: run ${{ matrix.name }}
-        run: make itest-parallel ${{ matrix.unit_type }}
+        run: make itest-parallel ${{ matrix.args }}
 
-      - name: Upload Artifact
+      - name: Zip log files on failure
-        uses: actions/upload-artifact@v2
+        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 }}
+          name: logs-itest-${{ matrix.name }}
-          path: lntest/itest/**/*.log
+          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
 
   ########################

docs/release-notes/release-notes-0.14.0.md

@@ -419,6 +419,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

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