Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: fix more flakes, move itests to GitHub (except ARM itest) #5811

Merged
merged 6 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 90 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
########################
Expand All @@ -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
Expand Down
63 changes: 4 additions & 59 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,71 +47,16 @@ jobs:
- GOGC=30 make lint

- stage: Integration Test
name: Btcd Integration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cy@ Travis 🤡

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'
5 changes: 3 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions docs/release-notes/release-notes-0.14.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 2 additions & 0 deletions lntest/btcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions lntest/itest/lnd_channel_force_close_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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[:],
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 15 additions & 2 deletions lntest/itest/test_harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var (
"lndexec", itestLndBinary, "full path to lnd binary",
)

slowMineDelay = 50 * time.Millisecond
slowMineDelay = 20 * time.Millisecond
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re commit message: why does decreasing this value slow things down?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already had the mineBlocksSlow function that used the 50ms delay. By replacing all instances of mineBlocks with mineBlocksSlow, we make slow everything down. To reduce the amount of overall slowdown we decrease the delay from 50ms to 20ms.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to update the commit message to make this more clear.

)

const (
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions lntest/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Comment on lines +1602 to +1604
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really understanding this comment? would this channel get closed when chanWatchRequests is finished with it? Also are we always sure this is close not another channel policy update?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's that if the channel has already been closed here, and we send in another request, it'll end up double closing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly.

select {
case <-eventChan:
return nil
default:
}

hn.chanWatchRequests <- &chanWatchRequest{
chanPoint: op,
eventChan: eventChan,
Expand Down
1 change: 1 addition & 0 deletions make/testing_flags.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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),)
Expand Down