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.

Sign up for GitHub

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

refactor(log): associate test logger with testing.T instance #15261

Merged
merged 5 commits into from
Mar 3, 2023
Merged

refactor(log): associate test logger with testing.T instance #15261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f07954
refactor(log): associate test logger with testing.T
mark-rushakoff Mar 3, 2023
74c894f
test: use log.NewTestLogger in tests
mark-rushakoff Mar 3, 2023
0ed8a41
chore: use cosmossdk.io/log commit from #15261
mark-rushakoff Mar 3, 2023
862fef6
chore: update changelog
mark-rushakoff Mar 3, 2023
351e2e2
chore: fix copy-paste error in e2e server test
mark-rushakoff Mar 3, 2023
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
14 changes: 6 additions & 8 deletions baseapp/abci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/stretchr/testify/require"

errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
pruningtypes "cosmossdk.io/store/pruning/types"
"cosmossdk.io/store/snapshots"
snapshottypes "cosmossdk.io/store/snapshots/types"
Expand Down Expand Up @@ -46,7 +47,7 @@ func TestABCI_Info(t *testing.T) {
func TestABCI_InitChain(t *testing.T) {
name := t.Name()
db := dbm.NewMemDB()
logger := defaultLogger()
logger := log.NewTestLogger(t)
app := baseapp.NewBaseApp(name, logger, db, nil)

capKey := storetypes.NewKVStoreKey("main")
Expand Down Expand Up @@ -126,8 +127,7 @@ func TestABCI_InitChain(t *testing.T) {
func TestABCI_InitChain_WithInitialHeight(t *testing.T) {
name := t.Name()
db := dbm.NewMemDB()
logger := defaultLogger()
app := baseapp.NewBaseApp(name, logger, db, nil)
app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil)

app.InitChain(
abci.RequestInitChain{
Expand All @@ -142,8 +142,7 @@ func TestABCI_InitChain_WithInitialHeight(t *testing.T) {
func TestABCI_BeginBlock_WithInitialHeight(t *testing.T) {
name := t.Name()
db := dbm.NewMemDB()
logger := defaultLogger()
app := baseapp.NewBaseApp(name, logger, db, nil)
app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil)

app.InitChain(
abci.RequestInitChain{
Expand Down Expand Up @@ -566,15 +565,14 @@ func TestABCI_ApplySnapshotChunk(t *testing.T) {
func TestABCI_EndBlock(t *testing.T) {
db := dbm.NewMemDB()
name := t.Name()
logger := defaultLogger()

cp := &cmtproto.ConsensusParams{
Block: &cmtproto.BlockParams{
MaxGas: 5000000,
},
}

app := baseapp.NewBaseApp(name, logger, db, nil)
app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil)
app.SetParamStore(&paramStore{db: dbm.NewMemDB()})
app.InitChain(abci.RequestInitChain{
ConsensusParams: cp,
Expand Down Expand Up @@ -1211,7 +1209,7 @@ func TestABCI_Query(t *testing.T) {
}

func TestABCI_GetBlockRetentionHeight(t *testing.T) {
logger := defaultLogger()
logger := log.NewTestLogger(t)
db := dbm.NewMemDB()
name := t.Name()

Expand Down
16 changes: 6 additions & 10 deletions baseapp/baseapp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@ func NewBaseAppSuite(t *testing.T, opts ...func(*baseapp.BaseApp)) *BaseAppSuite
baseapptestutil.RegisterInterfaces(cdc.InterfaceRegistry())

txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes)
logger := defaultLogger()
db := dbm.NewMemDB()

app := baseapp.NewBaseApp(t.Name(), logger, db, txConfig.TxDecoder(), opts...)
app := baseapp.NewBaseApp(t.Name(), log.NewTestLogger(t), db, txConfig.TxDecoder(), opts...)
require.Equal(t, t.Name(), app.Name())

app.SetInterfaceRegistry(cdc.InterfaceRegistry())
Expand Down Expand Up @@ -159,7 +158,7 @@ func NewBaseAppSuiteWithSnapshots(t *testing.T, cfg SnapshotsConfig, opts ...fun
}

func TestLoadVersion(t *testing.T) {
logger := defaultLogger()
logger := log.NewTestLogger(t)
pruningOpt := baseapp.SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningNothing))
db := dbm.NewMemDB()
name := t.Name()
Expand Down Expand Up @@ -284,7 +283,7 @@ func TestSetLoader(t *testing.T) {
if tc.setLoader != nil {
opts = append(opts, tc.setLoader)
}
app := baseapp.NewBaseApp(t.Name(), defaultLogger(), db, nil, opts...)
app := baseapp.NewBaseApp(t.Name(), log.NewTestLogger(t), db, nil, opts...)
app.MountStores(storetypes.NewKVStoreKey(tc.loadStoreKey))
err := app.LoadLatestVersion()
require.Nil(t, err)
Expand All @@ -302,11 +301,10 @@ func TestSetLoader(t *testing.T) {
}

func TestVersionSetterGetter(t *testing.T) {
logger := defaultLogger()
pruningOpt := baseapp.SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningDefault))
db := dbm.NewMemDB()
name := t.Name()
app := baseapp.NewBaseApp(name, logger, db, nil, pruningOpt)
app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil, pruningOpt)

require.Equal(t, "", app.Version())
res := app.Query(abci.RequestQuery{Path: "app/version"})
Expand Down Expand Up @@ -361,9 +359,8 @@ func TestOptionFunction(t *testing.T) {
}
}

logger := defaultLogger()
db := dbm.NewMemDB()
bap := baseapp.NewBaseApp("starting name", logger, db, nil, testChangeNameHelper("new name"))
bap := baseapp.NewBaseApp("starting name", log.NewTestLogger(t), db, nil, testChangeNameHelper("new name"))
require.Equal(t, bap.Name(), "new name", "BaseApp should have had name changed via option function")
}

Expand Down Expand Up @@ -543,10 +540,9 @@ func TestBaseAppAnteHandler(t *testing.T) {
func TestABCI_CreateQueryContext(t *testing.T) {
t.Parallel()

logger := defaultLogger()
db := dbm.NewMemDB()
name := t.Name()
app := baseapp.NewBaseApp(name, logger, db, nil)
app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil)

app.BeginBlock(abci.RequestBeginBlock{Header: cmtproto.Header{Height: 1}})
app.Commit()
Expand Down
2 changes: 1 addition & 1 deletion baseapp/grpcrouter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestRegisterQueryServiceTwice(t *testing.T) {
err := depinject.Inject(makeMinimalConfig(), &appBuilder)
require.NoError(t, err)
db := dbm.NewMemDB()
app := appBuilder.Build(log.NewLogger(), db, nil)
app := appBuilder.Build(log.NewTestLogger(t), db, nil)

// First time registering service shouldn't panic.
require.NotPanics(t, func() {
Expand Down
4 changes: 2 additions & 2 deletions baseapp/msg_service_router_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestRegisterMsgService(t *testing.T) {
)
err := depinject.Inject(makeMinimalConfig(), &appBuilder, &registry)
require.NoError(t, err)
app := appBuilder.Build(log.NewLogger(), dbm.NewMemDB(), nil)
app := appBuilder.Build(log.NewTestLogger(t), dbm.NewMemDB(), nil)

require.Panics(t, func() {
testdata.RegisterMsgServer(
Expand Down Expand Up @@ -57,7 +57,7 @@ func TestRegisterMsgServiceTwice(t *testing.T) {
err := depinject.Inject(makeMinimalConfig(), &appBuilder, &registry)
require.NoError(t, err)
db := dbm.NewMemDB()
app := appBuilder.Build(log.NewLogger(), db, nil)
app := appBuilder.Build(log.NewTestLogger(t), db, nil)
testdata.RegisterInterfaces(registry)

// First time registering service shouldn't panic.
Expand Down
9 changes: 0 additions & 9 deletions baseapp/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
dbm "github.com/cosmos/cosmos-db"
"github.com/stretchr/testify/require"

"cosmossdk.io/log"
"github.com/cosmos/cosmos-sdk/baseapp"
baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil"
"github.com/cosmos/cosmos-sdk/client"
Expand All @@ -52,14 +51,6 @@ import (

var ParamStoreKey = []byte("paramstore")

func defaultLogger() log.Logger {
if testing.Verbose() {
return log.NewLoggerWithKV("module", "baseapp/test")
}

return log.NewNopLogger()
}

// GenesisStateWithSingleValidator initializes GenesisState with a single validator and genesis accounts
// that also act as delegators.
func GenesisStateWithSingleValidator(t *testing.T, codec codec.Codec, builder *runtime.AppBuilder) map[string]json.RawMessage {
Expand Down
3 changes: 3 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ require (
nhooyr.io/websocket v1.8.6 // indirect
)

// Short-lived replace for an outstanding PR of log.
Copy link
Member

@julienrbrt julienrbrt Mar 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What we do now (for tools actively in dev) is to use the commit of the PR to create a pseudo version, so it limits the number of PRs.
We have a script to update it everywhere easily ./scripts/go-update-dep-all.sh cosmossdk.io/log@commit

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I just pushed 0ed8a41 which references commit 74c894f from this PR.

Copy link
Member

@julienrbrt julienrbrt Mar 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This one got forgotten I think.

replace cosmossdk.io/log => ./log

// Below are the long-lived replace of the Cosmos SDK
replace (
// use cosmos fork of keyring
Expand Down
4 changes: 4 additions & 0 deletions log/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
# Changelog

## [Unreleased]

### Improvements

- [#15261](https://github.com/cosmos/cosmos-sdk/pull/15261): Provide `log.NewTestLogger(*testing.T)` function to create new log.Logger associated with a test
34 changes: 20 additions & 14 deletions log/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
package log

import "testing"
import "github.com/rs/zerolog"

// reuse the same logger across all tests
var _testingLogger Logger
// TestingT is the interface required for logging in tests.
// It is a subset of testing.T to avoid a direct dependency on the testing package.
type TestingT zerolog.TestingLog

func NewTestingLogger() Logger {
if _testingLogger != nil {
return _testingLogger
// NewTestLogger returns a logger that calls t.Log to write entries.
//
// If the logs may help debug a test failure,
// you may want to use NewTestLogger(t) in your test.
// Otherwise, use NewNopLogger().
func NewTestLogger(t TestingT) Logger {
cw := zerolog.NewConsoleWriter()
cw.Out = zerolog.TestWriter{
T: t,
// Normally one would use zerolog.ConsoleTestWriter
// to set the option on NewConsoleWriter,
// but the zerolog source for that is hardcoded to Frame=6.
// With Frame=6, all source locations are printed as "logger.go",
// but Frame=7 prints correct source locations.
Frame: 7,
}

if testing.Verbose() {
_testingLogger = NewLoggerWithKV(ModuleKey, "test")
} else {
_testingLogger = NewNopLogger()
}

return _testingLogger
return NewCustomLogger(zerolog.New(cw))
}
31 changes: 19 additions & 12 deletions server/mock/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,30 @@ import (
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/stretchr/testify/require"

"cosmossdk.io/log"

simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types"
)

func TestInitApp(t *testing.T) {
app, closer, err := SetupApp()
// closer may need to be run, even when error in later stage
if closer != nil {
defer closer()
}
// SetupApp initializes a new application,
// failing t if initialization fails.
func SetupApp(t *testing.T) abci.Application {
t.Helper()

logger := log.NewTestLogger(t)

rootDir := t.TempDir()

app, err := NewApp(rootDir, logger)
require.NoError(t, err)

return app
}

func TestInitApp(t *testing.T) {
app := SetupApp(t)

appState, err := AppGenState(nil, genutiltypes.AppGenesis{}, nil)
require.NoError(t, err)

Expand All @@ -42,12 +54,7 @@ func TestInitApp(t *testing.T) {
}

func TestDeliverTx(t *testing.T) {
app, closer, err := SetupApp()
// closer may need to be run, even when error in later stage
if closer != nil {
defer closer()
}
require.NoError(t, err)
app := SetupApp(t)

key := "my-special-key"
value := "top-secret-data!!"
Expand Down
29 changes: 0 additions & 29 deletions server/mock/helpers.go
View file
Open in desktop

This file was deleted.

19 changes: 8 additions & 11 deletions simapp/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ import (

func TestSimAppExportAndBlockedAddrs(t *testing.T) {
db := dbm.NewMemDB()
logger := log.NewLogger()
logger := log.NewTestLogger(t)
app := NewSimappWithCustomOptions(t, false, SetupOptions{
Logger: logger,
Logger: logger.With("instance", "first"),
DB: db,
AppOpts: simtestutil.NewAppOptionsWithFlagHome(t.TempDir()),
})
Expand All @@ -66,20 +66,19 @@ func TestSimAppExportAndBlockedAddrs(t *testing.T) {

app.Commit()

logger2 := log.NewLogger()
// Making a new app object with the db, so that initchain hasn't been called
app2 := NewSimApp(logger2, db, nil, true, simtestutil.NewAppOptionsWithFlagHome(t.TempDir()))
app2 := NewSimApp(logger.With("instance", "second"), db, nil, true, simtestutil.NewAppOptionsWithFlagHome(t.TempDir()))
_, err := app2.ExportAppStateAndValidators(false, []string{}, []string{})
require.NoError(t, err, "ExportAppStateAndValidators should not have an error")
}

func TestRunMigrations(t *testing.T) {
db := dbm.NewMemDB()
logger := log.NewLogger()
app := NewSimApp(logger, db, nil, true, simtestutil.NewAppOptionsWithFlagHome(t.TempDir()))
logger := log.NewTestLogger(t)
app := NewSimApp(logger.With("instance", "simapp"), db, nil, true, simtestutil.NewAppOptionsWithFlagHome(t.TempDir()))

// Create a new baseapp and configurator for the purpose of this test.
bApp := baseapp.NewBaseApp(app.Name(), logger, db, app.TxConfig().TxDecoder())
bApp := baseapp.NewBaseApp(app.Name(), logger.With("instance", "baseapp"), db, app.TxConfig().TxDecoder())
bApp.SetCommitMultiStoreTracer(nil)
bApp.SetInterfaceRegistry(app.InterfaceRegistry())
app.BaseApp = bApp
Expand Down Expand Up @@ -216,8 +215,7 @@ func TestRunMigrations(t *testing.T) {

func TestInitGenesisOnMigration(t *testing.T) {
db := dbm.NewMemDB()
logger := log.NewLogger()
app := NewSimApp(logger, db, nil, true, simtestutil.NewAppOptionsWithFlagHome(t.TempDir()))
app := NewSimApp(log.NewTestLogger(t), db, nil, true, simtestutil.NewAppOptionsWithFlagHome(t.TempDir()))
ctx := app.NewContext(true, cmtproto.Header{Height: app.LastBlockHeight()})

// Create a mock module. This module will serve as the new module we're
Expand Down Expand Up @@ -259,9 +257,8 @@ func TestInitGenesisOnMigration(t *testing.T) {

func TestUpgradeStateOnGenesis(t *testing.T) {
db := dbm.NewMemDB()
logger := log.NewLogger()
app := NewSimappWithCustomOptions(t, false, SetupOptions{
Logger: logger,
Logger: log.NewTestLogger(t),
DB: db,
AppOpts: simtestutil.NewAppOptionsWithFlagHome(t.TempDir()),
})
Expand Down
2 changes: 1 addition & 1 deletion simapp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
cosmossdk.io/client/v2 v2.0.0-20230220152935-67f04e629623
cosmossdk.io/core v0.6.0
cosmossdk.io/depinject v1.0.0-alpha.3
cosmossdk.io/log v0.0.0-20230227204852-3535ee51c728
cosmossdk.io/log v0.0.0-20230303183710-74c894f12720
cosmossdk.io/math v1.0.0-beta.6.0.20230216172121-959ce49135e4
cosmossdk.io/store v0.0.0-20230227103508-bbe7f8a11b44
cosmossdk.io/tools/confix v0.0.0-20230120150717-4f6f6c00021f
Expand Down
4 changes: 2 additions & 2 deletions simapp/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z
cosmossdk.io/depinject v1.0.0-alpha.3/go.mod h1:eRbcdQ7MRpIPEM5YUJh8k97nxHpYbc3sMUnEtt8HPWU=
cosmossdk.io/errors v1.0.0-beta.7 h1:gypHW76pTQGVnHKo6QBkb4yFOJjC+sUGRc5Al3Odj1w=
cosmossdk.io/errors v1.0.0-beta.7/go.mod h1:mz6FQMJRku4bY7aqS/Gwfcmr/ue91roMEKAmDUDpBfE=
cosmossdk.io/log v0.0.0-20230227204852-3535ee51c728 h1:OpmiiO3x9esYawh4WX7jLL0UzBoOfjZrx3SdM3l8lhE=
cosmossdk.io/log v0.0.0-20230227204852-3535ee51c728/go.mod h1:FZRNfYm6Jgp+toyDR8Ek1qrmNjdsDddEQnfATMbpGYg=
cosmossdk.io/log v0.0.0-20230303183710-74c894f12720 h1:yrSwsgagxMHe/cj5V0TRRxNyr1k1cshizqaOGU4McKE=
cosmossdk.io/log v0.0.0-20230303183710-74c894f12720/go.mod h1:ilL9YXutMQo36MS9CD1cFNUqqHadAtyf8+A2b2EqO5A=
cosmossdk.io/math v1.0.0-beta.6.0.20230216172121-959ce49135e4 h1:/jnzJ9zFsL7qkV8LCQ1JH3dYHh2EsKZ3k8Mr6AqqiOA=
cosmossdk.io/math v1.0.0-beta.6.0.20230216172121-959ce49135e4/go.mod h1:gUVtWwIzfSXqcOT+lBVz2jyjfua8DoBdzRsIyaUAT/8=
cosmossdk.io/store v0.0.0-20230227103508-bbe7f8a11b44 h1:/pKsj/ApzO4+zMwpgLiPG5iakoHxziGpMiJcz4S+r4w=
Expand Down
Loading