Merge pull request #5811 from guggero/itest-flake-fix

CI: fix more flakes, move itests to GitHub (except ARM itest)
lightningnetwork · Oct 5, 2021 · 5cc10ef · 5cc10ef
2 parents af9b620 + 134be24
commit 5cc10ef
Show file tree

Hide file tree

Showing 11 changed files with 136 additions and 70 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -241,6 +241,95 @@ jobs:
           path-to-profile: coverage.txt
           parallel: true
 
+  ########################
+  # run integration tests
+  ########################
+  integration-test:
+    name: run itests
+    runs-on: ubuntu-latest
+    strategy:
+      # 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
+    steps:
+      - name: git checkout
+        uses: actions/checkout@v2
+
+      - name: go cache
+        uses: actions/cache@v1
+        with:
+          path: /home/runner/work/go
+          key: lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+            lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-
+            lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-
+            lnd-${{ runner.os }}-go-
+
+      - name: setup go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v2
+        with:
+          go-version: '${{ env.GO_VERSION }}'
+
+      - name: install bitcoind
+        run: ./scripts/install_bitcoind.sh
+
+      - name: run ${{ matrix.unit_type }}
+        run: make itest-parallel ${{ matrix.unit_type }}
+
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v2
+        with:
+          name: logs
+          path: lntest/itest/**/*.log
+          retention-days: 5
+
+  ########################
+  # run windows integration test
+  ########################
+  windows-integration-test:
+    name: run windows itest
+    runs-on: windows-latest
+    env:
+      GOCACHE: ${{ github.workspace }}/go/pkg/build
+      GOPATH: ${{ github.workspace }}/go
+    steps:
+      - name: git checkout
+        uses: actions/checkout@v2
+
+      - name: go cache
+        uses: actions/cache@v1
+        with:
+          path: ${{ env.GOPATH }}
+          key: lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }}
+            lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-
+            lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-
+            lnd-${{ runner.os }}-go-
+
+      - name: setup go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v2
+        with:
+          go-version: '${{ env.GO_VERSION }}'
+
+      - name: run itest
+        run: make itest-parallel windows=1 tranches=2 parallel=2
+
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v2
+        with:
+          name: logs
+          path: lntest/itest/**/*.log
+          retention-days: 5
+
   ########################
   # check pinned dependencies
   ########################
@@ -260,7 +349,7 @@ jobs:
         uses: actions/checkout@v2
 
       - name: ensure dependences at correct version
-        run:   if ! grep -q "${{ matrix.pinned_dep }}" go.mod; then echo dependency ${{ matrix.pinned_dep }} should not be altered ; exit 1 ; fi
+        run: if ! grep -q "${{ matrix.pinned_dep }}" go.mod; then echo dependency ${{ matrix.pinned_dep }} should not be altered ; exit 1 ; fi
 
   ########################
   # check PR updates release notes

diff --git a/.travis.yml b/.travis.yml
@@ -47,71 +47,16 @@ jobs:
         - GOGC=30 make lint
 
     - stage: Integration Test
-      name: Btcd Integration
-      script:
-        - make itest-parallel
-
-    - name: Bitcoind Integration (txindex enabled)
-      script:
-        - bash ./scripts/install_bitcoind.sh
-        - make itest-parallel backend=bitcoind
-
-    - name: Bitcoind Integration with etcd (txindex enabled)
-      script:
-        - bash ./scripts/install_bitcoind.sh
-        - make itest-parallel backend=bitcoind dbbackend=etcd
-
-    - name: Bitcoind Integration with postgres (txindex enabled)
-      script:
-        - bash ./scripts/install_bitcoind.sh
-        - make itest-parallel backend=bitcoind dbbackend=postgres
-
-    - name: Bitcoind Integration (txindex disabled)
-      script:
-        - bash ./scripts/install_bitcoind.sh
-        - make itest-parallel backend="bitcoind notxindex"
-
-    - name: Neutrino Integration
-      script:
-        - make itest-parallel backend=neutrino
-
-    - name: Bitcoind Integration ARM
+      name: Bitcoind Integration ARM
       script:
         - bash ./scripts/install_bitcoind.sh
         - GOARM=7 GOARCH=arm GOOS=linux make itest-parallel backend=bitcoind tranches=2 parallel=2
       arch: arm64
       services:
         - docker
 
-    - name: Btcd Integration Windows
-      script:
-        # The windows VM seems to be slower than the other Travis VMs. We only
-        # run 2 test suites in parallel instead of the default 4.
-        - make itest-parallel windows=1 tranches=2 parallel=2
-      os: windows
-      before_install:
-        - choco upgrade --no-progress -y make netcat curl findutils
-        - export MAKE=mingw32-make
-      after_failure:
-        - |-
-          case $TRAVIS_OS_NAME in
-            windows)
-              echo "Uploading to termbin.com..."
-              LOG_FILES=$(find ./lntest/itest -name '*.log')
-              for f in $LOG_FILES; do echo -n $f; cat $f | nc termbin.com 9999 | xargs -r0 printf ' uploaded to %s'; done
-              ;;
-          esac
-
 after_failure:
   - |-
-    case $TRAVIS_OS_NAME in
-      windows)
-        # Needs other commands, see after_script of the Windows build
-        ;;
-
-      *)
-        LOG_FILES=$(find ./lntest/itest -name '*.log')
-        echo "Uploading to termbin.com..." && for f in $LOG_FILES; do echo -n $f; cat $f | nc termbin.com 9999 | xargs -r0 printf ' uploaded to %s'; done
-        echo "Uploading to file.io..." && tar -zcvO $LOG_FILES | curl -s -F 'file=@-;filename=logs.tar.gz' https://file.io | xargs -r0 printf 'logs.tar.gz uploaded to %s\n'
-        ;;
-    esac
+    LOG_FILES=$(find ./lntest/itest -name '*.log')
+    echo "Uploading to termbin.com..." && for f in $LOG_FILES; do echo -n $f; cat $f | nc termbin.com 9999 | xargs -r0 printf ' uploaded to %s'; done
+    echo "Uploading to file.io..." && tar -zcvO $LOG_FILES | curl -s -F 'file=@-;filename=logs.tar.gz' https://file.io | xargs -r0 printf 'logs.tar.gz uploaded to %s\n'
diff --git a/Makefile b/Makefile
@@ -194,10 +194,11 @@ ifeq ($(dbbackend),postgres)
 
 	# Start a fresh postgres instance. Allow a maximum of 500 connections.
 	# This is required for the async benchmark to pass.
-	docker run --name lnd-postgres -e POSTGRES_PASSWORD=postgres -p 6432:5432 -d postgres -N 500
+	docker run --name lnd-postgres -e POSTGRES_PASSWORD=postgres -p 6432:5432 -d postgres:13-alpine -N 500
+	docker logs -f lnd-postgres &
 
 	# Wait for the instance to be started.
-	sleep 3
+	sleep $(POSTGRES_START_DELAY)
 endif
 
 itest-only: db-instance

diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md
@@ -224,6 +224,9 @@ you.
   1.17.1](https://github.com/lightningnetwork/lnd/pull/5650). All build tags have
   been updated accordingly to comply with the new Go 1.17.1 requirements.
 
+* [All integration tests (except the ARM itests) were moved from Travis CI to
+  GitHub Actions](https://github.com/lightningnetwork/lnd/pull/5811).
+
 ## Documentation
 
 * [Outdated warning about unsupported pruning was replaced with clarification that LND **does**

diff --git a/go.mod b/go.mod
@@ -5,7 +5,7 @@ require (
 	github.com/NebulousLabs/fastrand v0.0.0-20181203155948-6fb6489aac4e // indirect
 	github.com/NebulousLabs/go-upnp v0.0.0-20180202185039-29b680b06c82
 	github.com/Yawning/aez v0.0.0-20180114000226-4dad034d9db2
-	github.com/btcsuite/btcd v0.22.0-beta.0.20210916191717-f8e6854197cd
+	github.com/btcsuite/btcd v0.22.0-beta.0.20211005184431-e3449998be39
 	github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
 	github.com/btcsuite/btcutil v1.0.3-0.20210527170813-e2ba6805a890
 	github.com/btcsuite/btcutil/psbt v1.0.3-0.20210527170813-e2ba6805a890

diff --git a/go.sum b/go.sum
@@ -79,8 +79,8 @@ github.com/btcsuite/btcd v0.0.0-20190824003749-130ea5bddde3/go.mod h1:3J08xEfcug
 github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ=
 github.com/btcsuite/btcd v0.21.0-beta.0.20201208033208-6bd4c64a54fa/go.mod h1:Sv4JPQ3/M+teHz9Bo5jBpkNcP0x6r7rdihlNL/7tTAs=
 github.com/btcsuite/btcd v0.22.0-beta.0.20210803133449-f5a1fb9965e4/go.mod h1:9n5ntfhhHQBIhUvlhDvD3Qg6fRUj4jkN0VB8L8svzOA=
-github.com/btcsuite/btcd v0.22.0-beta.0.20210916191717-f8e6854197cd h1:Nq1fLF6IA8XfW0HTpLaVZoDKazt05J1C2AAeswYloBE=
-github.com/btcsuite/btcd v0.22.0-beta.0.20210916191717-f8e6854197cd/go.mod h1:9n5ntfhhHQBIhUvlhDvD3Qg6fRUj4jkN0VB8L8svzOA=
+github.com/btcsuite/btcd v0.22.0-beta.0.20211005184431-e3449998be39 h1:o6qacOzpKubr16y0RrE2fBauRZN1rDZ1YsE26ixCgQ0=
+github.com/btcsuite/btcd v0.22.0-beta.0.20211005184431-e3449998be39/go.mod h1:3PH+KbvLFfzBTCevQenPiDedjGQGt6aa70dVjJDWGTA=
 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo=
 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA=
 github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=

diff --git a/lntest/btcd.go b/lntest/btcd.go
@@ -90,6 +90,8 @@ func NewBackend(miner string, netParams *chaincfg.Params) (
 		// its requested data are not found. We add a nobanning flag to
 		// make sure they stay connected if it happens.
 		"--nobanning",
+		// Don't disconnect if a reply takes too long.
+		"--nostalldetect",
 	}
 	chainBackend, err := rpctest.New(netParams, nil, args, GetBtcdBinary())
 	if err != nil {

diff --git a/lntest/itest/lnd_channel_force_close_test.go b/lntest/itest/lnd_channel_force_close_test.go
@@ -94,9 +94,6 @@ func testCommitmentTransactionDeadline(net *lntest.NetworkHarness,
 	// calculateSweepFeeRate runs multiple steps to calculate the fee rate
 	// used in sweeping the transactions.
 	calculateSweepFeeRate := func(expectedSweepTxNum int) int64 {
-		ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout)
-		defer cancel()
-
 		// Create two nodes, Alice and Bob.
 		alice := setupNode("Alice")
 		defer shutdownAndAssert(net, t, alice)
@@ -118,6 +115,8 @@ func testCommitmentTransactionDeadline(net *lntest.NetworkHarness,
 		// Send a payment with a specified finalCTLVDelta, which will
 		// be used as our deadline later on when Alice force closes the
 		// channel.
+		ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout)
+		defer cancel()
 		_, err := alice.RouterClient.SendPaymentV2(
 			ctxt, &routerrpc.SendPaymentRequest{
 				Dest:           bob.PubKey[:],
@@ -145,6 +144,8 @@ func testCommitmentTransactionDeadline(net *lntest.NetworkHarness,
 		// Now that the channel has been force closed, it should show
 		// up in the PendingChannels RPC under the waiting close
 		// section.
+		ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout)
+		defer cancel()
 		pendingChansRequest := &lnrpc.PendingChannelsRequest{}
 		pendingChanResp, err := alice.PendingChannels(
 			ctxt, pendingChansRequest,

diff --git a/lntest/itest/test_harness.go b/lntest/itest/test_harness.go
@@ -31,7 +31,7 @@ var (
 		"lndexec", itestLndBinary, "full path to lnd binary",
 	)
 
-	slowMineDelay = 50 * time.Millisecond
+	slowMineDelay = 20 * time.Millisecond
 )
 
 const (
@@ -199,7 +199,7 @@ func waitForNTxsInMempool(miner *rpcclient.Client, n int,
 // mineBlocks mine 'num' of blocks and check that blocks are present in
 // node blockchain. numTxs should be set to the number of transactions
 // (excluding the coinbase) we expect to be included in the first mined block.
-func mineBlocks(t *harnessTest, net *lntest.NetworkHarness,
+func mineBlocksFast(t *harnessTest, net *lntest.NetworkHarness,
 	num uint32, numTxs int) []*wire.MsgBlock {
 
 	// If we expect transactions to be included in the blocks we'll mine,
@@ -240,6 +240,19 @@ func mineBlocks(t *harnessTest, net *lntest.NetworkHarness,
 	return blocks
 }
 
+// mineBlocksSlow mines 'num' of blocks and checks that blocks are present in
+// the mining node's blockchain. numTxs should be set to the number of
+// transactions (excluding the coinbase) we expect to be included in the first
+// mined block. Between each mined block an artificial delay is introduced to
+// give all network participants time to catch up.
+//
+// NOTE: This function currently is just an alias for mineBlocksSlow.
+func mineBlocks(t *harnessTest, net *lntest.NetworkHarness,
+	num uint32, numTxs int) []*wire.MsgBlock {
+
+	return mineBlocksSlow(t, net, num, numTxs)
+}
+
 // mineBlocksSlow mines 'num' of blocks and checks that blocks are present in
 // the mining node's blockchain. numTxs should be set to the number of
 // transactions (excluding the coinbase) we expect to be included in the first

diff --git a/lntest/node.go b/lntest/node.go
@@ -554,6 +554,8 @@ func NewMiner(baseLogDir, logFilename string, netParams *chaincfg.Params,
 		"--debuglevel=debug",
 		"--logdir=" + baseLogDir,
 		"--trickleinterval=100ms",
+		// Don't disconnect if a reply takes too long.
+		"--nostalldetect",
 	}
 
 	miner, err := rpctest.New(netParams, handler, args, btcdBinary)
@@ -1597,6 +1599,15 @@ func (hn *HarnessNode) WaitForChannelPolicyUpdate(ctx context.Context,
 		select {
 		// Send a watch request every second.
 		case <-ticker.C:
+			// Did the event can close in the meantime? We want to
+			// avoid a "close of closed channel" panic since we're
+			// re-using the same event chan for multiple requests.
+			select {
+			case <-eventChan:
+				return nil
+			default:
+			}
+
 			hn.chanWatchRequests <- &chanWatchRequest{
 				chanPoint:          op,
 				eventChan:          eventChan,

diff --git a/make/testing_flags.mk b/make/testing_flags.mk
@@ -7,6 +7,7 @@ EXEC_SUFFIX =
 COVER_PKG = $$(go list -deps -tags="$(DEV_TAGS)" ./... | grep '$(PKG)' | grep -v lnrpc)
 NUM_ITEST_TRANCHES = 4
 ITEST_PARALLELISM = $(NUM_ITEST_TRANCHES)
+POSTGRES_START_DELAY = 5
 
 # If rpc option is set also add all extra RPC tags to DEV_TAGS
 ifneq ($(with-rpc),)