diff --git a/CHANGELOG.md b/CHANGELOG.md index d1eb4c1e9f4d..9810afe70ffb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i * (testutil/integration) [#22616](https://github.com/cosmos/cosmos-sdk/pull/22616) Remove double context in integration tests v1. * Use `integrationApp.Context()` instead of creating a context prior. * (version) [#22807](https://github.com/cosmos/cosmos-sdk/pull/22807) Return server/v2 information in the `version` functions and commands. +* [#22826](https://github.com/cosmos/cosmos-sdk/pull/22826) Simplify testing frameworks by removing `testutil/cmdtest`. ### Bug Fixes diff --git a/client/keys/add.go b/client/keys/add.go index ee006231ace2..bbee335ba5f6 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -400,7 +400,7 @@ func printCreate(ctx client.Context, cmd *cobra.Command, k *keyring.Record, show return fmt.Errorf("failed to print mnemonic: %w", err) } } else { - if err = printDiscreetly(ctx, cmd.ErrOrStderr(), "**Important** write this mnemonic phrase in a safe place.\nIt is the only way to recover your account if you ever forget your password.", mnemonic); err != nil { + if err = printDiscreetly(cmd.ErrOrStderr(), "**Important** write this mnemonic phrase in a safe place.\nIt is the only way to recover your account if you ever forget your password.", mnemonic); err != nil { return fmt.Errorf("failed to print mnemonic: %w", err) } } diff --git a/client/keys/export.go b/client/keys/export.go index 9abdb684ec11..ef235113ac13 100644 --- a/client/keys/export.go +++ b/client/keys/export.go @@ -89,7 +89,7 @@ func exportUnsafeUnarmored(ctx client.Context, cmd *cobra.Command, uid string, b cmd.Println(hexPrivKey) return nil } - if err = printDiscreetly(ctx, cmd.ErrOrStderr(), "**Important** Do not share this private key.", hexPrivKey); err != nil { + if err = printDiscreetly(cmd.ErrOrStderr(), "**Important** Do not share this private key.", hexPrivKey); err != nil { return fmt.Errorf("failed to print private key: %w", err) } cmd.Println("Export private key successfully") diff --git a/client/keys/mnemonic.go b/client/keys/mnemonic.go index 245e969432a1..b3e8d99ca216 100644 --- a/client/keys/mnemonic.go +++ b/client/keys/mnemonic.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/go-bip39" "github.com/spf13/cobra" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/input" ) @@ -69,7 +68,7 @@ func MnemonicKeyCommand() *cobra.Command { } indiscreet, _ := cmd.Flags().GetBool(flagIndiscreet) if !indiscreet { - return printDiscreetly(client.GetClientContextFromCmd(cmd), cmd.ErrOrStderr(), "**Important** write this mnemonic phrase in a safe place. Do not share it to anyone.", mnemonic) + return printDiscreetly(cmd.ErrOrStderr(), "**Important** write this mnemonic phrase in a safe place. Do not share it to anyone.", mnemonic) } cmd.Println(mnemonic) return nil diff --git a/client/keys/show.go b/client/keys/show.go index 0c418b6ed6f9..9869dd4fecec 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -143,9 +143,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { qrterminal.GenerateHalfBlock(out, qrterminal.H, cmd.OutOrStdout()) } - if _, err := fmt.Fprintln(cmd.OutOrStdout(), out); err != nil { - return err - } + cmd.Println(out) default: if err := printKeyringRecord(cmd.OutOrStdout(), ko, outputFormat); err != nil { return err diff --git a/client/keys/utils.go b/client/keys/utils.go index 295af8750382..cb092a930be2 100644 --- a/client/keys/utils.go +++ b/client/keys/utils.go @@ -74,7 +74,7 @@ func printTextRecords(w io.Writer, kos []KeyOutput) error { } // printDiscreetly Print a secret string to an alternate screen, so the string isn't printed to the terminal. -func printDiscreetly(clientCtx client.Context, w io.Writer, promptMsg, secretMsg string) error { +func printDiscreetly(w io.Writer, promptMsg, secretMsg string) error { output := termenv.NewOutput(w) output.AltScreen() defer output.ExitAltScreen() diff --git a/tests/go.mod b/tests/go.mod index 12f3253b1c3e..f52c7a18e6f9 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -53,7 +53,6 @@ require ( github.com/google/go-cmp v0.6.0 github.com/google/gofuzz v1.2.0 github.com/jhump/protoreflect v1.17.0 - github.com/rs/zerolog v1.33.0 github.com/spf13/viper v1.19.0 gitlab.com/yawning/secp256k1-voi v0.0.0-20230925100816-f2616030848b ) @@ -196,6 +195,7 @@ require ( github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect github.com/rogpeppe/go-internal v1.13.1 // indirect github.com/rs/cors v1.11.1 // indirect + github.com/rs/zerolog v1.33.0 // indirect github.com/sagikazarmark/locafero v0.4.0 // indirect github.com/sagikazarmark/slog-shim v0.1.0 // indirect github.com/sasha-s/go-deadlock v0.3.5 // indirect diff --git a/tests/integration/genutil/export_test.go b/tests/integration/genutil/export_test.go deleted file mode 100644 index e583aa59e970..000000000000 --- a/tests/integration/genutil/export_test.go +++ /dev/null @@ -1,354 +0,0 @@ -package genutil - -import ( - "context" - "encoding/json" - "errors" - "io" - "os" - "path/filepath" - "testing" - "time" - - cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" - cmttypes "github.com/cometbft/cometbft/types" - "github.com/rs/zerolog" - "github.com/spf13/viper" - "github.com/stretchr/testify/require" - - corectx "cosmossdk.io/core/context" - corestore "cosmossdk.io/core/store" - "cosmossdk.io/log" - - "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/server/types" - "github.com/cosmos/cosmos-sdk/testutil/cmdtest" - "github.com/cosmos/cosmos-sdk/types/module" - "github.com/cosmos/cosmos-sdk/x/genutil/client/cli" - genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" -) - -// ExportSystem wraps a (*cmdtest).System -// and sets up appropriate client and server contexts, -// to simplify testing the export CLI. -type ExportSystem struct { - sys *cmdtest.System - - Ctx context.Context - - Viper *viper.Viper - Logger log.Logger - - HomeDir string -} - -// newExportSystem returns a cmdtest.System with export as a child command, -// and it returns a context.Background with an associated *server.Context value. -func NewExportSystem(t *testing.T, exporter types.AppExporter) *ExportSystem { - t.Helper() - - homeDir := t.TempDir() - - // Unclear why we have to create the config directory ourselves, - // but tests fail without this. - if err := os.MkdirAll(filepath.Join(homeDir, "config"), 0o700); err != nil { - t.Fatal(err) - } - - sys := cmdtest.NewSystem() - sys.AddCommands( - cli.ExportCmd(exporter), - cli.InitCmd(module.NewManager()), - ) - - tw := zerolog.NewTestWriter(t) - tw.Frame = 5 // Seems to be the magic number to get source location to match logger calls. - - viper := viper.New() - logger := log.NewCustomLogger(zerolog.New(tw)) - err := writeAndTrackDefaultConfig(viper, homeDir) - if err != nil { - t.Fatal(err) - } - - cCtx := (client.Context{}).WithHomeDir(homeDir) - - ctx := context.WithValue(context.Background(), corectx.ViperContextKey, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) - - ctx = context.WithValue(ctx, client.ClientContextKey, &cCtx) - - return &ExportSystem{ - sys: sys, - Ctx: ctx, - Viper: viper, - Logger: logger, - HomeDir: homeDir, - } -} - -// Run wraps (*cmdtest.System).RunC, providing e's context. -func (s *ExportSystem) Run(args ...string) cmdtest.RunResult { - return s.sys.RunC(s.Ctx, args...) -} - -// MustRun wraps (*cmdtest.System).MustRunC, providing e's context. -func (s *ExportSystem) MustRun(t *testing.T, args ...string) cmdtest.RunResult { - t.Helper() - return s.sys.MustRunC(t, s.Ctx, args...) -} - -// isZeroExportedApp reports whether all fields of a are unset. -// -// This is for the mockExporter to check if a return value was ever set. -func isZeroExportedApp(a types.ExportedApp) bool { - return a.AppState == nil && - len(a.Validators) == 0 && - a.Height == 0 && - a.ConsensusParams == cmtproto.ConsensusParams{} -} - -// mockExporter provides an Export method matching server/types.AppExporter, -// and it tracks relevant arguments when that method is called. -type mockExporter struct { - // The values to return from Export(). - ExportApp types.ExportedApp - Err error - - // Whether Export was called at all. - WasCalled bool - - // Called tracks the interesting arguments passed to Export(). - Called struct { - Height int64 - ForZeroHeight bool - JailAllowedAddrs []string - ModulesToExport []string - } -} - -// SetDefaultExportApp sets a valid ExportedApp to be returned -// when e.Export is called. -func (e *mockExporter) SetDefaultExportApp() { - e.ExportApp = types.ExportedApp{ - ConsensusParams: cmtproto.ConsensusParams{ - Block: &cmtproto.BlockParams{ - MaxBytes: 5 * 1024 * 1024, - MaxGas: -1, - }, - Evidence: &cmtproto.EvidenceParams{ - MaxAgeNumBlocks: 100, - MaxAgeDuration: time.Hour, - MaxBytes: 1024 * 1024, - }, - Validator: &cmtproto.ValidatorParams{ - PubKeyTypes: []string{cmttypes.ABCIPubKeyTypeEd25519}, - }, - }, - } -} - -// Export satisfies the server/types.AppExporter function type. -// -// e tracks relevant arguments under the e.Called struct. -// -// Export panics if neither e.ExportApp nor e.Err have been set. -func (e *mockExporter) Export( - logger log.Logger, - db corestore.KVStoreWithBatch, - traceWriter io.Writer, - height int64, - forZeroHeight bool, - jailAllowedAddrs []string, - opts types.AppOptions, - modulesToExport []string, -) (types.ExportedApp, error) { - if e.Err == nil && isZeroExportedApp(e.ExportApp) { - panic(errors.New("(*mockExporter).Export called without setting e.ExportApp or e.Err")) - } - e.WasCalled = true - - e.Called.Height = height - e.Called.ForZeroHeight = forZeroHeight - e.Called.JailAllowedAddrs = jailAllowedAddrs - e.Called.ModulesToExport = modulesToExport - - return e.ExportApp, e.Err -} - -func TestExportCLI(t *testing.T) { - // Use t.Parallel in all of the subtests, - // because they all read from disk and risk blocking on io. - - t.Run("fail on missing genesis file", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - sys := NewExportSystem(t, e.Export) - - res := sys.Run("export") - require.Error(t, res.Err) - require.Truef(t, os.IsNotExist(res.Err), "expected resulting error to be os.IsNotExist, got %T (%v)", res.Err, res.Err) - - require.False(t, e.WasCalled) - }) - - t.Run("prints to stdout by default", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - e.SetDefaultExportApp() - - sys := NewExportSystem(t, e.Export) - _ = sys.MustRun(t, "init", "some_moniker") - res := sys.MustRun(t, "export") - - require.Empty(t, res.Stderr.String()) - - CheckExportedGenesis(t, res.Stdout.Bytes()) - }) - - t.Run("passes expected default values to the AppExporter", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - e.SetDefaultExportApp() - - sys := NewExportSystem(t, e.Export) - _ = sys.MustRun(t, "init", "some_moniker") - _ = sys.MustRun(t, "export") - - require.True(t, e.WasCalled) - - require.Equal(t, int64(-1), e.Called.Height) - require.False(t, e.Called.ForZeroHeight) - require.Empty(t, e.Called.JailAllowedAddrs) - require.Empty(t, e.Called.ModulesToExport) - }) - - t.Run("passes flag values to the AppExporter", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - e.SetDefaultExportApp() - - sys := NewExportSystem(t, e.Export) - _ = sys.MustRun(t, "init", "some_moniker") - _ = sys.MustRun(t, "export", - "--height=100", - "--jail-allowed-addrs", "addr1,addr2", - "--modules-to-export", "foo,bar", - ) - - require.True(t, e.WasCalled) - - require.Equal(t, int64(100), e.Called.Height) - require.False(t, e.Called.ForZeroHeight) - require.Equal(t, []string{"addr1", "addr2"}, e.Called.JailAllowedAddrs) - require.Equal(t, []string{"foo", "bar"}, e.Called.ModulesToExport) - }) - - t.Run("passes --for-zero-height to the AppExporter", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - e.SetDefaultExportApp() - - sys := NewExportSystem(t, e.Export) - _ = sys.MustRun(t, "init", "some_moniker") - _ = sys.MustRun(t, "export", "--for-zero-height") - - require.True(t, e.WasCalled) - - require.Equal(t, int64(-1), e.Called.Height) - require.True(t, e.Called.ForZeroHeight) - require.Empty(t, e.Called.JailAllowedAddrs) - require.Empty(t, e.Called.ModulesToExport) - }) - - t.Run("prints to a given file with --output-document", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - e.SetDefaultExportApp() - - sys := NewExportSystem(t, e.Export) - _ = sys.MustRun(t, "init", "some_moniker") - - outDir := t.TempDir() - outFile := filepath.Join(outDir, "export.json") - - res := sys.MustRun(t, "export", "--output-document", outFile) - - require.Empty(t, res.Stderr.String()) - require.Empty(t, res.Stdout.String()) - - j, err := os.ReadFile(outFile) - require.NoError(t, err) - - CheckExportedGenesis(t, j) - }) - - t.Run("prints genesis to stdout when no app exporter defined", func(t *testing.T) { - t.Parallel() - - sys := NewExportSystem(t, nil) - _ = sys.MustRun(t, "init", "some_moniker") - - res := sys.MustRun(t, "export") - - require.Contains(t, res.Stderr.String(), "WARNING: App exporter not defined.") - - origGenesis, err := os.ReadFile(filepath.Join(sys.HomeDir, "config", "genesis.json")) - require.NoError(t, err) - - out := res.Stdout.Bytes() - - require.Equal(t, origGenesis, out) - }) - - t.Run("returns app exporter error", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - e.Err = errors.New("whoopsie") - - sys := NewExportSystem(t, e.Export) - _ = sys.MustRun(t, "init", "some_moniker") - - res := sys.Run("export") - - require.ErrorIs(t, res.Err, e.Err) - }) - - t.Run("rejects positional arguments", func(t *testing.T) { - t.Parallel() - - e := new(mockExporter) - e.SetDefaultExportApp() - - sys := NewExportSystem(t, e.Export) - _ = sys.MustRun(t, "init", "some_moniker") - - outDir := t.TempDir() - outFile := filepath.Join(outDir, "export.json") - - res := sys.Run("export", outFile) - require.Error(t, res.Err) - - require.NoFileExists(t, outFile) - }) -} - -// CheckExportedGenesis fails t if j cannot be unmarshaled into a valid AppGenesis. -func CheckExportedGenesis(t *testing.T, j []byte) { - t.Helper() - - var ag genutiltypes.AppGenesis - require.NoError(t, json.Unmarshal(j, &ag)) - - require.NotEmpty(t, ag.AppName) - require.NotZero(t, ag.GenesisTime) - require.NotEmpty(t, ag.ChainID) - require.NotNil(t, ag.Consensus) -} diff --git a/testutil/cmdtest/system.go b/testutil/cmdtest/system.go deleted file mode 100644 index 1b02b0a365a3..000000000000 --- a/testutil/cmdtest/system.go +++ /dev/null @@ -1,120 +0,0 @@ -// Package cmdtest contains a framework for testing cobra Commands within Go unit tests. -package cmdtest - -import ( - "bytes" - "context" - "io" - - "github.com/spf13/cobra" -) - -// System is a system under test. -type System struct { - commands []*cobra.Command -} - -// NewSystem returns a new System. -func NewSystem() *System { - // We aren't doing any special initialization yet, - // but let's encourage a constructor to make it simpler - // to update later, if needed. - return new(System) -} - -// AddCommands sets commands to be available to the Run family of methods on s. -func (s *System) AddCommands(cmds ...*cobra.Command) { - s.commands = append(s.commands, cmds...) -} - -// RunResult is the stdout and stderr resulting from a call to a System's Run family of methods, -// and any error that was returned. -type RunResult struct { - Stdout, Stderr bytes.Buffer - - Err error -} - -// Run calls s.RunC with context.Background(). -func (s *System) Run(args ...string) RunResult { - return s.RunC(context.Background(), args...) -} - -// RunC calls s.RunWithInput with an empty stdin. -func (s *System) RunC(ctx context.Context, args ...string) RunResult { - return s.RunWithInputC(ctx, bytes.NewReader(nil), args...) -} - -// RunWithInput calls s.RunWithInputC with context.Background(). -func (s *System) RunWithInput(in io.Reader, args ...string) RunResult { - return s.RunWithInputC(context.Background(), in, args...) -} - -// RunWithInputC executes a new root command with subcommands -// that were set in s.AddCommands(). -// The command's stdin is set to the in argument. -// RunWithInputC returns a RunResult wrapping stdout, stderr, and any returned error. -func (s *System) RunWithInputC(ctx context.Context, in io.Reader, args ...string) RunResult { - rootCmd := &cobra.Command{} - rootCmd.AddCommand(s.commands...) - - rootCmd.SetIn(in) - - var res RunResult - rootCmd.SetOutput(&res.Stdout) - rootCmd.SetErr(&res.Stderr) - - rootCmd.SetArgs(args) - - res.Err = rootCmd.ExecuteContext(ctx) - return res -} - -// MustRun calls s.Run, but also calls t.FailNow if RunResult.Err is not nil. -func (s *System) MustRun(t TestingT, args ...string) RunResult { - t.Helper() - - return s.MustRunC(t, context.Background(), args...) -} - -// MustRunC calls s.RunWithInput, but also calls t.FailNow if RunResult.Err is not nil. -func (s *System) MustRunC(t TestingT, ctx context.Context, args ...string) RunResult { - t.Helper() - - return s.MustRunWithInputC(t, ctx, bytes.NewReader(nil), args...) -} - -// MustRunWithInput calls s.RunWithInput, but also calls t.FailNow if RunResult.Err is not nil. -func (s *System) MustRunWithInput(t TestingT, in io.Reader, args ...string) RunResult { - t.Helper() - - return s.MustRunWithInputC(t, context.Background(), in, args...) -} - -// MustRunWithInputC calls s.RunWithInputC, but also calls t.FailNow if RunResult.Err is not nil. -func (s *System) MustRunWithInputC(t TestingT, ctx context.Context, in io.Reader, args ...string) RunResult { - t.Helper() - - res := s.RunWithInputC(ctx, in, args...) - if res.Err != nil { - t.Logf("Error executing %v: %v", args, res.Err) - t.Logf("Stdout: %q", res.Stdout.String()) - t.Logf("Stderr: %q", res.Stderr.String()) - t.FailNow() - } - - return res -} - -// TestingT is a subset of testing.TB, -// containing only what the (*System).Must methods use. -// -// This simplifies using other testing wrappers, -// such as testify suite, etc. -type TestingT interface { - Helper() - - Logf(format string, args ...any) - - FailNow() -} diff --git a/version/command.go b/version/command.go index d52a69668685..9514548f7a8a 100644 --- a/version/command.go +++ b/version/command.go @@ -2,7 +2,6 @@ package version import ( "encoding/json" - "fmt" "strings" "github.com/spf13/cobra" @@ -29,7 +28,7 @@ func NewVersionCommand() *cobra.Command { verInfo := NewInfo() if long, _ := cmd.Flags().GetBool(flagLong); !long { - fmt.Fprintln(cmd.OutOrStdout(), verInfo.Version) + cmd.Println(verInfo.Version) return nil } @@ -54,7 +53,7 @@ func NewVersionCommand() *cobra.Command { return err } - fmt.Fprintln(cmd.OutOrStdout(), string(bz)) + cmd.Println(string(bz)) return nil }, } diff --git a/version/version_test.go b/version/version_test.go index 48b4428bda0d..db32c6267fe8 100644 --- a/version/version_test.go +++ b/version/version_test.go @@ -5,14 +5,12 @@ import ( "encoding/json" "fmt" "runtime" - "strings" "testing" "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/testutil" - "github.com/cosmos/cosmos-sdk/testutil/cmdtest" "github.com/cosmos/cosmos-sdk/version" ) @@ -42,103 +40,6 @@ go version go1.14 linux/amd64` require.Equal(t, want, info.String()) } -func TestCLI(t *testing.T) { - setVersionPackageVars(t) - - sys := cmdtest.NewSystem() - sys.AddCommands(version.NewVersionCommand()) - - t.Run("no flags", func(t *testing.T) { - res := sys.MustRun(t, "version") - - // Only prints the version, with a newline, to stdout. - require.Equal(t, testVersion+"\n", res.Stdout.String()) - require.Empty(t, res.Stderr.String()) - }) - - t.Run("--long flag", func(t *testing.T) { - res := sys.MustRun(t, "version", "--long") - - out := res.Stdout.String() - lines := strings.Split(out, "\n") - require.Contains(t, lines, "name: testchain-app") - require.Contains(t, lines, "server_name: testchaind") - require.Contains(t, lines, `version: "3.14"`) - require.Contains(t, lines, "commit: abc123") - require.Contains(t, lines, "build_tags: mybuildtag") - - require.Empty(t, res.Stderr.String()) - }) - - t.Run("--output=json flag", func(t *testing.T) { - res := sys.MustRun(t, "version", "--output=json") - - var info version.Info - require.NoError(t, json.Unmarshal(res.Stdout.Bytes(), &info)) - - // Assert against a couple fields that are difficult to predict in test - // without copying and pasting code. - require.NotEmpty(t, info.GoVersion) - - // The SDK version appears to not be set during this test, so we'll ignore it here. - - // Now clear out the non-empty fields, so we can compare against a fixed value. - info.GoVersion = "" - - want := version.Info{ - Name: testName, - AppName: testAppName, - Version: testVersion, - GitCommit: testCommit, - BuildTags: testBuildTags, - } - require.Equal(t, want, info) - - require.Empty(t, res.Stderr.String()) - }) - - t.Run("positional args rejected", func(t *testing.T) { - res := sys.Run("version", "foo") - require.Error(t, res.Err) - }) -} - -const ( - testName = "testchain-app" - testAppName = "testchaind" - testVersion = "3.14" - testCommit = "abc123" - testBuildTags = "mybuildtag" -) - -// setVersionPackageVars temporarily overrides the package variables in the version package -// so that we can assert meaningful output. -func setVersionPackageVars(t *testing.T) { - t.Helper() - - var ( - origName = version.Name - origAppName = version.AppName - origVersion = version.Version - origCommit = version.Commit - origBuildTags = version.BuildTags - ) - - t.Cleanup(func() { - version.Name = origName - version.AppName = origAppName - version.Version = origVersion - version.Commit = origCommit - version.BuildTags = origBuildTags - }) - - version.Name = testName - version.AppName = testAppName - version.Version = testVersion - version.Commit = testCommit - version.BuildTags = testBuildTags -} - func Test_runVersionCmd(t *testing.T) { cmd := version.NewVersionCommand() _, mockOut := testutil.ApplyMockIO(cmd)