From 658f2f38e465e6b20e1d34d9d49ba3f803f294a8 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Tue, 19 May 2026 09:05:44 +0000 Subject: [PATCH 01/11] DAOS-18304 ddb: add Go unit tests using build-tag CGo stubs Introduce the Go test suite for the ddb CLI layer, built on top of the build-tag CGo stub infrastructure landed in #18124: - Add test_helpers.go: newTestContext(t) resets all CGo stubs via resetDdbStubs() and returns a *DdbContext ready for use in tests. Test cases set per-function _Fn hook variables directly. - All test files carry the //go:build test_stubs tag so they only compile when the stub infrastructure is present. - TestCmds: open (default, write_mode, db_path), feature (show, enable, disable), and dtx_aggr (mutual exclusion, cmt_time, cmt_date, path). Adds skipCmdLine field for flags shared between CLI and grumble layers. - TestHelpCmds: unknown-command help flow. - TestParseOpts / TestRun: CLI-level option parsing and run() dispatch, including unknown-command detection for both command-line and command-file paths. - TestNewLogger: 6 sub-cases (default level, explicit debug, invalid level, valid LogDir, non-existent LogDir, LogDir is a file). - TestClosePoolIfOpen: Close not called when already closed, called when open, Close error tolerated. Test-tag: unittest Required-githooks: yes Signed-off-by: Cedric Koch-Hofer --- .../cmd/ddb/command_completers_test.go | 23 +- src/control/cmd/ddb/ddb_commands_test.go | 470 ++++++++++++++ src/control/cmd/ddb/main_test.go | 575 ++++++++++++++++++ src/control/cmd/ddb/test_helpers.go | 122 ++++ 4 files changed, 1181 insertions(+), 9 deletions(-) create mode 100644 src/control/cmd/ddb/ddb_commands_test.go create mode 100644 src/control/cmd/ddb/main_test.go create mode 100644 src/control/cmd/ddb/test_helpers.go diff --git a/src/control/cmd/ddb/command_completers_test.go b/src/control/cmd/ddb/command_completers_test.go index 834fbfb048a..5594fae469d 100644 --- a/src/control/cmd/ddb/command_completers_test.go +++ b/src/control/cmd/ddb/command_completers_test.go @@ -1,3 +1,9 @@ +// +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP +// +// SPDX-License-Identifier: BSD-2-Clause-Patent +// + package main import ( @@ -18,7 +24,7 @@ func createFile(t *testing.T, filePath string) { fd, err := os.Create(filePath) if err != nil { - t.Fatalf("Failed to create test vos file %s: %v", filePath, err) + t.Fatalf("failed to create test vos file %s: %v", filePath, err) } fd.Close() } @@ -27,14 +33,14 @@ func createDirAll(t *testing.T, dirPath string) { t.Helper() if err := os.MkdirAll(dirPath, 0755); err != nil { - t.Fatalf("Failed to create test pool directory %s: %v", dirPath, err) + t.Fatalf("failed to create test pool directory %s: %v", dirPath, err) } } -func testSetup(t *testing.T) (tmpDir string, teardown func()) { +func testSetup(t *testing.T) string { t.Helper() - tmpDir, teardown = test.CreateTestDir(t) + tmpDir := t.TempDir() for _, dir := range testPoolDirs { createDirAll(t, filepath.Join(tmpDir, dir)) @@ -51,12 +57,11 @@ func testSetup(t *testing.T) (tmpDir string, teardown func()) { createDirAll(t, filepath.Join(tmpDir, "bar", "baz")) createFile(t, filepath.Join(tmpDir, "bar", "baz", "no_vos")) - return + return tmpDir } func TestListVosFiles(t *testing.T) { - tmpDir, teardown := testSetup(t) - t.Cleanup(teardown) + tmpDir := testSetup(t) for name, tc := range map[string]struct { args string @@ -118,7 +123,7 @@ func TestListVosFiles(t *testing.T) { } { t.Run(name, func(t *testing.T) { results := listVosFiles(tc.args) - test.AssertStringsEqual(t, tc.expRes, results, "listDirVos results do not match expected") + test.AssertStringsEqual(t, tc.expRes, results, "unexpected listVosFiles results") }) } } @@ -169,7 +174,7 @@ func TestFilterSuggestions(t *testing.T) { } { t.Run(name, func(t *testing.T) { results := filterSuggestions(tc.prefix, initialSuggestions, additionalSuggestions) - test.AssertStringsEqual(t, tc.expRes, results, "filterSuggestions results do not match expected") + test.AssertStringsEqual(t, tc.expRes, results, "unexpected filterSuggestions results") }) } } diff --git a/src/control/cmd/ddb/ddb_commands_test.go b/src/control/cmd/ddb/ddb_commands_test.go new file mode 100644 index 00000000000..1571f9857d3 --- /dev/null +++ b/src/control/cmd/ddb/ddb_commands_test.go @@ -0,0 +1,470 @@ +// +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP +// +// SPDX-License-Identifier: BSD-2-Clause-Patent +// + +//go:build test_stubs + +package main + +import ( + "fmt" + "os" + "path" + "path/filepath" + "strings" + "testing" + + "github.com/daos-stack/daos/src/control/common/test" +) + +func runHelpCmd(t *testing.T, cmdStr string, helpSubStr string) { + t.Helper() + + ctx := newTestContext(t) + + // Create a temporary config file with the help command + tmpCfgDir := t.TempDir() + tmpCfgFile := path.Join(tmpCfgDir, "ddb-cmd_file.txt") + if err := os.WriteFile(tmpCfgFile, []byte(fmt.Sprintf("%s --help", cmdStr)), 0644); err != nil { + t.Fatalf("failed to write temp config file: %v", err) + } + + // Run the help command with a command file + args := test.JoinArgs(nil, "--cmd_file="+tmpCfgFile) + stdoutCmdFile, err := runMainFlow(ctx, args) + if err != nil { + t.Fatalf("unexpected error when running '%s --help' via command file: want nil, got %v", cmdStr, err) + } + test.AssertTrue(t, strings.Contains(stdoutCmdFile, helpSubStr), + fmt.Sprintf("expected stdout to contain %q: got\n%s", helpSubStr, stdoutCmdFile)) + + // Run the help command with a command line + args = test.JoinArgs(nil, cmdStr, "--help") + stdoutCmdLine, err := runMainFlow(ctx, args) + if err != nil { + t.Fatalf("unexpected error when running '%s --help' via command line: want nil, got %v", cmdStr, err) + } + test.AssertTrue(t, strings.Contains(stdoutCmdLine, helpSubStr), + fmt.Sprintf("expected stdout to contain %q: got\n%s", helpSubStr, stdoutCmdLine)) + + // Compare command line and command file outputs + test.AssertEqual(t, stdoutCmdFile, stdoutCmdLine, + fmt.Sprintf("unexpected help output mismatch between command file and command line for '%s'", cmdStr)) +} + +func TestHelpCmds(t *testing.T) { + for name, tc := range map[string]struct { + cmdStr string + helpSubStr string + }{ + "help for 'ls' command": { + cmdStr: "ls", + helpSubStr: "Usage:\n ls [flags] [path]\n", + }, + "help for 'open' command": { + cmdStr: "open", + helpSubStr: "Usage:\n open [flags] path\n", + }, + // TODO(follow-up PR): Add help tests for the remaining commands. + // Use runHelpCmd(t, "", "Usage:\n ") following the same pattern. + } { + t.Run(name, func(t *testing.T) { + runHelpCmd(t, tc.cmdStr, tc.helpSubStr) + }) + } +} + +func TestCmds(t *testing.T) { + for name, tc := range map[string]struct { + args []string + setup func() + expStdout []string + expErr error + // skipCmdLine skips the command-line sub-test with a message. Use when + // a flag is shared between the CLI layer and the grumble command: go-flags + // consumes it before grumble can see it, making a clean command-line test + // impossible for that particular flag. + skipCmdLine string + }{ + "ls invalid options": { + args: []string{"ls", "--bar"}, + expErr: ddbTestErr("invalid flag: --bar"), + }, + "ls default": { + args: []string{"ls"}, + setup: func() { + ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { + fmt.Println("ls called") + if err := isArgEqual("", path, "path"); err != nil { + return err + } + if err := isArgEqual(false, recursive, "recursive"); err != nil { + return err + } + if err := isArgEqual(false, details, "details"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"ls called"}, + }, + "ls path": { + args: []string{"ls", "/[0]"}, + setup: func() { + ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { + fmt.Println("ls called") + if err := isArgEqual("/[0]", path, "path"); err != nil { + return err + } + if err := isArgEqual(false, recursive, "recursive"); err != nil { + return err + } + if err := isArgEqual(false, details, "details"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"ls called"}, + }, + "ls long recursive opt": { + args: []string{"ls", "--recursive"}, + setup: func() { + ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { + fmt.Println("ls called") + if err := isArgEqual("", path, "path"); err != nil { + return err + } + if err := isArgEqual(true, recursive, "recursive"); err != nil { + return err + } + if err := isArgEqual(false, details, "details"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"ls called"}, + }, + "ls short details opt": { + args: []string{"ls", "-d"}, + setup: func() { + ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { + fmt.Println("ls called") + if err := isArgEqual("", path, "path"); err != nil { + return err + } + if err := isArgEqual(false, recursive, "recursive"); err != nil { + return err + } + if err := isArgEqual(true, details, "details"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"ls called"}, + }, + "ls details long opt": { + args: []string{"ls", "--details"}, + setup: func() { + ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { + fmt.Println("ls called") + if err := isArgEqual("", path, "path"); err != nil { + return err + } + if err := isArgEqual(false, recursive, "recursive"); err != nil { + return err + } + if err := isArgEqual(true, details, "details"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"ls called"}, + }, + + // --- open command --- + // Note: the -w/--write_mode and -p/--db_path flags of the grumble 'open' + // command share names with CLI-level flags that are consumed by go-flags + // before reaching grumble in command-line mode. The command-line test for + // those flags would silently test wrong values. They are correctly exercised + // in command-file mode; see TestRun for CLI-level flag coverage. + "open default": { + args: []string{"open", "/path/to/vos-0"}, + setup: func() { + ddb_run_open_Fn = func(path, dbPath string, writeMode bool) error { + fmt.Println("open called") + if err := isArgEqual("/path/to/vos-0", path, "path"); err != nil { + return err + } + if err := isArgEqual("", dbPath, "db_path"); err != nil { + return err + } + if err := isArgEqual(false, writeMode, "write_mode"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"open called"}, + }, + "open write mode": { + args: []string{"open", "-w", "/path/to/vos-0"}, + skipCmdLine: "-w is consumed by the CLI write_mode flag before reaching grumble", + setup: func() { + ddb_run_open_Fn = func(path, dbPath string, writeMode bool) error { + fmt.Println("open called") + if err := isArgEqual(true, writeMode, "write_mode"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"open called"}, + }, + "open with db path": { + args: []string{"open", "-p", "/sysdb", "/path/to/vos-0"}, + skipCmdLine: "-p is consumed by the CLI db_path flag before reaching grumble", + setup: func() { + ddb_run_open_Fn = func(path, dbPath string, writeMode bool) error { + fmt.Println("open called") + if err := isArgEqual("/path/to/vos-0", path, "path"); err != nil { + return err + } + if err := isArgEqual("/sysdb", dbPath, "db_path"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"open called"}, + }, + + // --- feature command --- + // feature --show: verifies the show flag is forwarded to the C layer. + "feature show": { + args: []string{"feature", "--show"}, + setup: func() { + ddb_run_feature_Fn = func(path, dbPath, enable, disable string, show bool) error { + fmt.Println("feature called") + if err := isArgEqual(true, show, "show"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"feature called"}, + }, + // feature --enable: verifies that the enable string reaches ddb_feature_string2flags. + "feature enable": { + args: []string{"feature", "--enable=myflag"}, + setup: func() { + var capturedFlag string + ddb_feature_string2flags_Fn = func(s string) (uint64, uint64, error) { + capturedFlag = s + return 0, 0, nil + } + ddb_run_feature_Fn = func(path, dbPath, enable, disable string, show bool) error { + fmt.Println("feature called") + if err := isArgEqual("myflag", capturedFlag, "enable flag string"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"feature called"}, + }, + // feature --disable: verifies that the disable string reaches ddb_feature_string2flags. + "feature disable": { + args: []string{"feature", "--disable=otherflag"}, + setup: func() { + var capturedFlag string + ddb_feature_string2flags_Fn = func(s string) (uint64, uint64, error) { + capturedFlag = s + return 0, 0, nil + } + ddb_run_feature_Fn = func(path, dbPath, enable, disable string, show bool) error { + fmt.Println("feature called") + if err := isArgEqual("otherflag", capturedFlag, "disable flag string"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"feature called"}, + }, + + // --- dtx_aggr command --- + // The Run handler in ddb_commands.go enforces that exactly one of --cmt_time or + // --cmt_date is provided. These tests exercise that Go-layer validation. + "dtx_aggr both cmt_time and cmt_date": { + args: []string{"dtx_aggr", "--cmt_time=0", "--cmt_date=2024-01-01"}, + expErr: ddbTestErr("mutually exclusive"), + }, + "dtx_aggr neither cmt_time nor cmt_date": { + args: []string{"dtx_aggr"}, + expErr: ddbTestErr("has to be defined"), + }, + "dtx_aggr cmt_time": { + args: []string{"dtx_aggr", "--cmt_time=1000"}, + setup: func() { + ddb_run_dtx_aggr_Fn = func(path string, cmtTime uint64, cmtDate string) error { + fmt.Println("dtx_aggr called") + if err := isArgEqual(uint64(1000), cmtTime, "cmtTime"); err != nil { + return err + } + if err := isArgEqual("", cmtDate, "cmtDate"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"dtx_aggr called"}, + }, + "dtx_aggr cmt_date": { + args: []string{"dtx_aggr", "--cmt_date=2024-01-01"}, + setup: func() { + ddb_run_dtx_aggr_Fn = func(path string, cmtTime uint64, cmtDate string) error { + fmt.Println("dtx_aggr called") + if err := isArgEqual("2024-01-01", cmtDate, "cmtDate"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"dtx_aggr called"}, + }, + "dtx_aggr with path": { + args: []string{"dtx_aggr", "--cmt_time=0", "[0]"}, + setup: func() { + ddb_run_dtx_aggr_Fn = func(path string, cmtTime uint64, cmtDate string) error { + fmt.Println("dtx_aggr called") + if err := isArgEqual("[0]", path, "path"); err != nil { + return err + } + if err := isArgEqual(uint64(0), cmtTime, "cmtTime"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"dtx_aggr called"}, + }, + + // --- close command --- + "close": { + args: []string{"close"}, + setup: func() { + ddb_run_close_Fn = func() error { + fmt.Println("close called") + return nil + } + }, + expStdout: []string{"close called"}, + }, + + // --- version command --- + "version": { + args: []string{"version"}, + setup: func() { + ddb_run_version_Fn = func() error { + fmt.Println("version called") + return nil + } + }, + expStdout: []string{"version called"}, + }, + + // TODO(follow-up PR): Add TestCmds cases for the remaining commands. + // Each new test case follows the same pattern as the cases above: set the + // corresponding ddb_run__Fn hook in setup() to verify argument passing, + // then add the case to this table. + // Commands still to be covered: superblock_dump, value_dump, rm, + // value_load, ilog_dump, ilog_commit, ilog_clear, dtx_dump, dtx_cmt_clear, + // smd_sync, vea_dump, vea_update, dtx_act_commit, dtx_act_abort, rm_pool, + // dtx_act_discard_invalid, dev_list, dev_replace, dtx_stat, prov_mem. + } { + t.Run(name, func(t *testing.T) { + checkCmd := func(t *testing.T, stdout string, err error) { + t.Helper() + test.CmpErr(t, tc.expErr, err) + if tc.expErr != nil { + return + } + for _, msg := range tc.expStdout { + test.AssertTrue(t, strings.Contains(stdout, msg), + fmt.Sprintf("expected stdout to contain %q: got\n%s", msg, stdout)) + } + } + + t.Run("command-line", func(t *testing.T) { + if tc.skipCmdLine != "" { + t.Skipf("skipping command-line mode: %s", tc.skipCmdLine) + } + ctx := newTestContext(t) + if tc.setup != nil { + tc.setup() + } + stdout, err := runMainFlow(ctx, tc.args) + checkCmd(t, stdout, err) + }) + + t.Run("command-file", func(t *testing.T) { + tmpDir := t.TempDir() + cmdFile := filepath.Join(tmpDir, "cmds.txt") + cmdLine := strings.Join(tc.args, " ") + if err := os.WriteFile(cmdFile, []byte(cmdLine), 0644); err != nil { + t.Fatalf("failed to write command file: %v", err) + } + ctx := newTestContext(t) + if tc.setup != nil { + tc.setup() + } + stdout, err := runMainFlow(ctx, []string{"--cmd_file=" + cmdFile}) + checkCmd(t, stdout, err) + }) + }) + } +} + +func TestManPage(t *testing.T) { + // Expected sections and commands present in every man page rendering. + expSections := []string{ + manArgsHeader, + manCmdsHeader, + manPathSection[:20], + manMdOnSsdSection[:20], + manLoggingSection[:20], + ".B ls\n", + ".B open\n", + } + + // manpage to stdout: must contain all section headers and known commands. + ctx := newTestContext(t) + stdout, err := runMainFlow(ctx, []string{"manpage"}) + test.CmpErr(t, nil, err) + assertContainsAll(t, stdout, expSections) + + // --output flag: man page is written to a file, stdout is empty. + tmpDir := t.TempDir() + outFile := filepath.Join(tmpDir, "ddb.groff") + + ctx = newTestContext(t) + stdout, err = runMainFlow(ctx, []string{"manpage", "--output=" + outFile}) + if err != nil { + t.Fatalf("unexpected error when running 'manpage --output': want nil, got %v", err) + } + test.AssertTrue(t, stdout == "", + fmt.Sprintf("expected empty stdout when --output is set: got\n%s", stdout)) + + content, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("failed to read output file: %v", err) + } + assertContainsAll(t, string(content), expSections) +} diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go new file mode 100644 index 00000000000..ceca06a104d --- /dev/null +++ b/src/control/cmd/ddb/main_test.go @@ -0,0 +1,575 @@ +// +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP +// +// SPDX-License-Identifier: BSD-2-Clause-Patent +// + +//go:build test_stubs + +package main + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/daos-stack/daos/src/control/common/test" + "github.com/daos-stack/daos/src/control/logging" + "github.com/daos-stack/daos/src/control/server/engine" +) + +func TestParseOpts(t *testing.T) { + for name, tc := range map[string]struct { + args []string + checkFunc func(opts *cliOptions) error + expStdout []string + expErr error + }{ + "General help message": { + args: []string{"--help"}, + expStdout: []string{ + "Usage:\n ddb [OPTIONS] [ddb_command] [ddb_command_args...]\n", + "VOS Paths:\n", + "Available Commands:\n", + }, + }, + "General help message with opt": { + args: []string{"-w", "--help"}, + expStdout: []string{ + "Usage:\n ddb [OPTIONS] [ddb_command] [ddb_command_args...]\n", + "VOS Paths:\n", + "Available Commands:\n", + }, + }, + "Unknown commands with help": { + args: []string{"foo", "--help"}, + expErr: errUnknownCmd, + }, + "Unknown commands with help and opt": { + args: []string{"-w", "foo", "--help"}, + expErr: errUnknownCmd, + }, + "Default option values": { + args: []string{"ls", "-d", "-r"}, + checkFunc: func(opts *cliOptions) error { + if opts.Debug != "" { + return fmt.Errorf("expected Debug to be empty, got %q", opts.Debug) + } + if opts.WriteMode { + return fmt.Errorf("expected WriteMode to be false") + } + if opts.CmdFile != "" { + return fmt.Errorf("expected CmdFile to be empty") + } + if opts.SysdbPath != "" { + return fmt.Errorf("expected SysdbPath to be empty") + } + if opts.VosPath != "" { + return fmt.Errorf("expected VosPath to be empty") + } + if opts.Args.RunCmd != "ls" { + return fmt.Errorf("expected RunCmd to be 'ls', got %q", opts.Args.RunCmd) + } + if opts.Args.RunCmdArgs[0] != "-d" { + return fmt.Errorf("expected first RunCmdArgs to be '-d', got %q", opts.Args.RunCmdArgs[0]) + } + if opts.Args.RunCmdArgs[1] != "-r" { + return fmt.Errorf("expected second RunCmdArgs to be '-r', got %q", opts.Args.RunCmdArgs[1]) + } + return nil + }, + }, + "Short miss vos path error": { + args: []string{"-p", "/foo", "ls"}, + expErr: ddbTestErr(vosPathMissErr), + }, + "Long miss vos path error": { + args: []string{"--db_path=/bar", "ls"}, + expErr: ddbTestErr(vosPathMissErr), + }, + "Short cmd args error": { + args: []string{"-f", "/foo/bar.cmd", "ls"}, + expErr: ddbTestErr(runCmdArgsErr), + }, + "Long cmd args error": { + args: []string{"--cmd_file=/foo/bar.cmd", "ls"}, + expErr: ddbTestErr(runCmdArgsErr), + }, + "Short vos path miss error": { + args: []string{"-p", "/foo"}, + expErr: ddbTestErr(vosPathMissErr), + }, + "Long vos path miss error": { + args: []string{"--db_path=/foo"}, + expErr: ddbTestErr(vosPathMissErr), + }, + "Long debug option": { + args: []string{"--debug=DEBUG", "ls"}, + checkFunc: func(opts *cliOptions) error { + if opts.Debug != "DEBUG" { + return fmt.Errorf("expected Debug to be 'DEBUG', got %q", opts.Debug) + } + return nil + }, + }, + "Short write option": { + args: []string{"-w", "ls"}, + checkFunc: func(opts *cliOptions) error { + if !opts.WriteMode { + return fmt.Errorf("expected WriteMode to be true") + } + return nil + }, + }, + "Long write option": { + args: []string{"--write_mode", "ls"}, + checkFunc: func(opts *cliOptions) error { + if !opts.WriteMode { + return fmt.Errorf("expected WriteMode to be true") + } + return nil + }, + }, + "Short vos path option": { + args: []string{"-s", "/foo/vos-0", "ls"}, + checkFunc: func(opts *cliOptions) error { + if opts.VosPath != "/foo/vos-0" { + return fmt.Errorf("expected VosPath to be '/foo/vos-0', got %q", opts.VosPath) + } + return nil + }, + }, + "Long vos path option": { + args: []string{"--vos_path=/foo/vos-0", "ls"}, + checkFunc: func(opts *cliOptions) error { + if opts.VosPath != "/foo/vos-0" { + return fmt.Errorf("expected VosPath to be '/foo/vos-0', got %q", opts.VosPath) + } + return nil + }, + }, + "Short db path option": { + args: []string{"-s", "/foo/vos-0", "-p", "/bar", "ls"}, + checkFunc: func(opts *cliOptions) error { + if opts.VosPath != "/foo/vos-0" { + return fmt.Errorf("expected VosPath to be '/foo/vos-0', got %q", opts.VosPath) + } + if opts.SysdbPath != "/bar" { + return fmt.Errorf("expected SysdbPath to be '/bar', got %q", opts.SysdbPath) + } + return nil + }, + }, + "Long db path option": { + args: []string{"--vos_path=/foo/vos-0", "--db_path=/bar", "ls"}, + checkFunc: func(opts *cliOptions) error { + if opts.VosPath != "/foo/vos-0" { + return fmt.Errorf("expected VosPath to be '/foo/vos-0', got %q", opts.VosPath) + } + if opts.SysdbPath != "/bar" { + return fmt.Errorf("expected SysdbPath to be '/bar', got %q", opts.SysdbPath) + } + return nil + }, + }, + "Short version option": { + args: []string{"-v"}, + checkFunc: func(opts *cliOptions) error { + if !opts.Version { + return fmt.Errorf("expected Version to be true") + } + return nil + }, + }, + "Long version option": { + args: []string{"--version"}, + checkFunc: func(opts *cliOptions) error { + if !opts.Version { + return fmt.Errorf("expected Version to be true") + } + return nil + }, + }, + } { + t.Run(name, func(t *testing.T) { + ctx := newTestContext(t) + + opts, stdout, err := runCmdToStdout(ctx, tc.args) + test.CmpErr(t, tc.expErr, err) + if tc.expErr != nil { + return + } + + for _, msg := range tc.expStdout { + test.AssertTrue(t, strings.Contains(stdout, msg), + fmt.Sprintf("expected stdout to contain %q: got\n%s", msg, stdout)) + } + + if tc.checkFunc != nil { + if err := tc.checkFunc(&opts); err != nil { + t.Fatal(err) + } + } + }) + } +} + +// TestRun covers the non-interactive execution paths of run() (command-line and +// command-file modes). Interactive mode is intentionally not tested: it delegates +// entirely to grumble's app.Run(), which requires a real terminal (readline) and +// piping os.Stdin — making tests fragile and hard to maintain for little gain. +func TestRun(t *testing.T) { + for name, tc := range map[string]struct { + args []string + setup func() + expStdout []string + expErr error + // When cmdFileCmd is non-empty the test is also run in command-file mode. + // cmdFileArgs holds the CLI flags (everything except the positional command), + // and cmdFileCmd is the line written to the temporary command file. + // Note: "no auto-open" cases intentionally omit cmdFileCmd because in + // command-file mode opts.Args.RunCmd is always empty, so noAutoOpen is + // never triggered and the CLI would pre-open the pool. + cmdFileArgs []string + cmdFileCmd string + }{ + "Version output": { + args: []string{"-v"}, + expStdout: []string{"ddb version"}, + }, + "Long version output": { + args: []string{"--version"}, + expStdout: []string{"ddb version"}, + }, + "Unknown command": { + args: []string{"foo"}, + expErr: errUnknownCmd, + cmdFileArgs: []string{}, + cmdFileCmd: "foo", + }, + "Unknown command with write option": { + args: []string{"-w", "foo"}, + expErr: errUnknownCmd, + cmdFileArgs: []string{"-w"}, + cmdFileCmd: "foo", + }, + "Open called with short vos path and db path": { + args: []string{"-s", "/foo/vos-0", "-p", "/bar", "ls"}, + setup: func() { + ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { + fmt.Println("Open called") + if err := isArgEqual("/foo/vos-0", path, "vos path"); err != nil { + return err + } + if err := isArgEqual("/bar", dbPath, "sysdb path"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"Open called"}, + cmdFileArgs: []string{"-s", "/foo/vos-0", "-p", "/bar"}, + cmdFileCmd: "ls", + }, + "Open called with long vos path and db path": { + args: []string{"--vos_path=/foo/vos-0", "--db_path=/bar", "ls"}, + setup: func() { + ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { + fmt.Println("Open called") + if err := isArgEqual("/foo/vos-0", path, "vos path"); err != nil { + return err + } + if err := isArgEqual("/bar", dbPath, "sysdb path"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"Open called"}, + cmdFileArgs: []string{"--vos_path=/foo/vos-0", "--db_path=/bar"}, + cmdFileCmd: "ls", + }, + "Open called with write mode": { + args: []string{"-w", "-s", "/foo/vos-0", "ls"}, + setup: func() { + ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { + fmt.Println("Open called") + if err := isArgEqual(true, writeMode, "write_mode"); err != nil { + return err + } + return nil + } + }, + expStdout: []string{"Open called"}, + cmdFileArgs: []string{"-w", "-s", "/foo/vos-0"}, + cmdFileCmd: "ls", + }, + "No auto-open for feature command": { + // noAutoOpen is keyed on opts.Args.RunCmd which is empty in command-file + // mode, so this case only applies to command-line mode. + args: []string{"-s", "/foo/vos-0", "feature", "--show"}, + setup: func() { + ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { + return fmt.Errorf("open should not have been called") + } + }, + }, + "No auto-open for open command": { + // The CLI should NOT pre-open when the 'open' command is issued; only the + // command itself should call ctx.Open (exactly once). + // Only valid for command-line mode (see note above). + args: []string{"-s", "/foo/vos-0", "open", "/foo/vos-0"}, + setup: func() { + openCount := 0 + ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { + openCount++ + if openCount > 1 { + return fmt.Errorf("open pre-opened by CLI (called %d times)", openCount) + } + return nil + } + }, + }, + "No auto-open for smd_sync": { + args: []string{"-s", "/foo/vos-0", "smd_sync"}, + setup: func() { + ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { + return fmt.Errorf("open should not have been called") + } + }, + }, + "Init failure": { + args: []string{"ls"}, + expErr: ddbTestErr(ctxInitErr), + setup: func() { + ddb_init_RC = -1 // non-zero triggers DER_UNKNOWN + }, + }, + "Open failure": { + args: []string{"-s", "/foo/vos-0", "ls"}, + expErr: ddbTestErr("Error opening VOS path"), + setup: func() { + ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { + return fmt.Errorf("simulated open failure") + } + }, + }, + } { + t.Run(name, func(t *testing.T) { + checkRun := func(t *testing.T, stdout string, err error) { + t.Helper() + test.CmpErr(t, tc.expErr, err) + if tc.expErr != nil { + return + } + for _, msg := range tc.expStdout { + test.AssertTrue(t, strings.Contains(stdout, msg), + fmt.Sprintf("expected stdout to contain %q: got\n%s", msg, stdout)) + } + } + + // Command-line mode + t.Run("command-line", func(t *testing.T) { + ctx := newTestContext(t) + if tc.setup != nil { + tc.setup() + } + stdout, err := runMainFlow(ctx, tc.args) + checkRun(t, stdout, err) + }) + + // Command-file mode (only for cases that provide a command file command) + if tc.cmdFileCmd != "" { + t.Run("command-file", func(t *testing.T) { + tmpDir := t.TempDir() + cmdFile := filepath.Join(tmpDir, "cmds.txt") + if err := os.WriteFile(cmdFile, []byte(tc.cmdFileCmd), 0644); err != nil { + t.Fatalf("failed to write command file: %v", err) + } + + ctx := newTestContext(t) + if tc.setup != nil { + tc.setup() + } + args := append(tc.cmdFileArgs, "--cmd_file="+cmdFile) + stdout, err := runMainFlow(ctx, args) + checkRun(t, stdout, err) + }) + } + }) + } +} + +// TestRunMultiLineCommandFile verifies that runFileCmds iterates over multiple +// lines in a command file, executing each one in sequence. +func TestRunMultiLineCommandFile(t *testing.T) { + ctx := newTestContext(t) + ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { + fmt.Println("ls called") + return nil + } + ddb_run_version_Fn = func() error { + fmt.Println("version called") + return nil + } + + tmpDir := t.TempDir() + cmdFile := filepath.Join(tmpDir, "cmds.txt") + if err := os.WriteFile(cmdFile, []byte("ls\nversion\n"), 0644); err != nil { + t.Fatalf("failed to write command file: %v", err) + } + + stdout, err := runMainFlow(ctx, []string{"--cmd_file=" + cmdFile}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + assertContainsAll(t, stdout, []string{"ls called", "version called"}) +} + +func TestStrToLogLevels(t *testing.T) { + for name, tc := range map[string]struct { + input string + expCliLevel logging.LogLevel + expEngineLevel engine.LogLevel + expErr bool + }{ + "TRACE": {input: "TRACE", expCliLevel: logging.LogLevelTrace, expEngineLevel: engine.LogLevelDbug}, + "DEBUG": {input: "DEBUG", expCliLevel: logging.LogLevelDebug, expEngineLevel: engine.LogLevelDbug}, + "DBUG": {input: "DBUG", expCliLevel: logging.LogLevelDebug, expEngineLevel: engine.LogLevelDbug}, + "INFO": {input: "INFO", expCliLevel: logging.LogLevelInfo, expEngineLevel: engine.LogLevelInfo}, + "NOTE": {input: "NOTE", expCliLevel: logging.LogLevelNotice, expEngineLevel: engine.LogLevelNote}, + "NOTICE": {input: "NOTICE", expCliLevel: logging.LogLevelNotice, expEngineLevel: engine.LogLevelNote}, + "WARN": {input: "WARN", expCliLevel: logging.LogLevelNotice, expEngineLevel: engine.LogLevelWarn}, + "ERROR": {input: "ERROR", expCliLevel: logging.LogLevelError, expEngineLevel: engine.LogLevelErr}, + "ERR": {input: "ERR", expCliLevel: logging.LogLevelError, expEngineLevel: engine.LogLevelErr}, + "CRIT": {input: "CRIT", expCliLevel: logging.LogLevelError, expEngineLevel: engine.LogLevelCrit}, + "ALRT": {input: "ALRT", expCliLevel: logging.LogLevelError, expEngineLevel: engine.LogLevelAlrt}, + "FATAL": {input: "FATAL", expCliLevel: logging.LogLevelError, expEngineLevel: engine.LogLevelEmrg}, + "EMRG": {input: "EMRG", expCliLevel: logging.LogLevelError, expEngineLevel: engine.LogLevelEmrg}, + "EMIT": {input: "EMIT", expCliLevel: logging.LogLevelError, expEngineLevel: engine.LogLevelEmit}, + "lowercase debug": { + input: "debug", expCliLevel: logging.LogLevelDebug, expEngineLevel: engine.LogLevelDbug, + }, + "invalid level": {input: "INVALID", expErr: true}, + "empty string": {input: "", expErr: true}, + } { + t.Run(name, func(t *testing.T) { + cliLevel, engineLevel, err := strToLogLevels(tc.input) + if tc.expErr { + if err == nil { + t.Fatalf("expected an error for input %q: got nil", tc.input) + } + return + } + if err != nil { + t.Fatalf("unexpected error for input %q: %v", tc.input, err) + } + test.AssertTrue(t, cliLevel == tc.expCliLevel, + fmt.Sprintf("unexpected CLI log level for input %q: want %v, got %v", tc.input, tc.expCliLevel, cliLevel)) + test.AssertTrue(t, engineLevel == tc.expEngineLevel, + fmt.Sprintf("unexpected engine log level for input %q: want %v, got %v", tc.input, tc.expEngineLevel, engineLevel)) + }) + } +} + +// TestNewLogger verifies the newLogger code paths: default level, explicit debug +// level, invalid level, and all three LogDir branches (valid dir, non-existent +// path, path that is a file rather than a directory). +func TestNewLogger(t *testing.T) { + t.Run("no LogDir default level", func(t *testing.T) { + log, err := newLogger(cliOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if log == nil { + t.Fatal("expected non-nil logger") + } + }) + + t.Run("explicit valid debug level", func(t *testing.T) { + log, err := newLogger(cliOptions{Debug: "DEBUG"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if log == nil { + t.Fatal("expected non-nil logger") + } + }) + + t.Run("invalid debug level", func(t *testing.T) { + _, err := newLogger(cliOptions{Debug: "INVALID"}) + if err == nil { + t.Fatal("expected error for invalid log level, got nil") + } + }) + + t.Run("valid LogDir", func(t *testing.T) { + tmpDir := t.TempDir() + log, err := newLogger(cliOptions{LogDir: tmpDir}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if log == nil { + t.Fatal("expected non-nil logger") + } + }) + + t.Run("non-existent LogDir", func(t *testing.T) { + _, err := newLogger(cliOptions{LogDir: "/non/existent/path"}) + if err == nil { + t.Fatal("expected error for non-existent log dir, got nil") + } + }) + + t.Run("LogDir is a file", func(t *testing.T) { + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "not-a-dir") + if err := os.WriteFile(tmpFile, []byte(""), 0644); err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + _, err := newLogger(cliOptions{LogDir: tmpFile}) + if err == nil { + t.Fatal("expected error when LogDir is a file, got nil") + } + }) +} + +// TestClosePoolIfOpen verifies that closePoolIfOpen only calls Close when the +// pool is actually open, and that it tolerates a Close error (log only, no panic). +func TestClosePoolIfOpen(t *testing.T) { + log := logging.NewCommandLineLogger() + + t.Run("pool not open, close not called", func(t *testing.T) { + ctx := newTestContext(t) + ddb_pool_is_open_RC = false + ddb_run_close_Fn = func() error { + t.Fatal("Close should not be called when pool is not open") + return nil + } + closePoolIfOpen(ctx, log) + }) + + t.Run("pool open, close called", func(t *testing.T) { + ctx := newTestContext(t) + ddb_pool_is_open_RC = true + closeCalled := false + ddb_run_close_Fn = func() error { + closeCalled = true + return nil + } + closePoolIfOpen(ctx, log) + test.AssertTrue(t, closeCalled, "expected Close to have been called when pool is open") + }) + + t.Run("pool open, close returns error", func(t *testing.T) { + ctx := newTestContext(t) + ddb_pool_is_open_RC = true + ddb_run_close_Fn = func() error { + return fmt.Errorf("close failed") + } + // Should not panic; the error is only logged. + closePoolIfOpen(ctx, log) + }) +} diff --git a/src/control/cmd/ddb/test_helpers.go b/src/control/cmd/ddb/test_helpers.go new file mode 100644 index 00000000000..2f2d7e4e75e --- /dev/null +++ b/src/control/cmd/ddb/test_helpers.go @@ -0,0 +1,122 @@ +// +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP +// +// SPDX-License-Identifier: BSD-2-Clause-Patent +// + +//go:build test_stubs + +package main + +import ( + "bytes" + "fmt" + "io" + "os" + "reflect" + "strings" + "testing" + + "github.com/daos-stack/daos/src/control/build" + "github.com/daos-stack/daos/src/control/common/test" + "github.com/daos-stack/daos/src/control/logging" + "github.com/pkg/errors" +) + +type ddbTestErr string + +func (dte ddbTestErr) Error() string { + return string(dte) +} + +const ( + errUnknownCmd = ddbTestErr("Unknown command:") +) + +// newTestContext creates a fresh DdbContext for use in tests, resetting all +// stub variables to their zero values to ensure test isolation. +func newTestContext(t *testing.T) *DdbContext { + t.Helper() + resetDdbStubs() + return &DdbContext{} +} + +// captureStdout replaces os.Stdout with a pipe, runs fn, restores os.Stdout, +// and returns the captured output along with any error from fn. +func captureStdout(fn func() error) (string, error) { + var result bytes.Buffer + r, w, _ := os.Pipe() + done := make(chan struct{}) + go func() { + _, _ = io.Copy(&result, r) + close(done) + }() + stdout := os.Stdout + defer func() { os.Stdout = stdout }() + os.Stdout = w + + err := fn() + w.Close() + <-done + + return result.String(), err +} + +// runCmdToStdout calls parseOpts with the given args and captures stdout +// output. errHelpRequested is treated as a non-error (consistent with main()). +// Returns the parsed options, stdout output, and error. +func runCmdToStdout(ctx *DdbContext, args []string) (cliOptions, string, error) { + var opts cliOptions + stdout, err := captureStdout(func() error { + var e error + opts, _, e = parseOpts(args, ctx) + return e + }) + + if errors.Is(err, errHelpRequested) { + return opts, stdout, nil + } + return opts, stdout, err +} + +// runMainFlow simulates the main() execution flow without calling os.Exit. +// It calls parseOpts, handles the version flag, then calls run(). +// errHelpRequested is treated as a non-error (consistent with main()). +// Returns stdout output and any error. +func runMainFlow(ctx *DdbContext, args []string) (string, error) { + stdout, err := captureStdout(func() error { + opts, parser, e := parseOpts(args, ctx) + if errors.Is(e, errHelpRequested) { + return nil + } + if e != nil { + return e + } + + if opts.Version { + fmt.Printf("ddb version %s\n", build.DaosVersion) + return nil + } + + log := logging.NewCommandLineLogger() + return run(ctx, log, opts, parser) + }) + return stdout, err +} + +// assertContainsAll asserts that s contains each of the given substrings. +func assertContainsAll(t *testing.T, s string, substrings []string) { + t.Helper() + for _, sub := range substrings { + test.AssertTrue(t, strings.Contains(s, sub), + fmt.Sprintf("expected output to contain %q: got\n%s", sub, s)) + } +} + +func isArgEqual(want interface{}, got interface{}, wantName string) error { + if reflect.DeepEqual(want, got) { + return nil + } + + return errors.New(fmt.Sprintf("Unexpected %s argument: wanted '%+v', got '%+v'", wantName, want, got)) +} From bac707c9a8ecb303aad881be9fba9138090b426f Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Tue, 26 May 2026 07:27:38 +0000 Subject: [PATCH 02/11] DAOS-18304 ddb: replace inline anonymous test fns Fix @tanabarr reviewer feedback regarding anonymous inline functions. In TestRun (main_test.go), extract five named helper factories: - openFnChecking(wantPath, wantDbPath) -- checks path and sysdb path args - openFnCheckingWriteMode(wantWriteMode) -- checks write-mode flag only - openFnMustNotBeCalled -- fails if open is invoked - openFnAllowedOnce() -- allows exactly one call - openFnFailing -- always returns an error In TestCmds (ddb_commands_test.go), extract four named helper factories: - openFnChecking(wantPath, wantDbPath, wantWriteMode) - lsFnChecking(wantPath, wantRecursive, wantDetails) - featureFnCheckingShow(wantShow) - dtxAggrFnChecking(wantPath, wantCmtTime, wantCmtDate) Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/ddb_commands_test.go | 190 ++++++++--------------- src/control/cmd/ddb/main_test.go | 90 +++++------ 2 files changed, 107 insertions(+), 173 deletions(-) diff --git a/src/control/cmd/ddb/ddb_commands_test.go b/src/control/cmd/ddb/ddb_commands_test.go index 1571f9857d3..47ac468710b 100644 --- a/src/control/cmd/ddb/ddb_commands_test.go +++ b/src/control/cmd/ddb/ddb_commands_test.go @@ -77,6 +77,55 @@ func TestHelpCmds(t *testing.T) { } func TestCmds(t *testing.T) { + // Helper factories for command stub functions — declared here to avoid + // anonymous functions nested inside the test table. + + lsFnChecking := func(wantPath string, wantRecursive, wantDetails bool) func(string, bool, bool) error { + return func(path string, recursive, details bool) error { + fmt.Println("ls called") + if err := isArgEqual(wantPath, path, "path"); err != nil { + return err + } + if err := isArgEqual(wantRecursive, recursive, "recursive"); err != nil { + return err + } + return isArgEqual(wantDetails, details, "details") + } + } + + openFnChecking := func(wantPath, wantDbPath string, wantWriteMode bool) func(string, string, bool) error { + return func(path, dbPath string, writeMode bool) error { + fmt.Println("open called") + if err := isArgEqual(wantPath, path, "path"); err != nil { + return err + } + if err := isArgEqual(wantDbPath, dbPath, "db_path"); err != nil { + return err + } + return isArgEqual(wantWriteMode, writeMode, "write_mode") + } + } + + featureFnCheckingShow := func(wantShow bool) func(string, string, string, string, bool) error { + return func(_, _, _, _ string, show bool) error { + fmt.Println("feature called") + return isArgEqual(wantShow, show, "show") + } + } + + dtxAggrFnChecking := func(wantPath string, wantCmtTime uint64, wantCmtDate string) func(string, uint64, string) error { + return func(path string, cmtTime uint64, cmtDate string) error { + fmt.Println("dtx_aggr called") + if err := isArgEqual(wantPath, path, "path"); err != nil { + return err + } + if err := isArgEqual(wantCmtTime, cmtTime, "cmtTime"); err != nil { + return err + } + return isArgEqual(wantCmtDate, cmtDate, "cmtDate") + } + } + for name, tc := range map[string]struct { args []string setup func() @@ -95,95 +144,35 @@ func TestCmds(t *testing.T) { "ls default": { args: []string{"ls"}, setup: func() { - ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { - fmt.Println("ls called") - if err := isArgEqual("", path, "path"); err != nil { - return err - } - if err := isArgEqual(false, recursive, "recursive"); err != nil { - return err - } - if err := isArgEqual(false, details, "details"); err != nil { - return err - } - return nil - } + ddb_run_ls_Fn = lsFnChecking("", false, false) }, expStdout: []string{"ls called"}, }, "ls path": { args: []string{"ls", "/[0]"}, setup: func() { - ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { - fmt.Println("ls called") - if err := isArgEqual("/[0]", path, "path"); err != nil { - return err - } - if err := isArgEqual(false, recursive, "recursive"); err != nil { - return err - } - if err := isArgEqual(false, details, "details"); err != nil { - return err - } - return nil - } + ddb_run_ls_Fn = lsFnChecking("/[0]", false, false) }, expStdout: []string{"ls called"}, }, "ls long recursive opt": { args: []string{"ls", "--recursive"}, setup: func() { - ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { - fmt.Println("ls called") - if err := isArgEqual("", path, "path"); err != nil { - return err - } - if err := isArgEqual(true, recursive, "recursive"); err != nil { - return err - } - if err := isArgEqual(false, details, "details"); err != nil { - return err - } - return nil - } + ddb_run_ls_Fn = lsFnChecking("", true, false) }, expStdout: []string{"ls called"}, }, "ls short details opt": { args: []string{"ls", "-d"}, setup: func() { - ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { - fmt.Println("ls called") - if err := isArgEqual("", path, "path"); err != nil { - return err - } - if err := isArgEqual(false, recursive, "recursive"); err != nil { - return err - } - if err := isArgEqual(true, details, "details"); err != nil { - return err - } - return nil - } + ddb_run_ls_Fn = lsFnChecking("", false, true) }, expStdout: []string{"ls called"}, }, "ls details long opt": { args: []string{"ls", "--details"}, setup: func() { - ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { - fmt.Println("ls called") - if err := isArgEqual("", path, "path"); err != nil { - return err - } - if err := isArgEqual(false, recursive, "recursive"); err != nil { - return err - } - if err := isArgEqual(true, details, "details"); err != nil { - return err - } - return nil - } + ddb_run_ls_Fn = lsFnChecking("", false, true) }, expStdout: []string{"ls called"}, }, @@ -197,19 +186,7 @@ func TestCmds(t *testing.T) { "open default": { args: []string{"open", "/path/to/vos-0"}, setup: func() { - ddb_run_open_Fn = func(path, dbPath string, writeMode bool) error { - fmt.Println("open called") - if err := isArgEqual("/path/to/vos-0", path, "path"); err != nil { - return err - } - if err := isArgEqual("", dbPath, "db_path"); err != nil { - return err - } - if err := isArgEqual(false, writeMode, "write_mode"); err != nil { - return err - } - return nil - } + ddb_run_open_Fn = openFnChecking("/path/to/vos-0", "", false) }, expStdout: []string{"open called"}, }, @@ -217,13 +194,7 @@ func TestCmds(t *testing.T) { args: []string{"open", "-w", "/path/to/vos-0"}, skipCmdLine: "-w is consumed by the CLI write_mode flag before reaching grumble", setup: func() { - ddb_run_open_Fn = func(path, dbPath string, writeMode bool) error { - fmt.Println("open called") - if err := isArgEqual(true, writeMode, "write_mode"); err != nil { - return err - } - return nil - } + ddb_run_open_Fn = openFnChecking("/path/to/vos-0", "", true) }, expStdout: []string{"open called"}, }, @@ -231,16 +202,7 @@ func TestCmds(t *testing.T) { args: []string{"open", "-p", "/sysdb", "/path/to/vos-0"}, skipCmdLine: "-p is consumed by the CLI db_path flag before reaching grumble", setup: func() { - ddb_run_open_Fn = func(path, dbPath string, writeMode bool) error { - fmt.Println("open called") - if err := isArgEqual("/path/to/vos-0", path, "path"); err != nil { - return err - } - if err := isArgEqual("/sysdb", dbPath, "db_path"); err != nil { - return err - } - return nil - } + ddb_run_open_Fn = openFnChecking("/path/to/vos-0", "/sysdb", false) }, expStdout: []string{"open called"}, }, @@ -250,13 +212,7 @@ func TestCmds(t *testing.T) { "feature show": { args: []string{"feature", "--show"}, setup: func() { - ddb_run_feature_Fn = func(path, dbPath, enable, disable string, show bool) error { - fmt.Println("feature called") - if err := isArgEqual(true, show, "show"); err != nil { - return err - } - return nil - } + ddb_run_feature_Fn = featureFnCheckingShow(true) }, expStdout: []string{"feature called"}, }, @@ -313,45 +269,21 @@ func TestCmds(t *testing.T) { "dtx_aggr cmt_time": { args: []string{"dtx_aggr", "--cmt_time=1000"}, setup: func() { - ddb_run_dtx_aggr_Fn = func(path string, cmtTime uint64, cmtDate string) error { - fmt.Println("dtx_aggr called") - if err := isArgEqual(uint64(1000), cmtTime, "cmtTime"); err != nil { - return err - } - if err := isArgEqual("", cmtDate, "cmtDate"); err != nil { - return err - } - return nil - } + ddb_run_dtx_aggr_Fn = dtxAggrFnChecking("", 1000, "") }, expStdout: []string{"dtx_aggr called"}, }, "dtx_aggr cmt_date": { args: []string{"dtx_aggr", "--cmt_date=2024-01-01"}, setup: func() { - ddb_run_dtx_aggr_Fn = func(path string, cmtTime uint64, cmtDate string) error { - fmt.Println("dtx_aggr called") - if err := isArgEqual("2024-01-01", cmtDate, "cmtDate"); err != nil { - return err - } - return nil - } + ddb_run_dtx_aggr_Fn = dtxAggrFnChecking("", 0, "2024-01-01") }, expStdout: []string{"dtx_aggr called"}, }, "dtx_aggr with path": { args: []string{"dtx_aggr", "--cmt_time=0", "[0]"}, setup: func() { - ddb_run_dtx_aggr_Fn = func(path string, cmtTime uint64, cmtDate string) error { - fmt.Println("dtx_aggr called") - if err := isArgEqual("[0]", path, "path"); err != nil { - return err - } - if err := isArgEqual(uint64(0), cmtTime, "cmtTime"); err != nil { - return err - } - return nil - } + ddb_run_dtx_aggr_Fn = dtxAggrFnChecking("[0]", 0, "") }, expStdout: []string{"dtx_aggr called"}, }, diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index ceca06a104d..17cad1fafdb 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -221,6 +221,45 @@ func TestParseOpts(t *testing.T) { // entirely to grumble's app.Run(), which requires a real terminal (readline) and // piping os.Stdin — making tests fragile and hard to maintain for little gain. func TestRun(t *testing.T) { + // Helper factories for ddb_run_open_Fn — declared here to keep the test + // table readable instead of embedding anonymous functions inline. + + openFnChecking := func(wantPath, wantDbPath string) func(string, string, bool) error { + return func(path, dbPath string, _ bool) error { + fmt.Println("Open called") + if err := isArgEqual(wantPath, path, "vos path"); err != nil { + return err + } + return isArgEqual(wantDbPath, dbPath, "sysdb path") + } + } + + openFnCheckingWriteMode := func(wantWriteMode bool) func(string, string, bool) error { + return func(_ string, _ string, writeMode bool) error { + fmt.Println("Open called") + return isArgEqual(wantWriteMode, writeMode, "write_mode") + } + } + + openFnMustNotBeCalled := func(_ string, _ string, _ bool) error { + return fmt.Errorf("open should not have been called") + } + + openFnAllowedOnce := func() func(string, string, bool) error { + count := 0 + return func(_ string, _ string, _ bool) error { + count++ + if count > 1 { + return fmt.Errorf("open pre-opened by CLI (called %d times)", count) + } + return nil + } + } + + openFnFailing := func(_ string, _ string, _ bool) error { + return fmt.Errorf("simulated open failure") + } + for name, tc := range map[string]struct { args []string setup func() @@ -258,16 +297,7 @@ func TestRun(t *testing.T) { "Open called with short vos path and db path": { args: []string{"-s", "/foo/vos-0", "-p", "/bar", "ls"}, setup: func() { - ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { - fmt.Println("Open called") - if err := isArgEqual("/foo/vos-0", path, "vos path"); err != nil { - return err - } - if err := isArgEqual("/bar", dbPath, "sysdb path"); err != nil { - return err - } - return nil - } + ddb_run_open_Fn = openFnChecking("/foo/vos-0", "/bar") }, expStdout: []string{"Open called"}, cmdFileArgs: []string{"-s", "/foo/vos-0", "-p", "/bar"}, @@ -276,16 +306,7 @@ func TestRun(t *testing.T) { "Open called with long vos path and db path": { args: []string{"--vos_path=/foo/vos-0", "--db_path=/bar", "ls"}, setup: func() { - ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { - fmt.Println("Open called") - if err := isArgEqual("/foo/vos-0", path, "vos path"); err != nil { - return err - } - if err := isArgEqual("/bar", dbPath, "sysdb path"); err != nil { - return err - } - return nil - } + ddb_run_open_Fn = openFnChecking("/foo/vos-0", "/bar") }, expStdout: []string{"Open called"}, cmdFileArgs: []string{"--vos_path=/foo/vos-0", "--db_path=/bar"}, @@ -294,13 +315,7 @@ func TestRun(t *testing.T) { "Open called with write mode": { args: []string{"-w", "-s", "/foo/vos-0", "ls"}, setup: func() { - ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { - fmt.Println("Open called") - if err := isArgEqual(true, writeMode, "write_mode"); err != nil { - return err - } - return nil - } + ddb_run_open_Fn = openFnCheckingWriteMode(true) }, expStdout: []string{"Open called"}, cmdFileArgs: []string{"-w", "-s", "/foo/vos-0"}, @@ -311,9 +326,7 @@ func TestRun(t *testing.T) { // mode, so this case only applies to command-line mode. args: []string{"-s", "/foo/vos-0", "feature", "--show"}, setup: func() { - ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { - return fmt.Errorf("open should not have been called") - } + ddb_run_open_Fn = openFnMustNotBeCalled }, }, "No auto-open for open command": { @@ -322,22 +335,13 @@ func TestRun(t *testing.T) { // Only valid for command-line mode (see note above). args: []string{"-s", "/foo/vos-0", "open", "/foo/vos-0"}, setup: func() { - openCount := 0 - ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { - openCount++ - if openCount > 1 { - return fmt.Errorf("open pre-opened by CLI (called %d times)", openCount) - } - return nil - } + ddb_run_open_Fn = openFnAllowedOnce() }, }, "No auto-open for smd_sync": { args: []string{"-s", "/foo/vos-0", "smd_sync"}, setup: func() { - ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { - return fmt.Errorf("open should not have been called") - } + ddb_run_open_Fn = openFnMustNotBeCalled }, }, "Init failure": { @@ -351,9 +355,7 @@ func TestRun(t *testing.T) { args: []string{"-s", "/foo/vos-0", "ls"}, expErr: ddbTestErr("Error opening VOS path"), setup: func() { - ddb_run_open_Fn = func(path string, dbPath string, writeMode bool) error { - return fmt.Errorf("simulated open failure") - } + ddb_run_open_Fn = openFnFailing }, }, } { From 8568eae7c5f8463d1901b6e0b1360c969e4fd269 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Thu, 28 May 2026 08:42:19 +0000 Subject: [PATCH 03/11] DAOS-18304 ddb: extract runDdb() to decouple test helper from main() Per reviewer feedback (r3314932260): runMainFlow in test_helpers.go duplicated main()'s logic and had already silently drifted (calling logging.NewCommandLineLogger() directly instead of newLogger(opts)). Extract runDdb(ctx, args) as a production function containing the core execution logic: parseOpts, version flag, newLogger, and run(). main() retains only OS-level setup (SetTraceback, DisableCStdoutBuffering) and calls runDdb. The runMainFlow test helper becomes a thin wrapper around captureStdout(runDdb), ensuring tests always exercise the real code path. The io.Writer suggestion from the reviewer was not implemented: grumble writes to os.Stdout directly and cannot be easily redirected; the existing captureStdout pipe approach already handles all output correctly. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/main.go | 46 +++++++++++++++++------------ src/control/cmd/ddb/test_helpers.go | 27 +++-------------- 2 files changed, 31 insertions(+), 42 deletions(-) diff --git a/src/control/cmd/ddb/main.go b/src/control/cmd/ddb/main.go index 67cc415069b..97e2603b76a 100644 --- a/src/control/cmd/ddb/main.go +++ b/src/control/cmd/ddb/main.go @@ -365,37 +365,45 @@ func run(ctx *DdbContext, log *logging.LeveledLogger, opts cliOptions, parser *f return result } -func main() { - // Set the traceback level such that a crash results in - // a coredump (when ulimit -c is set appropriately). - debug.SetTraceback("crash") - - // Must be called before any write to stdout. - if err := logging.DisableCStdoutBuffering(); err != nil { - exitWithError(err) - } - - ctx := &DdbContext{} - opts, parser, err := parseOpts(os.Args[1:], ctx) +// runDdb contains the core ddb execution logic. It is separate from main() so +// that it can be tested without triggering os.Exit. main() handles only +// OS-level setup (traceback, stdout buffering) and calls exitWithError on +// failure; runDdb returns errors instead. +func runDdb(ctx *DdbContext, args []string) error { + opts, parser, err := parseOpts(args, ctx) if errors.Is(err, errHelpRequested) { - return + return nil } if err != nil { - exitWithError(err) + return err } if opts.Version { fmt.Printf("ddb version %s\n", build.DaosVersion) - return + return nil } - log, err := newLogger(opts) - if err != nil { - exitWithError(errors.Wrap(err, loggerInitErr)) + var log *logging.LeveledLogger + if log, err = newLogger(opts); err != nil { + return errors.Wrap(err, loggerInitErr) } log.Debug("Logging facilities initialized") - if err = run(ctx, log, opts, parser); err != nil { + return run(ctx, log, opts, parser) +} + +func main() { + // Set the traceback level such that a crash results in + // a coredump (when ulimit -c is set appropriately). + debug.SetTraceback("crash") + + // Must be called before any write to stdout. + if err := logging.DisableCStdoutBuffering(); err != nil { + exitWithError(err) + } + + ctx := &DdbContext{} + if err := runDdb(ctx, os.Args[1:]); err != nil { exitWithError(err) } } diff --git a/src/control/cmd/ddb/test_helpers.go b/src/control/cmd/ddb/test_helpers.go index 2f2d7e4e75e..3e9af3af650 100644 --- a/src/control/cmd/ddb/test_helpers.go +++ b/src/control/cmd/ddb/test_helpers.go @@ -17,9 +17,7 @@ import ( "strings" "testing" - "github.com/daos-stack/daos/src/control/build" "github.com/daos-stack/daos/src/control/common/test" - "github.com/daos-stack/daos/src/control/logging" "github.com/pkg/errors" ) @@ -79,29 +77,12 @@ func runCmdToStdout(ctx *DdbContext, args []string) (cliOptions, string, error) return opts, stdout, err } -// runMainFlow simulates the main() execution flow without calling os.Exit. -// It calls parseOpts, handles the version flag, then calls run(). -// errHelpRequested is treated as a non-error (consistent with main()). -// Returns stdout output and any error. +// runMainFlow captures stdout and delegates to the production runDdb function. +// This ensures tests exercise the same code path as main(), without calling os.Exit. func runMainFlow(ctx *DdbContext, args []string) (string, error) { - stdout, err := captureStdout(func() error { - opts, parser, e := parseOpts(args, ctx) - if errors.Is(e, errHelpRequested) { - return nil - } - if e != nil { - return e - } - - if opts.Version { - fmt.Printf("ddb version %s\n", build.DaosVersion) - return nil - } - - log := logging.NewCommandLineLogger() - return run(ctx, log, opts, parser) + return captureStdout(func() error { + return runDdb(ctx, args) }) - return stdout, err } // assertContainsAll asserts that s contains each of the given substrings. From a0fb368f067eb13c56eff79f5d07ec358019003a Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Fri, 29 May 2026 09:28:59 +0000 Subject: [PATCH 04/11] DAOS-18304 ddb: replace isArgEqual with test.CmpAny in stub check helpers Per reviewer feedback (r3318912192): replace the hand-rolled isArgEqual helper with the existing test.CmpAny utility. test.CmpAny uses cmp.Diff for clearer failure output and calls t.Fatalf directly, removing the error propagation boilerplate from stub closures. To flow the subtest t into the stub closures, setup fields in TestRun and TestCmds are changed from func() to func(*testing.T), and named helper factories (openFnChecking, openFnCheckingWriteMode, lsFnChecking, featureFnCheckingShow, dtxAggrFnChecking) gain a leading t *testing.T parameter. The isArgEqual function and its unused reflect import are removed from test_helpers.go. Since test.CmpAny calls t.Fatalf (which triggers runtime.Goexit), captureStdout is updated to use named return values and move w.Close() and <-done into the defer, ensuring the pipe and goroutine are always cleaned up even when the test fails mid-stub. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/ddb_commands_test.go | 114 ++++++++++------------- src/control/cmd/ddb/main_test.go | 42 ++++----- src/control/cmd/ddb/test_helpers.go | 27 +++--- 3 files changed, 83 insertions(+), 100 deletions(-) diff --git a/src/control/cmd/ddb/ddb_commands_test.go b/src/control/cmd/ddb/ddb_commands_test.go index 47ac468710b..a5450a71c96 100644 --- a/src/control/cmd/ddb/ddb_commands_test.go +++ b/src/control/cmd/ddb/ddb_commands_test.go @@ -80,55 +80,47 @@ func TestCmds(t *testing.T) { // Helper factories for command stub functions — declared here to avoid // anonymous functions nested inside the test table. - lsFnChecking := func(wantPath string, wantRecursive, wantDetails bool) func(string, bool, bool) error { + lsFnChecking := func(t *testing.T, wantPath string, wantRecursive, wantDetails bool) func(string, bool, bool) error { return func(path string, recursive, details bool) error { fmt.Println("ls called") - if err := isArgEqual(wantPath, path, "path"); err != nil { - return err - } - if err := isArgEqual(wantRecursive, recursive, "recursive"); err != nil { - return err - } - return isArgEqual(wantDetails, details, "details") + test.CmpAny(t, "path", wantPath, path) + test.CmpAny(t, "recursive", wantRecursive, recursive) + test.CmpAny(t, "details", wantDetails, details) + return nil } } - openFnChecking := func(wantPath, wantDbPath string, wantWriteMode bool) func(string, string, bool) error { + openFnChecking := func(t *testing.T, wantPath, wantDbPath string, wantWriteMode bool) func(string, string, bool) error { return func(path, dbPath string, writeMode bool) error { fmt.Println("open called") - if err := isArgEqual(wantPath, path, "path"); err != nil { - return err - } - if err := isArgEqual(wantDbPath, dbPath, "db_path"); err != nil { - return err - } - return isArgEqual(wantWriteMode, writeMode, "write_mode") + test.CmpAny(t, "path", wantPath, path) + test.CmpAny(t, "db_path", wantDbPath, dbPath) + test.CmpAny(t, "write_mode", wantWriteMode, writeMode) + return nil } } - featureFnCheckingShow := func(wantShow bool) func(string, string, string, string, bool) error { + featureFnCheckingShow := func(t *testing.T, wantShow bool) func(string, string, string, string, bool) error { return func(_, _, _, _ string, show bool) error { fmt.Println("feature called") - return isArgEqual(wantShow, show, "show") + test.CmpAny(t, "show", wantShow, show) + return nil } } - dtxAggrFnChecking := func(wantPath string, wantCmtTime uint64, wantCmtDate string) func(string, uint64, string) error { + dtxAggrFnChecking := func(t *testing.T, wantPath string, wantCmtTime uint64, wantCmtDate string) func(string, uint64, string) error { return func(path string, cmtTime uint64, cmtDate string) error { fmt.Println("dtx_aggr called") - if err := isArgEqual(wantPath, path, "path"); err != nil { - return err - } - if err := isArgEqual(wantCmtTime, cmtTime, "cmtTime"); err != nil { - return err - } - return isArgEqual(wantCmtDate, cmtDate, "cmtDate") + test.CmpAny(t, "path", wantPath, path) + test.CmpAny(t, "cmtTime", wantCmtTime, cmtTime) + test.CmpAny(t, "cmtDate", wantCmtDate, cmtDate) + return nil } } for name, tc := range map[string]struct { args []string - setup func() + setup func(*testing.T) expStdout []string expErr error // skipCmdLine skips the command-line sub-test with a message. Use when @@ -143,36 +135,36 @@ func TestCmds(t *testing.T) { }, "ls default": { args: []string{"ls"}, - setup: func() { - ddb_run_ls_Fn = lsFnChecking("", false, false) + setup: func(t *testing.T) { + ddb_run_ls_Fn = lsFnChecking(t, "", false, false) }, expStdout: []string{"ls called"}, }, "ls path": { args: []string{"ls", "/[0]"}, - setup: func() { - ddb_run_ls_Fn = lsFnChecking("/[0]", false, false) + setup: func(t *testing.T) { + ddb_run_ls_Fn = lsFnChecking(t, "/[0]", false, false) }, expStdout: []string{"ls called"}, }, "ls long recursive opt": { args: []string{"ls", "--recursive"}, - setup: func() { - ddb_run_ls_Fn = lsFnChecking("", true, false) + setup: func(t *testing.T) { + ddb_run_ls_Fn = lsFnChecking(t, "", true, false) }, expStdout: []string{"ls called"}, }, "ls short details opt": { args: []string{"ls", "-d"}, - setup: func() { - ddb_run_ls_Fn = lsFnChecking("", false, true) + setup: func(t *testing.T) { + ddb_run_ls_Fn = lsFnChecking(t, "", false, true) }, expStdout: []string{"ls called"}, }, "ls details long opt": { args: []string{"ls", "--details"}, - setup: func() { - ddb_run_ls_Fn = lsFnChecking("", false, true) + setup: func(t *testing.T) { + ddb_run_ls_Fn = lsFnChecking(t, "", false, true) }, expStdout: []string{"ls called"}, }, @@ -185,24 +177,24 @@ func TestCmds(t *testing.T) { // in command-file mode; see TestRun for CLI-level flag coverage. "open default": { args: []string{"open", "/path/to/vos-0"}, - setup: func() { - ddb_run_open_Fn = openFnChecking("/path/to/vos-0", "", false) + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnChecking(t, "/path/to/vos-0", "", false) }, expStdout: []string{"open called"}, }, "open write mode": { args: []string{"open", "-w", "/path/to/vos-0"}, skipCmdLine: "-w is consumed by the CLI write_mode flag before reaching grumble", - setup: func() { - ddb_run_open_Fn = openFnChecking("/path/to/vos-0", "", true) + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnChecking(t, "/path/to/vos-0", "", true) }, expStdout: []string{"open called"}, }, "open with db path": { args: []string{"open", "-p", "/sysdb", "/path/to/vos-0"}, skipCmdLine: "-p is consumed by the CLI db_path flag before reaching grumble", - setup: func() { - ddb_run_open_Fn = openFnChecking("/path/to/vos-0", "/sysdb", false) + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnChecking(t, "/path/to/vos-0", "/sysdb", false) }, expStdout: []string{"open called"}, }, @@ -211,15 +203,15 @@ func TestCmds(t *testing.T) { // feature --show: verifies the show flag is forwarded to the C layer. "feature show": { args: []string{"feature", "--show"}, - setup: func() { - ddb_run_feature_Fn = featureFnCheckingShow(true) + setup: func(t *testing.T) { + ddb_run_feature_Fn = featureFnCheckingShow(t, true) }, expStdout: []string{"feature called"}, }, // feature --enable: verifies that the enable string reaches ddb_feature_string2flags. "feature enable": { args: []string{"feature", "--enable=myflag"}, - setup: func() { + setup: func(t *testing.T) { var capturedFlag string ddb_feature_string2flags_Fn = func(s string) (uint64, uint64, error) { capturedFlag = s @@ -227,9 +219,7 @@ func TestCmds(t *testing.T) { } ddb_run_feature_Fn = func(path, dbPath, enable, disable string, show bool) error { fmt.Println("feature called") - if err := isArgEqual("myflag", capturedFlag, "enable flag string"); err != nil { - return err - } + test.CmpAny(t, "enable flag string", "myflag", capturedFlag) return nil } }, @@ -238,7 +228,7 @@ func TestCmds(t *testing.T) { // feature --disable: verifies that the disable string reaches ddb_feature_string2flags. "feature disable": { args: []string{"feature", "--disable=otherflag"}, - setup: func() { + setup: func(t *testing.T) { var capturedFlag string ddb_feature_string2flags_Fn = func(s string) (uint64, uint64, error) { capturedFlag = s @@ -246,9 +236,7 @@ func TestCmds(t *testing.T) { } ddb_run_feature_Fn = func(path, dbPath, enable, disable string, show bool) error { fmt.Println("feature called") - if err := isArgEqual("otherflag", capturedFlag, "disable flag string"); err != nil { - return err - } + test.CmpAny(t, "disable flag string", "otherflag", capturedFlag) return nil } }, @@ -268,22 +256,22 @@ func TestCmds(t *testing.T) { }, "dtx_aggr cmt_time": { args: []string{"dtx_aggr", "--cmt_time=1000"}, - setup: func() { - ddb_run_dtx_aggr_Fn = dtxAggrFnChecking("", 1000, "") + setup: func(t *testing.T) { + ddb_run_dtx_aggr_Fn = dtxAggrFnChecking(t, "", 1000, "") }, expStdout: []string{"dtx_aggr called"}, }, "dtx_aggr cmt_date": { args: []string{"dtx_aggr", "--cmt_date=2024-01-01"}, - setup: func() { - ddb_run_dtx_aggr_Fn = dtxAggrFnChecking("", 0, "2024-01-01") + setup: func(t *testing.T) { + ddb_run_dtx_aggr_Fn = dtxAggrFnChecking(t, "", 0, "2024-01-01") }, expStdout: []string{"dtx_aggr called"}, }, "dtx_aggr with path": { args: []string{"dtx_aggr", "--cmt_time=0", "[0]"}, - setup: func() { - ddb_run_dtx_aggr_Fn = dtxAggrFnChecking("[0]", 0, "") + setup: func(t *testing.T) { + ddb_run_dtx_aggr_Fn = dtxAggrFnChecking(t, "[0]", 0, "") }, expStdout: []string{"dtx_aggr called"}, }, @@ -291,7 +279,7 @@ func TestCmds(t *testing.T) { // --- close command --- "close": { args: []string{"close"}, - setup: func() { + setup: func(t *testing.T) { ddb_run_close_Fn = func() error { fmt.Println("close called") return nil @@ -303,7 +291,7 @@ func TestCmds(t *testing.T) { // --- version command --- "version": { args: []string{"version"}, - setup: func() { + setup: func(t *testing.T) { ddb_run_version_Fn = func() error { fmt.Println("version called") return nil @@ -340,7 +328,7 @@ func TestCmds(t *testing.T) { } ctx := newTestContext(t) if tc.setup != nil { - tc.setup() + tc.setup(t) } stdout, err := runMainFlow(ctx, tc.args) checkCmd(t, stdout, err) @@ -355,7 +343,7 @@ func TestCmds(t *testing.T) { } ctx := newTestContext(t) if tc.setup != nil { - tc.setup() + tc.setup(t) } stdout, err := runMainFlow(ctx, []string{"--cmd_file=" + cmdFile}) checkCmd(t, stdout, err) diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index 17cad1fafdb..f35cab0c2be 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -224,20 +224,20 @@ func TestRun(t *testing.T) { // Helper factories for ddb_run_open_Fn — declared here to keep the test // table readable instead of embedding anonymous functions inline. - openFnChecking := func(wantPath, wantDbPath string) func(string, string, bool) error { + openFnChecking := func(t *testing.T, wantPath, wantDbPath string) func(string, string, bool) error { return func(path, dbPath string, _ bool) error { fmt.Println("Open called") - if err := isArgEqual(wantPath, path, "vos path"); err != nil { - return err - } - return isArgEqual(wantDbPath, dbPath, "sysdb path") + test.CmpAny(t, "vos path", wantPath, path) + test.CmpAny(t, "sysdb path", wantDbPath, dbPath) + return nil } } - openFnCheckingWriteMode := func(wantWriteMode bool) func(string, string, bool) error { + openFnCheckingWriteMode := func(t *testing.T, wantWriteMode bool) func(string, string, bool) error { return func(_ string, _ string, writeMode bool) error { fmt.Println("Open called") - return isArgEqual(wantWriteMode, writeMode, "write_mode") + test.CmpAny(t, "write_mode", wantWriteMode, writeMode) + return nil } } @@ -262,7 +262,7 @@ func TestRun(t *testing.T) { for name, tc := range map[string]struct { args []string - setup func() + setup func(*testing.T) expStdout []string expErr error // When cmdFileCmd is non-empty the test is also run in command-file mode. @@ -296,8 +296,8 @@ func TestRun(t *testing.T) { }, "Open called with short vos path and db path": { args: []string{"-s", "/foo/vos-0", "-p", "/bar", "ls"}, - setup: func() { - ddb_run_open_Fn = openFnChecking("/foo/vos-0", "/bar") + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") }, expStdout: []string{"Open called"}, cmdFileArgs: []string{"-s", "/foo/vos-0", "-p", "/bar"}, @@ -305,8 +305,8 @@ func TestRun(t *testing.T) { }, "Open called with long vos path and db path": { args: []string{"--vos_path=/foo/vos-0", "--db_path=/bar", "ls"}, - setup: func() { - ddb_run_open_Fn = openFnChecking("/foo/vos-0", "/bar") + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") }, expStdout: []string{"Open called"}, cmdFileArgs: []string{"--vos_path=/foo/vos-0", "--db_path=/bar"}, @@ -314,8 +314,8 @@ func TestRun(t *testing.T) { }, "Open called with write mode": { args: []string{"-w", "-s", "/foo/vos-0", "ls"}, - setup: func() { - ddb_run_open_Fn = openFnCheckingWriteMode(true) + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnCheckingWriteMode(t, true) }, expStdout: []string{"Open called"}, cmdFileArgs: []string{"-w", "-s", "/foo/vos-0"}, @@ -325,7 +325,7 @@ func TestRun(t *testing.T) { // noAutoOpen is keyed on opts.Args.RunCmd which is empty in command-file // mode, so this case only applies to command-line mode. args: []string{"-s", "/foo/vos-0", "feature", "--show"}, - setup: func() { + setup: func(t *testing.T) { ddb_run_open_Fn = openFnMustNotBeCalled }, }, @@ -334,27 +334,27 @@ func TestRun(t *testing.T) { // command itself should call ctx.Open (exactly once). // Only valid for command-line mode (see note above). args: []string{"-s", "/foo/vos-0", "open", "/foo/vos-0"}, - setup: func() { + setup: func(t *testing.T) { ddb_run_open_Fn = openFnAllowedOnce() }, }, "No auto-open for smd_sync": { args: []string{"-s", "/foo/vos-0", "smd_sync"}, - setup: func() { + setup: func(t *testing.T) { ddb_run_open_Fn = openFnMustNotBeCalled }, }, "Init failure": { args: []string{"ls"}, expErr: ddbTestErr(ctxInitErr), - setup: func() { + setup: func(t *testing.T) { ddb_init_RC = -1 // non-zero triggers DER_UNKNOWN }, }, "Open failure": { args: []string{"-s", "/foo/vos-0", "ls"}, expErr: ddbTestErr("Error opening VOS path"), - setup: func() { + setup: func(t *testing.T) { ddb_run_open_Fn = openFnFailing }, }, @@ -376,7 +376,7 @@ func TestRun(t *testing.T) { t.Run("command-line", func(t *testing.T) { ctx := newTestContext(t) if tc.setup != nil { - tc.setup() + tc.setup(t) } stdout, err := runMainFlow(ctx, tc.args) checkRun(t, stdout, err) @@ -393,7 +393,7 @@ func TestRun(t *testing.T) { ctx := newTestContext(t) if tc.setup != nil { - tc.setup() + tc.setup(t) } args := append(tc.cmdFileArgs, "--cmd_file="+cmdFile) stdout, err := runMainFlow(ctx, args) diff --git a/src/control/cmd/ddb/test_helpers.go b/src/control/cmd/ddb/test_helpers.go index 3e9af3af650..3a1a3c75b32 100644 --- a/src/control/cmd/ddb/test_helpers.go +++ b/src/control/cmd/ddb/test_helpers.go @@ -13,7 +13,6 @@ import ( "fmt" "io" "os" - "reflect" "strings" "testing" @@ -41,7 +40,7 @@ func newTestContext(t *testing.T) *DdbContext { // captureStdout replaces os.Stdout with a pipe, runs fn, restores os.Stdout, // and returns the captured output along with any error from fn. -func captureStdout(fn func() error) (string, error) { +func captureStdout(fn func() error) (output string, err error) { var result bytes.Buffer r, w, _ := os.Pipe() done := make(chan struct{}) @@ -50,14 +49,18 @@ func captureStdout(fn func() error) (string, error) { close(done) }() stdout := os.Stdout - defer func() { os.Stdout = stdout }() os.Stdout = w + // Deferred so cleanup and output capture always run, even if fn() exits + // via runtime.Goexit() (e.g., t.Fatalf called from a stub via test.CmpAny). + defer func() { + w.Close() + <-done + os.Stdout = stdout + output = result.String() + }() - err := fn() - w.Close() - <-done - - return result.String(), err + err = fn() + return } // runCmdToStdout calls parseOpts with the given args and captures stdout @@ -93,11 +96,3 @@ func assertContainsAll(t *testing.T, s string, substrings []string) { fmt.Sprintf("expected output to contain %q: got\n%s", sub, s)) } } - -func isArgEqual(want interface{}, got interface{}, wantName string) error { - if reflect.DeepEqual(want, got) { - return nil - } - - return errors.New(fmt.Sprintf("Unexpected %s argument: wanted '%+v', got '%+v'", wantName, want, got)) -} From 1133d7ce93b239b6e5821af690c315d21a594421 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Mon, 1 Jun 2026 07:11:13 +0000 Subject: [PATCH 05/11] DAOS-18304 ddb: inline runMainFlow into call sites Per reviewer feedback (r3318906385): remove the runMainFlow helper and inline captureStdout(func() error { return runDdb(ctx, args) }) directly at the 8 call sites in main_test.go and ddb_commands_test.go. runMainFlow was a meaningful abstraction when it contained ~15 lines of logic (parseOpts, version handling, run). After the runDdb extraction commit it became a thin 3-line pass-through that adds indirection without semantic value. Inlining makes each test site self-contained and explicit about what it does: capture stdout while running the production entry point. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/ddb_commands_test.go | 24 ++++++++++++++++++------ src/control/cmd/ddb/main_test.go | 12 +++++++++--- src/control/cmd/ddb/test_helpers.go | 8 -------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/control/cmd/ddb/ddb_commands_test.go b/src/control/cmd/ddb/ddb_commands_test.go index a5450a71c96..4d5032cf87e 100644 --- a/src/control/cmd/ddb/ddb_commands_test.go +++ b/src/control/cmd/ddb/ddb_commands_test.go @@ -33,7 +33,9 @@ func runHelpCmd(t *testing.T, cmdStr string, helpSubStr string) { // Run the help command with a command file args := test.JoinArgs(nil, "--cmd_file="+tmpCfgFile) - stdoutCmdFile, err := runMainFlow(ctx, args) + stdoutCmdFile, err := captureStdout(func() error { + return runDdb(ctx, args) + }) if err != nil { t.Fatalf("unexpected error when running '%s --help' via command file: want nil, got %v", cmdStr, err) } @@ -42,7 +44,9 @@ func runHelpCmd(t *testing.T, cmdStr string, helpSubStr string) { // Run the help command with a command line args = test.JoinArgs(nil, cmdStr, "--help") - stdoutCmdLine, err := runMainFlow(ctx, args) + stdoutCmdLine, err := captureStdout(func() error { + return runDdb(ctx, args) + }) if err != nil { t.Fatalf("unexpected error when running '%s --help' via command line: want nil, got %v", cmdStr, err) } @@ -330,7 +334,9 @@ func TestCmds(t *testing.T) { if tc.setup != nil { tc.setup(t) } - stdout, err := runMainFlow(ctx, tc.args) + stdout, err := captureStdout(func() error { + return runDdb(ctx, tc.args) + }) checkCmd(t, stdout, err) }) @@ -345,7 +351,9 @@ func TestCmds(t *testing.T) { if tc.setup != nil { tc.setup(t) } - stdout, err := runMainFlow(ctx, []string{"--cmd_file=" + cmdFile}) + stdout, err := captureStdout(func() error { + return runDdb(ctx, []string{"--cmd_file=" + cmdFile}) + }) checkCmd(t, stdout, err) }) }) @@ -366,7 +374,9 @@ func TestManPage(t *testing.T) { // manpage to stdout: must contain all section headers and known commands. ctx := newTestContext(t) - stdout, err := runMainFlow(ctx, []string{"manpage"}) + stdout, err := captureStdout(func() error { + return runDdb(ctx, []string{"manpage"}) + }) test.CmpErr(t, nil, err) assertContainsAll(t, stdout, expSections) @@ -375,7 +385,9 @@ func TestManPage(t *testing.T) { outFile := filepath.Join(tmpDir, "ddb.groff") ctx = newTestContext(t) - stdout, err = runMainFlow(ctx, []string{"manpage", "--output=" + outFile}) + stdout, err = captureStdout(func() error { + return runDdb(ctx, []string{"manpage", "--output=" + outFile}) + }) if err != nil { t.Fatalf("unexpected error when running 'manpage --output': want nil, got %v", err) } diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index f35cab0c2be..c76a6c5ebca 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -378,7 +378,9 @@ func TestRun(t *testing.T) { if tc.setup != nil { tc.setup(t) } - stdout, err := runMainFlow(ctx, tc.args) + stdout, err := captureStdout(func() error { + return runDdb(ctx, tc.args) + }) checkRun(t, stdout, err) }) @@ -396,7 +398,9 @@ func TestRun(t *testing.T) { tc.setup(t) } args := append(tc.cmdFileArgs, "--cmd_file="+cmdFile) - stdout, err := runMainFlow(ctx, args) + stdout, err := captureStdout(func() error { + return runDdb(ctx, args) + }) checkRun(t, stdout, err) }) } @@ -423,7 +427,9 @@ func TestRunMultiLineCommandFile(t *testing.T) { t.Fatalf("failed to write command file: %v", err) } - stdout, err := runMainFlow(ctx, []string{"--cmd_file=" + cmdFile}) + stdout, err := captureStdout(func() error { + return runDdb(ctx, []string{"--cmd_file=" + cmdFile}) + }) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/src/control/cmd/ddb/test_helpers.go b/src/control/cmd/ddb/test_helpers.go index 3a1a3c75b32..c12746f42fa 100644 --- a/src/control/cmd/ddb/test_helpers.go +++ b/src/control/cmd/ddb/test_helpers.go @@ -80,14 +80,6 @@ func runCmdToStdout(ctx *DdbContext, args []string) (cliOptions, string, error) return opts, stdout, err } -// runMainFlow captures stdout and delegates to the production runDdb function. -// This ensures tests exercise the same code path as main(), without calling os.Exit. -func runMainFlow(ctx *DdbContext, args []string) (string, error) { - return captureStdout(func() error { - return runDdb(ctx, args) - }) -} - // assertContainsAll asserts that s contains each of the given substrings. func assertContainsAll(t *testing.T, s string, substrings []string) { t.Helper() From a05118862e841cccb647eff68e620d25152bd1fd Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Mon, 1 Jun 2026 07:27:01 +0000 Subject: [PATCH 06/11] DAOS-18304 ddb: move assertContainsAll to common/test as AssertStringContains Per reviewer feedback (r3318926829): assertContainsAll has no ddb-specific logic and belongs in the shared test utilities package so it can be reused across the entire control-plane test suite. Add AssertStringContains(t, s string, substrings ...string) to common/test/utils.go. The variadic signature handles both single and multiple substring checks; existing slice call sites use the spread operator (expSections...). Remove assertContainsAll from test_helpers.go along with the now-unused fmt, strings, and common/test imports. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/ddb_commands_test.go | 4 ++-- src/control/cmd/ddb/main_test.go | 2 +- src/control/cmd/ddb/test_helpers.go | 12 ------------ src/control/common/test/utils.go | 12 +++++++++++- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/control/cmd/ddb/ddb_commands_test.go b/src/control/cmd/ddb/ddb_commands_test.go index 4d5032cf87e..2138f8221a0 100644 --- a/src/control/cmd/ddb/ddb_commands_test.go +++ b/src/control/cmd/ddb/ddb_commands_test.go @@ -378,7 +378,7 @@ func TestManPage(t *testing.T) { return runDdb(ctx, []string{"manpage"}) }) test.CmpErr(t, nil, err) - assertContainsAll(t, stdout, expSections) + test.AssertStringContains(t, stdout, expSections...) // --output flag: man page is written to a file, stdout is empty. tmpDir := t.TempDir() @@ -398,5 +398,5 @@ func TestManPage(t *testing.T) { if err != nil { t.Fatalf("failed to read output file: %v", err) } - assertContainsAll(t, string(content), expSections) + test.AssertStringContains(t, string(content), expSections...) } diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index c76a6c5ebca..d8571e059b8 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -433,7 +433,7 @@ func TestRunMultiLineCommandFile(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - assertContainsAll(t, stdout, []string{"ls called", "version called"}) + test.AssertStringContains(t, stdout, "ls called", "version called") } func TestStrToLogLevels(t *testing.T) { diff --git a/src/control/cmd/ddb/test_helpers.go b/src/control/cmd/ddb/test_helpers.go index c12746f42fa..a2662b2fad4 100644 --- a/src/control/cmd/ddb/test_helpers.go +++ b/src/control/cmd/ddb/test_helpers.go @@ -10,13 +10,10 @@ package main import ( "bytes" - "fmt" "io" "os" - "strings" "testing" - "github.com/daos-stack/daos/src/control/common/test" "github.com/pkg/errors" ) @@ -79,12 +76,3 @@ func runCmdToStdout(ctx *DdbContext, args []string) (cliOptions, string, error) } return opts, stdout, err } - -// assertContainsAll asserts that s contains each of the given substrings. -func assertContainsAll(t *testing.T, s string, substrings []string) { - t.Helper() - for _, sub := range substrings { - test.AssertTrue(t, strings.Contains(s, sub), - fmt.Sprintf("expected output to contain %q: got\n%s", sub, s)) - } -} diff --git a/src/control/common/test/utils.go b/src/control/common/test/utils.go index 5b731550b9c..3f149baa41e 100644 --- a/src/control/common/test/utils.go +++ b/src/control/common/test/utils.go @@ -1,6 +1,6 @@ // // (C) Copyright 2018-2024 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // (C) Copyright 2025 Google LLC // // SPDX-License-Identifier: BSD-2-Clause-Patent @@ -150,6 +150,16 @@ func CmpAny(t *testing.T, desc string, want, got any, cmpOpts ...cmp.Option) { } } +// AssertStringContains asserts that s contains each of the provided substrings. +func AssertStringContains(t *testing.T, s string, substrings ...string) { + t.Helper() + for _, sub := range substrings { + if !strings.Contains(s, sub) { + t.Fatalf("expected string to contain %q: got\n%s", sub, s) + } + } +} + // SplitFile separates file content into contiguous sections separated by // a blank line. func SplitFile(path string) (sections [][]string, err error) { From 0d67e4277f4cfea755579f26966811e18f70d573 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Mon, 1 Jun 2026 07:53:16 +0000 Subject: [PATCH 07/11] DAOS-18304 ddb: split TestRun into TestRun and TestRunCommandFile Per reviewer feedback (r3318965302): split the single TestRun function into two focused test functions. Previously TestRun ran each case in two modes (command-line and command-file) controlled by optional cmdFileArgs/cmdFileCmd struct fields and a conditional t.Run block. This made the table struct heavier and the runner harder to follow. After the split: - TestRun covers command-line mode only; the table has a flat (args, setup, expStdout, expErr) struct and a single straightforward runner body. - TestRunCommandFile covers command-file mode only; the table uses a (flags, cmdLine, setup, expStdout, expErr) struct that matches the command-file execution model directly. The five openFn* helper factories are moved to package level so both test functions can share them without duplication. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/main_test.go | 223 +++++++++++++++++-------------- 1 file changed, 123 insertions(+), 100 deletions(-) diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index d8571e059b8..27a96a48753 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -216,63 +216,62 @@ func TestParseOpts(t *testing.T) { } } -// TestRun covers the non-interactive execution paths of run() (command-line and -// command-file modes). Interactive mode is intentionally not tested: it delegates -// entirely to grumble's app.Run(), which requires a real terminal (readline) and -// piping os.Stdin — making tests fragile and hard to maintain for little gain. -func TestRun(t *testing.T) { - // Helper factories for ddb_run_open_Fn — declared here to keep the test - // table readable instead of embedding anonymous functions inline. - - openFnChecking := func(t *testing.T, wantPath, wantDbPath string) func(string, string, bool) error { - return func(path, dbPath string, _ bool) error { - fmt.Println("Open called") - test.CmpAny(t, "vos path", wantPath, path) - test.CmpAny(t, "sysdb path", wantDbPath, dbPath) - return nil - } +// openFnChecking returns a ddb_run_open_Fn stub that verifies the vos path and +// sysdb path arguments passed to the open call. +func openFnChecking(t *testing.T, wantPath, wantDbPath string) func(string, string, bool) error { + return func(path, dbPath string, _ bool) error { + fmt.Println("Open called") + test.CmpAny(t, "vos path", wantPath, path) + test.CmpAny(t, "sysdb path", wantDbPath, dbPath) + return nil } +} - openFnCheckingWriteMode := func(t *testing.T, wantWriteMode bool) func(string, string, bool) error { - return func(_ string, _ string, writeMode bool) error { - fmt.Println("Open called") - test.CmpAny(t, "write_mode", wantWriteMode, writeMode) - return nil - } +// openFnCheckingWriteMode returns a ddb_run_open_Fn stub that verifies the +// write_mode argument passed to the open call. +func openFnCheckingWriteMode(t *testing.T, wantWriteMode bool) func(string, string, bool) error { + return func(_ string, _ string, writeMode bool) error { + fmt.Println("Open called") + test.CmpAny(t, "write_mode", wantWriteMode, writeMode) + return nil } +} - openFnMustNotBeCalled := func(_ string, _ string, _ bool) error { - return fmt.Errorf("open should not have been called") - } +// openFnMustNotBeCalled is a ddb_run_open_Fn stub that fails the test if +// the open function is called at all (used to verify no-auto-open behavior). +func openFnMustNotBeCalled(_ string, _ string, _ bool) error { + return fmt.Errorf("open should not have been called") +} - openFnAllowedOnce := func() func(string, string, bool) error { - count := 0 - return func(_ string, _ string, _ bool) error { - count++ - if count > 1 { - return fmt.Errorf("open pre-opened by CLI (called %d times)", count) - } - return nil +// openFnAllowedOnce returns a ddb_run_open_Fn stub that allows the open +// function to be called exactly once (used to verify the 'open' command +// itself calls open but the CLI does not pre-open). +func openFnAllowedOnce() func(string, string, bool) error { + count := 0 + return func(_ string, _ string, _ bool) error { + count++ + if count > 1 { + return fmt.Errorf("open pre-opened by CLI (called %d times)", count) } + return nil } +} - openFnFailing := func(_ string, _ string, _ bool) error { - return fmt.Errorf("simulated open failure") - } +// openFnFailing is a ddb_run_open_Fn stub that always returns an error, +// used to verify that open failures are propagated correctly. +func openFnFailing(_ string, _ string, _ bool) error { + return fmt.Errorf("simulated open failure") +} +// TestRun covers the non-interactive command-line execution paths of runDdb(). +// Interactive mode is intentionally not tested: it delegates entirely to +// grumble's app.Run(), which requires a real terminal and is hard to automate. +func TestRun(t *testing.T) { for name, tc := range map[string]struct { args []string setup func(*testing.T) expStdout []string expErr error - // When cmdFileCmd is non-empty the test is also run in command-file mode. - // cmdFileArgs holds the CLI flags (everything except the positional command), - // and cmdFileCmd is the line written to the temporary command file. - // Note: "no auto-open" cases intentionally omit cmdFileCmd because in - // command-file mode opts.Args.RunCmd is always empty, so noAutoOpen is - // never triggered and the CLI would pre-open the pool. - cmdFileArgs []string - cmdFileCmd string }{ "Version output": { args: []string{"-v"}, @@ -283,43 +282,33 @@ func TestRun(t *testing.T) { expStdout: []string{"ddb version"}, }, "Unknown command": { - args: []string{"foo"}, - expErr: errUnknownCmd, - cmdFileArgs: []string{}, - cmdFileCmd: "foo", + args: []string{"foo"}, + expErr: errUnknownCmd, }, "Unknown command with write option": { - args: []string{"-w", "foo"}, - expErr: errUnknownCmd, - cmdFileArgs: []string{"-w"}, - cmdFileCmd: "foo", + args: []string{"-w", "foo"}, + expErr: errUnknownCmd, }, "Open called with short vos path and db path": { args: []string{"-s", "/foo/vos-0", "-p", "/bar", "ls"}, setup: func(t *testing.T) { ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") }, - expStdout: []string{"Open called"}, - cmdFileArgs: []string{"-s", "/foo/vos-0", "-p", "/bar"}, - cmdFileCmd: "ls", + expStdout: []string{"Open called"}, }, "Open called with long vos path and db path": { args: []string{"--vos_path=/foo/vos-0", "--db_path=/bar", "ls"}, setup: func(t *testing.T) { ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") }, - expStdout: []string{"Open called"}, - cmdFileArgs: []string{"--vos_path=/foo/vos-0", "--db_path=/bar"}, - cmdFileCmd: "ls", + expStdout: []string{"Open called"}, }, "Open called with write mode": { args: []string{"-w", "-s", "/foo/vos-0", "ls"}, setup: func(t *testing.T) { ddb_run_open_Fn = openFnCheckingWriteMode(t, true) }, - expStdout: []string{"Open called"}, - cmdFileArgs: []string{"-w", "-s", "/foo/vos-0"}, - cmdFileCmd: "ls", + expStdout: []string{"Open called"}, }, "No auto-open for feature command": { // noAutoOpen is keyed on opts.Args.RunCmd which is empty in command-file @@ -360,49 +349,83 @@ func TestRun(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - checkRun := func(t *testing.T, stdout string, err error) { - t.Helper() - test.CmpErr(t, tc.expErr, err) - if tc.expErr != nil { - return - } - for _, msg := range tc.expStdout { - test.AssertTrue(t, strings.Contains(stdout, msg), - fmt.Sprintf("expected stdout to contain %q: got\n%s", msg, stdout)) - } + ctx := newTestContext(t) + if tc.setup != nil { + tc.setup(t) } - - // Command-line mode - t.Run("command-line", func(t *testing.T) { - ctx := newTestContext(t) - if tc.setup != nil { - tc.setup(t) - } - stdout, err := captureStdout(func() error { - return runDdb(ctx, tc.args) - }) - checkRun(t, stdout, err) + stdout, err := captureStdout(func() error { + return runDdb(ctx, tc.args) }) + test.CmpErr(t, tc.expErr, err) + if tc.expErr == nil { + test.AssertStringContains(t, stdout, tc.expStdout...) + } + }) + } +} - // Command-file mode (only for cases that provide a command file command) - if tc.cmdFileCmd != "" { - t.Run("command-file", func(t *testing.T) { - tmpDir := t.TempDir() - cmdFile := filepath.Join(tmpDir, "cmds.txt") - if err := os.WriteFile(cmdFile, []byte(tc.cmdFileCmd), 0644); err != nil { - t.Fatalf("failed to write command file: %v", err) - } - - ctx := newTestContext(t) - if tc.setup != nil { - tc.setup(t) - } - args := append(tc.cmdFileArgs, "--cmd_file="+cmdFile) - stdout, err := captureStdout(func() error { - return runDdb(ctx, args) - }) - checkRun(t, stdout, err) - }) +// TestRunCommandFile covers command-file execution paths of runDdb(). +// It exercises the same behavior as TestRun for cases where command-file +// mode produces the same result as command-line mode. +func TestRunCommandFile(t *testing.T) { + for name, tc := range map[string]struct { + flags []string // CLI flags (before --cmd_file) + cmdLine string // line written to the temporary command file + setup func(*testing.T) + expStdout []string + expErr error + }{ + "Unknown command": { + cmdLine: "foo", + expErr: errUnknownCmd, + }, + "Unknown command with write option": { + flags: []string{"-w"}, + cmdLine: "foo", + expErr: errUnknownCmd, + }, + "Open called with short vos path and db path": { + flags: []string{"-s", "/foo/vos-0", "-p", "/bar"}, + cmdLine: "ls", + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") + }, + expStdout: []string{"Open called"}, + }, + "Open called with long vos path and db path": { + flags: []string{"--vos_path=/foo/vos-0", "--db_path=/bar"}, + cmdLine: "ls", + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") + }, + expStdout: []string{"Open called"}, + }, + "Open called with write mode": { + flags: []string{"-w", "-s", "/foo/vos-0"}, + cmdLine: "ls", + setup: func(t *testing.T) { + ddb_run_open_Fn = openFnCheckingWriteMode(t, true) + }, + expStdout: []string{"Open called"}, + }, + } { + t.Run(name, func(t *testing.T) { + tmpDir := t.TempDir() + cmdFile := filepath.Join(tmpDir, "cmds.txt") + if err := os.WriteFile(cmdFile, []byte(tc.cmdLine), 0644); err != nil { + t.Fatalf("failed to write command file: %v", err) + } + ctx := newTestContext(t) + if tc.setup != nil { + tc.setup(t) + } + args := append(tc.flags, "--cmd_file="+cmdFile) + stdout, err := captureStdout(func() error { + return runDdb(ctx, args) + }) + test.CmpErr(t, tc.expErr, err) + if tc.expErr == nil { + test.AssertStringContains(t, stdout, tc.expStdout...) } }) } From 0219aa4c6e3d8a10a29c49a1a96b9ded898004c3 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Mon, 1 Jun 2026 08:11:02 +0000 Subject: [PATCH 08/11] DAOS-18304 ddb: rename test functions with TestDdb_ prefix Per reviewer tip (r3318987058): follow the Test_ convention used in cmd/daos and other control-plane packages, so that failing tests are immediately identifiable by package when running the full unit test suite. Rename map (12 functions, 3 files): TestParseOpts -> TestDdb_parseOpts TestRun -> TestDdb_runDdb TestRunCommandFile -> TestDdb_runDdbCommandFile TestRunMultiLineCommandFile -> TestDdb_runFileCmds TestStrToLogLevels -> TestDdb_strToLogLevels TestNewLogger -> TestDdb_newLogger TestClosePoolIfOpen -> TestDdb_closePoolIfOpen TestHelpCmds -> TestDdb_HelpCmds TestCmds -> TestDdb_Cmds TestManPage -> TestDdb_ManPage TestListVosFiles -> TestDdb_listVosFiles TestFilterSuggestions -> TestDdb_filterSuggestions No logic changes. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/command_completers_test.go | 4 ++-- src/control/cmd/ddb/ddb_commands_test.go | 6 +++--- src/control/cmd/ddb/main_test.go | 14 +++++++------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/control/cmd/ddb/command_completers_test.go b/src/control/cmd/ddb/command_completers_test.go index 5594fae469d..70f7b131d54 100644 --- a/src/control/cmd/ddb/command_completers_test.go +++ b/src/control/cmd/ddb/command_completers_test.go @@ -60,7 +60,7 @@ func testSetup(t *testing.T) string { return tmpDir } -func TestListVosFiles(t *testing.T) { +func TestDdb_listVosFiles(t *testing.T) { tmpDir := testSetup(t) for name, tc := range map[string]struct { @@ -128,7 +128,7 @@ func TestListVosFiles(t *testing.T) { } } -func TestFilterSuggestions(t *testing.T) { +func TestDdb_filterSuggestions(t *testing.T) { // The test cases are designed to cover various prefix scenarios. // It should notably cover the case where the prefix is a single character that matches the // second character of a suggestion, which is a special case in the appendSuggestion diff --git a/src/control/cmd/ddb/ddb_commands_test.go b/src/control/cmd/ddb/ddb_commands_test.go index 2138f8221a0..23b293225ed 100644 --- a/src/control/cmd/ddb/ddb_commands_test.go +++ b/src/control/cmd/ddb/ddb_commands_test.go @@ -58,7 +58,7 @@ func runHelpCmd(t *testing.T, cmdStr string, helpSubStr string) { fmt.Sprintf("unexpected help output mismatch between command file and command line for '%s'", cmdStr)) } -func TestHelpCmds(t *testing.T) { +func TestDdb_HelpCmds(t *testing.T) { for name, tc := range map[string]struct { cmdStr string helpSubStr string @@ -80,7 +80,7 @@ func TestHelpCmds(t *testing.T) { } } -func TestCmds(t *testing.T) { +func TestDdb_Cmds(t *testing.T) { // Helper factories for command stub functions — declared here to avoid // anonymous functions nested inside the test table. @@ -360,7 +360,7 @@ func TestCmds(t *testing.T) { } } -func TestManPage(t *testing.T) { +func TestDdb_ManPage(t *testing.T) { // Expected sections and commands present in every man page rendering. expSections := []string{ manArgsHeader, diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index 27a96a48753..991d4992b9c 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -20,7 +20,7 @@ import ( "github.com/daos-stack/daos/src/control/server/engine" ) -func TestParseOpts(t *testing.T) { +func TestDdb_parseOpts(t *testing.T) { for name, tc := range map[string]struct { args []string checkFunc func(opts *cliOptions) error @@ -266,7 +266,7 @@ func openFnFailing(_ string, _ string, _ bool) error { // TestRun covers the non-interactive command-line execution paths of runDdb(). // Interactive mode is intentionally not tested: it delegates entirely to // grumble's app.Run(), which requires a real terminal and is hard to automate. -func TestRun(t *testing.T) { +func TestDdb_runDdb(t *testing.T) { for name, tc := range map[string]struct { args []string setup func(*testing.T) @@ -367,7 +367,7 @@ func TestRun(t *testing.T) { // TestRunCommandFile covers command-file execution paths of runDdb(). // It exercises the same behavior as TestRun for cases where command-file // mode produces the same result as command-line mode. -func TestRunCommandFile(t *testing.T) { +func TestDdb_runDdbCommandFile(t *testing.T) { for name, tc := range map[string]struct { flags []string // CLI flags (before --cmd_file) cmdLine string // line written to the temporary command file @@ -433,7 +433,7 @@ func TestRunCommandFile(t *testing.T) { // TestRunMultiLineCommandFile verifies that runFileCmds iterates over multiple // lines in a command file, executing each one in sequence. -func TestRunMultiLineCommandFile(t *testing.T) { +func TestDdb_runFileCmds(t *testing.T) { ctx := newTestContext(t) ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { fmt.Println("ls called") @@ -459,7 +459,7 @@ func TestRunMultiLineCommandFile(t *testing.T) { test.AssertStringContains(t, stdout, "ls called", "version called") } -func TestStrToLogLevels(t *testing.T) { +func TestDdb_strToLogLevels(t *testing.T) { for name, tc := range map[string]struct { input string expCliLevel logging.LogLevel @@ -508,7 +508,7 @@ func TestStrToLogLevels(t *testing.T) { // TestNewLogger verifies the newLogger code paths: default level, explicit debug // level, invalid level, and all three LogDir branches (valid dir, non-existent // path, path that is a file rather than a directory). -func TestNewLogger(t *testing.T) { +func TestDdb_newLogger(t *testing.T) { t.Run("no LogDir default level", func(t *testing.T) { log, err := newLogger(cliOptions{}) if err != nil { @@ -569,7 +569,7 @@ func TestNewLogger(t *testing.T) { // TestClosePoolIfOpen verifies that closePoolIfOpen only calls Close when the // pool is actually open, and that it tolerates a Close error (log only, no panic). -func TestClosePoolIfOpen(t *testing.T) { +func TestDdb_closePoolIfOpen(t *testing.T) { log := logging.NewCommandLineLogger() t.Run("pool not open, close not called", func(t *testing.T) { From 62e75b187340a7ba220409e0e4f2da388d9fb204 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Mon, 1 Jun 2026 08:33:23 +0000 Subject: [PATCH 09/11] DAOS-18304 ddb: inline runHelpCmd into TestDdb_HelpCmds The runHelpCmd helper was called in a single place with straightforward logic. Inlining it into the t.Run body makes TestDdb_HelpCmds fully self-contained without unnecessary indirection. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/ddb_commands_test.go | 75 +++++++++++------------- 1 file changed, 34 insertions(+), 41 deletions(-) diff --git a/src/control/cmd/ddb/ddb_commands_test.go b/src/control/cmd/ddb/ddb_commands_test.go index 23b293225ed..a01c5c16d05 100644 --- a/src/control/cmd/ddb/ddb_commands_test.go +++ b/src/control/cmd/ddb/ddb_commands_test.go @@ -19,45 +19,6 @@ import ( "github.com/daos-stack/daos/src/control/common/test" ) -func runHelpCmd(t *testing.T, cmdStr string, helpSubStr string) { - t.Helper() - - ctx := newTestContext(t) - - // Create a temporary config file with the help command - tmpCfgDir := t.TempDir() - tmpCfgFile := path.Join(tmpCfgDir, "ddb-cmd_file.txt") - if err := os.WriteFile(tmpCfgFile, []byte(fmt.Sprintf("%s --help", cmdStr)), 0644); err != nil { - t.Fatalf("failed to write temp config file: %v", err) - } - - // Run the help command with a command file - args := test.JoinArgs(nil, "--cmd_file="+tmpCfgFile) - stdoutCmdFile, err := captureStdout(func() error { - return runDdb(ctx, args) - }) - if err != nil { - t.Fatalf("unexpected error when running '%s --help' via command file: want nil, got %v", cmdStr, err) - } - test.AssertTrue(t, strings.Contains(stdoutCmdFile, helpSubStr), - fmt.Sprintf("expected stdout to contain %q: got\n%s", helpSubStr, stdoutCmdFile)) - - // Run the help command with a command line - args = test.JoinArgs(nil, cmdStr, "--help") - stdoutCmdLine, err := captureStdout(func() error { - return runDdb(ctx, args) - }) - if err != nil { - t.Fatalf("unexpected error when running '%s --help' via command line: want nil, got %v", cmdStr, err) - } - test.AssertTrue(t, strings.Contains(stdoutCmdLine, helpSubStr), - fmt.Sprintf("expected stdout to contain %q: got\n%s", helpSubStr, stdoutCmdLine)) - - // Compare command line and command file outputs - test.AssertEqual(t, stdoutCmdFile, stdoutCmdLine, - fmt.Sprintf("unexpected help output mismatch between command file and command line for '%s'", cmdStr)) -} - func TestDdb_HelpCmds(t *testing.T) { for name, tc := range map[string]struct { cmdStr string @@ -72,10 +33,42 @@ func TestDdb_HelpCmds(t *testing.T) { helpSubStr: "Usage:\n open [flags] path\n", }, // TODO(follow-up PR): Add help tests for the remaining commands. - // Use runHelpCmd(t, "", "Usage:\n ") following the same pattern. } { t.Run(name, func(t *testing.T) { - runHelpCmd(t, tc.cmdStr, tc.helpSubStr) + ctx := newTestContext(t) + + // Create a temporary config file with the help command + tmpCfgDir := t.TempDir() + tmpCfgFile := path.Join(tmpCfgDir, "ddb-cmd_file.txt") + if err := os.WriteFile(tmpCfgFile, []byte(fmt.Sprintf("%s --help", tc.cmdStr)), 0644); err != nil { + t.Fatalf("failed to write temp config file: %v", err) + } + + // Run the help command with a command file + args := test.JoinArgs(nil, "--cmd_file="+tmpCfgFile) + stdoutCmdFile, err := captureStdout(func() error { + return runDdb(ctx, args) + }) + if err != nil { + t.Fatalf("unexpected error when running '%s --help' via command file: want nil, got %v", tc.cmdStr, err) + } + test.AssertTrue(t, strings.Contains(stdoutCmdFile, tc.helpSubStr), + fmt.Sprintf("expected stdout to contain %q: got\n%s", tc.helpSubStr, stdoutCmdFile)) + + // Run the help command with a command line + args = test.JoinArgs(nil, tc.cmdStr, "--help") + stdoutCmdLine, err := captureStdout(func() error { + return runDdb(ctx, args) + }) + if err != nil { + t.Fatalf("unexpected error when running '%s --help' via command line: want nil, got %v", tc.cmdStr, err) + } + test.AssertTrue(t, strings.Contains(stdoutCmdLine, tc.helpSubStr), + fmt.Sprintf("expected stdout to contain %q: got\n%s", tc.helpSubStr, stdoutCmdLine)) + + // Compare command line and command file outputs + test.AssertEqual(t, stdoutCmdFile, stdoutCmdLine, + fmt.Sprintf("unexpected help output mismatch between command file and command line for '%s'", tc.cmdStr)) }) } } From 996fca0c65e97be0e9061c4e2306b2910363c3df Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Tue, 2 Jun 2026 09:05:27 +0000 Subject: [PATCH 10/11] DAOS-18304 ddb: expose errHelpRequested from runCmdToStdout The helper was silently converting errHelpRequested to nil, hiding a sentinel that runDdb acts on. Unit tests should verify that parseOpts returns the sentinel when --help is passed. Remove the masking guard from runCmdToStdout and add expErr: errHelpRequested to the two help-message test cases. Move the expStdout check before the early-return guard so these cases still verify both the sentinel and the stdout output. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/main_test.go | 9 ++++++--- src/control/cmd/ddb/test_helpers.go | 8 +------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index 991d4992b9c..0a5d56e83f7 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -34,6 +34,7 @@ func TestDdb_parseOpts(t *testing.T) { "VOS Paths:\n", "Available Commands:\n", }, + expErr: errHelpRequested, }, "General help message with opt": { args: []string{"-w", "--help"}, @@ -42,6 +43,7 @@ func TestDdb_parseOpts(t *testing.T) { "VOS Paths:\n", "Available Commands:\n", }, + expErr: errHelpRequested, }, "Unknown commands with help": { args: []string{"foo", "--help"}, @@ -198,15 +200,16 @@ func TestDdb_parseOpts(t *testing.T) { opts, stdout, err := runCmdToStdout(ctx, tc.args) test.CmpErr(t, tc.expErr, err) - if tc.expErr != nil { - return - } for _, msg := range tc.expStdout { test.AssertTrue(t, strings.Contains(stdout, msg), fmt.Sprintf("expected stdout to contain %q: got\n%s", msg, stdout)) } + if tc.expErr != nil { + return + } + if tc.checkFunc != nil { if err := tc.checkFunc(&opts); err != nil { t.Fatal(err) diff --git a/src/control/cmd/ddb/test_helpers.go b/src/control/cmd/ddb/test_helpers.go index a2662b2fad4..3a171fec36c 100644 --- a/src/control/cmd/ddb/test_helpers.go +++ b/src/control/cmd/ddb/test_helpers.go @@ -13,8 +13,6 @@ import ( "io" "os" "testing" - - "github.com/pkg/errors" ) type ddbTestErr string @@ -61,8 +59,7 @@ func captureStdout(fn func() error) (output string, err error) { } // runCmdToStdout calls parseOpts with the given args and captures stdout -// output. errHelpRequested is treated as a non-error (consistent with main()). -// Returns the parsed options, stdout output, and error. +// output. Returns the parsed options, stdout output, and error. func runCmdToStdout(ctx *DdbContext, args []string) (cliOptions, string, error) { var opts cliOptions stdout, err := captureStdout(func() error { @@ -71,8 +68,5 @@ func runCmdToStdout(ctx *DdbContext, args []string) (cliOptions, string, error) return e }) - if errors.Is(err, errHelpRequested) { - return opts, stdout, nil - } return opts, stdout, err } From 59a68c7e2493a865c59a008ab84e232d595078cb Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Tue, 2 Jun 2026 12:48:06 +0000 Subject: [PATCH 11/11] DAOS-18304 ddb: use closure flags instead of stdout for stub signalling Replace fmt.Println("Open called") in openFnChecking and openFnCheckingWriteMode with a *bool pointer parameter. Each test case declares a closure-scoped "called" bool, passes &called to the stub, and registers t.Cleanup to assert invocation. The same pattern is applied to TestDdb_runFileCmds which used fmt.Println in ls/version stubs. This removes the dependency on captureStdout for stub verification and eliminates the risk of false positives from production code printing the same string. The remaining captureStdout usages (version output and help text) are legitimate: grumble writes directly to os.Stdout with no injection point. Signed-off-by: Cedric Koch-Hofer --- src/control/cmd/ddb/main_test.go | 57 ++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/src/control/cmd/ddb/main_test.go b/src/control/cmd/ddb/main_test.go index 0a5d56e83f7..03c761b94b8 100644 --- a/src/control/cmd/ddb/main_test.go +++ b/src/control/cmd/ddb/main_test.go @@ -220,10 +220,11 @@ func TestDdb_parseOpts(t *testing.T) { } // openFnChecking returns a ddb_run_open_Fn stub that verifies the vos path and -// sysdb path arguments passed to the open call. -func openFnChecking(t *testing.T, wantPath, wantDbPath string) func(string, string, bool) error { +// sysdb path arguments passed to the open call. called is set to true when the +// stub is invoked, allowing the caller to assert that open was called. +func openFnChecking(t *testing.T, wantPath, wantDbPath string, called *bool) func(string, string, bool) error { return func(path, dbPath string, _ bool) error { - fmt.Println("Open called") + *called = true test.CmpAny(t, "vos path", wantPath, path) test.CmpAny(t, "sysdb path", wantDbPath, dbPath) return nil @@ -231,10 +232,11 @@ func openFnChecking(t *testing.T, wantPath, wantDbPath string) func(string, stri } // openFnCheckingWriteMode returns a ddb_run_open_Fn stub that verifies the -// write_mode argument passed to the open call. -func openFnCheckingWriteMode(t *testing.T, wantWriteMode bool) func(string, string, bool) error { +// write_mode argument passed to the open call. called is set to true when the +// stub is invoked, allowing the caller to assert that open was called. +func openFnCheckingWriteMode(t *testing.T, wantWriteMode bool, called *bool) func(string, string, bool) error { return func(_ string, _ string, writeMode bool) error { - fmt.Println("Open called") + *called = true test.CmpAny(t, "write_mode", wantWriteMode, writeMode) return nil } @@ -295,23 +297,26 @@ func TestDdb_runDdb(t *testing.T) { "Open called with short vos path and db path": { args: []string{"-s", "/foo/vos-0", "-p", "/bar", "ls"}, setup: func(t *testing.T) { - ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") + var called bool + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar", &called) + t.Cleanup(func() { test.AssertTrue(t, called, "open was not called") }) }, - expStdout: []string{"Open called"}, }, "Open called with long vos path and db path": { args: []string{"--vos_path=/foo/vos-0", "--db_path=/bar", "ls"}, setup: func(t *testing.T) { - ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") + var called bool + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar", &called) + t.Cleanup(func() { test.AssertTrue(t, called, "open was not called") }) }, - expStdout: []string{"Open called"}, }, "Open called with write mode": { args: []string{"-w", "-s", "/foo/vos-0", "ls"}, setup: func(t *testing.T) { - ddb_run_open_Fn = openFnCheckingWriteMode(t, true) + var called bool + ddb_run_open_Fn = openFnCheckingWriteMode(t, true, &called) + t.Cleanup(func() { test.AssertTrue(t, called, "open was not called") }) }, - expStdout: []string{"Open called"}, }, "No auto-open for feature command": { // noAutoOpen is keyed on opts.Args.RunCmd which is empty in command-file @@ -391,25 +396,28 @@ func TestDdb_runDdbCommandFile(t *testing.T) { flags: []string{"-s", "/foo/vos-0", "-p", "/bar"}, cmdLine: "ls", setup: func(t *testing.T) { - ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") + var called bool + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar", &called) + t.Cleanup(func() { test.AssertTrue(t, called, "open was not called") }) }, - expStdout: []string{"Open called"}, }, "Open called with long vos path and db path": { flags: []string{"--vos_path=/foo/vos-0", "--db_path=/bar"}, cmdLine: "ls", setup: func(t *testing.T) { - ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar") + var called bool + ddb_run_open_Fn = openFnChecking(t, "/foo/vos-0", "/bar", &called) + t.Cleanup(func() { test.AssertTrue(t, called, "open was not called") }) }, - expStdout: []string{"Open called"}, }, "Open called with write mode": { flags: []string{"-w", "-s", "/foo/vos-0"}, cmdLine: "ls", setup: func(t *testing.T) { - ddb_run_open_Fn = openFnCheckingWriteMode(t, true) + var called bool + ddb_run_open_Fn = openFnCheckingWriteMode(t, true, &called) + t.Cleanup(func() { test.AssertTrue(t, called, "open was not called") }) }, - expStdout: []string{"Open called"}, }, } { t.Run(name, func(t *testing.T) { @@ -438,12 +446,13 @@ func TestDdb_runDdbCommandFile(t *testing.T) { // lines in a command file, executing each one in sequence. func TestDdb_runFileCmds(t *testing.T) { ctx := newTestContext(t) + var lsCalled, versionCalled bool ddb_run_ls_Fn = func(path string, recursive bool, details bool) error { - fmt.Println("ls called") + lsCalled = true return nil } ddb_run_version_Fn = func() error { - fmt.Println("version called") + versionCalled = true return nil } @@ -453,13 +462,11 @@ func TestDdb_runFileCmds(t *testing.T) { t.Fatalf("failed to write command file: %v", err) } - stdout, err := captureStdout(func() error { - return runDdb(ctx, []string{"--cmd_file=" + cmdFile}) - }) - if err != nil { + if err := runDdb(ctx, []string{"--cmd_file=" + cmdFile}); err != nil { t.Fatalf("unexpected error: %v", err) } - test.AssertStringContains(t, stdout, "ls called", "version called") + test.AssertTrue(t, lsCalled, "ls was not called") + test.AssertTrue(t, versionCalled, "version was not called") } func TestDdb_strToLogLevels(t *testing.T) {