From a79c13230373f75d896fbb38c198402a78ddb2b4 Mon Sep 17 00:00:00 2001 From: Woosang Son Date: Mon, 3 May 2021 18:35:52 +0900 Subject: [PATCH] fix: test-race failure (#159) * fix: test-race failure * chore: to increase codecov * chore: increase codecov --- .github/workflows/test.yml | 144 ++++++++++++++++++------------------- baseapp/abci.go | 12 +++- baseapp/baseapp.go | 10 ++- client/keys/utils_test.go | 35 +++++++++ simapp/utils_test.go | 17 +++++ 5 files changed, 136 insertions(+), 82 deletions(-) create mode 100644 client/keys/utils_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 29ac39d6e4..70880742c7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -198,80 +198,78 @@ jobs: file: ./coverage.txt if: env.GIT_DIFF -# TODO: disable test-race. please enable this after fixing concurrent checkTx -# -# test-race: -# runs-on: ubuntu-latest -# needs: split-test-files -# strategy: -# fail-fast: false -# matrix: -# part: ["00", "01", "02", "03"] -# steps: -# - name: Configure git for private modules -# env: -# TOKEN: ${{ secrets.TOKEN }} -# run: git config --global url."https://${TOKEN}:x-oauth-basic@github.com".insteadOf "https://github.com" -# - uses: actions/checkout@v2 -# - uses: actions/setup-go@v2.1.3 -# with: -# go-version: 1.15 -# - uses: technote-space/get-diff-action@v4 -# with: -# PATTERNS: | -# **/**.go -# go.mod -# go.sum -# - uses: actions/download-artifact@v2 -# with: -# name: "${{ github.sha }}-${{ matrix.part }}" -# if: env.GIT_DIFF -# - name: test & coverage report creation -# run: | -# xargs --arg-file=pkgs.txt.part.${{ matrix.part }} go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock goleveldb' | tee ${{ matrix.part }}-race-output.txt -# if: env.GIT_DIFF -# - uses: actions/upload-artifact@v2 -# with: -# name: "${{ github.sha }}-${{ matrix.part }}-race-output" -# path: ./${{ matrix.part }}-race-output.txt + test-race: + runs-on: ubuntu-latest + needs: split-test-files + strategy: + fail-fast: false + matrix: + part: ["00", "01", "02", "03"] + steps: + - name: Configure git for private modules + env: + TOKEN: ${{ secrets.TOKEN }} + run: git config --global url."https://${TOKEN}:x-oauth-basic@github.com".insteadOf "https://github.com" + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2.1.3 + with: + go-version: 1.15 + - uses: technote-space/get-diff-action@v4 + with: + PATTERNS: | + **/**.go + go.mod + go.sum + - uses: actions/download-artifact@v2 + with: + name: "${{ github.sha }}-${{ matrix.part }}" + if: env.GIT_DIFF + - name: test & coverage report creation + run: | + xargs --arg-file=pkgs.txt.part.${{ matrix.part }} go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock goleveldb' | tee ${{ matrix.part }}-race-output.txt + if: env.GIT_DIFF + - uses: actions/upload-artifact@v2 + with: + name: "${{ github.sha }}-${{ matrix.part }}-race-output" + path: ./${{ matrix.part }}-race-output.txt -# race-detector-report: -# runs-on: ubuntu-latest -# needs: [test-race, install-tparse] -# timeout-minutes: 5 -# steps: -# - uses: actions/checkout@v2 -# - uses: technote-space/get-diff-action@v4 -# id: git_diff -# with: -# PATTERNS: | -# **/**.go -# go.mod -# go.sum -# - uses: actions/download-artifact@v2 -# with: -# name: "${{ github.sha }}-00-race-output" -# if: env.GIT_DIFF -# - uses: actions/download-artifact@v2 -# with: -# name: "${{ github.sha }}-01-race-output" -# if: env.GIT_DIFF -# - uses: actions/download-artifact@v2 -# with: -# name: "${{ github.sha }}-02-race-output" -# if: env.GIT_DIFF -# - uses: actions/download-artifact@v2 -# with: -# name: "${{ github.sha }}-03-race-output" -# if: env.GIT_DIFF -# - uses: actions/cache@v2.1.3 -# with: -# path: ~/go/bin -# key: ${{ runner.os }}-go-tparse-binary -# if: env.GIT_DIFF -# - name: Generate test report (go test -race) -# run: cat ./*-race-output.txt | ~/go/bin/tparse -# if: env.GIT_DIFF + race-detector-report: + runs-on: ubuntu-latest + needs: [test-race, install-tparse] + timeout-minutes: 5 + steps: + - uses: actions/checkout@v2 + - uses: technote-space/get-diff-action@v4 + id: git_diff + with: + PATTERNS: | + **/**.go + go.mod + go.sum + - uses: actions/download-artifact@v2 + with: + name: "${{ github.sha }}-00-race-output" + if: env.GIT_DIFF + - uses: actions/download-artifact@v2 + with: + name: "${{ github.sha }}-01-race-output" + if: env.GIT_DIFF + - uses: actions/download-artifact@v2 + with: + name: "${{ github.sha }}-02-race-output" + if: env.GIT_DIFF + - uses: actions/download-artifact@v2 + with: + name: "${{ github.sha }}-03-race-output" + if: env.GIT_DIFF + - uses: actions/cache@v2.1.3 + with: + path: ~/go/bin + key: ${{ runner.os }}-go-tparse-binary + if: env.GIT_DIFF + - name: Generate test report (go test -race) + run: cat ./*-race-output.txt | ~/go/bin/tparse + if: env.GIT_DIFF # TODO ebony: enable this test # liveness-test: diff --git a/baseapp/abci.go b/baseapp/abci.go index 1f983d3645..2300dce003 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -152,6 +152,9 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg panic(err) } + // set the signed validators for addition to context in deliverTx + app.voteInfos = req.LastCommitInfo.GetVotes() + // Initialize the DeliverTx state. If this is the first block, it should // already be initialized in InitChain. Otherwise app.deliverState will be // nil, since it is reset on Commit. @@ -173,14 +176,15 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg gasMeter = sdk.NewInfiniteGasMeter() } - app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(gasMeter) + app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(gasMeter). + WithVoteInfos(app.voteInfos). + WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx)) if app.beginBlocker != nil { res = app.beginBlocker(app.deliverState.ctx, req) res.Events = sdk.MarkEventsToIndex(res.Events, app.indexEvents) } - // set the signed validators for addition to context in deliverTx - app.voteInfos = req.LastCommitInfo.GetVotes() + return res } @@ -654,9 +658,11 @@ func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, e } // branch the commit-multistore for safety + app.checkStateMtx.RLock() ctx := sdk.NewContext( cacheMS, app.checkState.ctx.BlockHeader(), true, app.logger, ).WithMinGasPrices(app.minGasPrices) + app.checkStateMtx.RUnlock() return ctx, nil } diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 8297c979e0..344bdd5ded 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -367,7 +367,8 @@ func (app *BaseApp) setCheckState(header ostproto.Header) { defer app.checkStateMtx.Unlock() ctx := sdk.NewContext(ms, header, true, app.logger). - WithMinGasPrices(app.minGasPrices) + WithMinGasPrices(app.minGasPrices). + WithVoteInfos(app.voteInfos) app.checkState = &state{ ms: ms, @@ -528,11 +529,8 @@ func (app *BaseApp) getRunContextForTx(txBytes []byte, simulate bool) sdk.Contex } func (app *BaseApp) getContextForTx(s *state, txBytes []byte) sdk.Context { - ctx := s.ctx. - WithTxBytes(txBytes). - WithVoteInfos(app.voteInfos) - - return ctx.WithConsensusParams(app.GetConsensusParams(ctx)) + ctx := s.ctx.WithTxBytes(txBytes) + return ctx } // cacheTxContext returns a new context based off of the provided context with diff --git a/client/keys/utils_test.go b/client/keys/utils_test.go new file mode 100644 index 0000000000..bae33b3cbf --- /dev/null +++ b/client/keys/utils_test.go @@ -0,0 +1,35 @@ +package keys + +import ( + "bytes" + "testing" + + "github.com/line/lbm-sdk/v2/client" + "github.com/line/lbm-sdk/v2/crypto/hd" + "github.com/line/lbm-sdk/v2/crypto/keyring" + "github.com/line/lbm-sdk/v2/testutil" + "github.com/line/lbm-sdk/v2/types" + "github.com/stretchr/testify/require" +) + +func Test_printInfos(t *testing.T) { + cmd := ListKeysCmd() + cmd.Flags().AddFlagSet(Commands("home").PersistentFlags()) + + kbHome := t.TempDir() + + mockIn := testutil.ApplyMockIODiscardOutErr(cmd) + kb, err := keyring.New(types.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn) + require.NoError(t, err) + + kb.NewAccount("something", testutil.TestMnemonic, "", "", hd.Secp256k1) + + clientCtx := client.Context{}.WithKeyring(kb) + require.NoError(t, err) + + infos, err := clientCtx.Keyring.List() + require.NoError(t, err) + buf := bytes.NewBufferString("") + printInfos(buf, infos, OutputFormatJSON) + require.Equal(t, buf.String(), "[{\"name\":\"something\",\"type\":\"local\",\"address\":\"link1jyyxx9phqw6tarnxanhyx7ecr992d6yrztj4d0\",\"pubkey\":\"linkpub1cqmsrdepqg8qdahungt0uladr5g4v6kh56x4wx7auws5rfxunyrapnqn7kfkyrszyvh\"}]") +} diff --git a/simapp/utils_test.go b/simapp/utils_test.go index 6f86c0f6cc..7bef72b65f 100644 --- a/simapp/utils_test.go +++ b/simapp/utils_test.go @@ -4,6 +4,8 @@ import ( "fmt" "testing" + "github.com/line/ostracon/abci/types" + types2 "github.com/line/ostracon/proto/ostracon/types" "github.com/stretchr/testify/require" "github.com/line/lbm-sdk/v2/codec" @@ -23,6 +25,21 @@ func makeCodec(bm module.BasicManager) *codec.LegacyAmino { return cdc } +func TestSetup(t *testing.T) { + app := Setup(false) + ctx := app.BaseApp.NewContext(false, types2.Header{}) + + app.InitChain( + types.RequestInitChain{ + AppStateBytes: []byte("{}"), + ChainId: "test-chain-id", + }, + ) + + acc := app.AccountKeeper.GetAccount(ctx, authtypes.NewModuleAddress(authtypes.FeeCollectorName)) + require.NotNil(t, acc) +} + func TestGetSimulationLog(t *testing.T) { cdc := makeCodec(ModuleBasics)