urfave/cli's built-in version flag claimed -v, so "mfer -v --version" failed to parse, and -v meant version at the root but verbose on generate, check, freshen and fetch. Verbose is the more common meaning, so the root now takes -v and -q too, and the version flag takes -V. A -v or -q before generate, check, freshen, fetch or export applies to that subcommand; list keeps its fixed quiet logging. User-visible changes: -v no longer prints the version; use -V or --version. "mfer version" prints "mfer version 0.1.0 (...)", the same line as --version, instead of the bare "0.1.0 (...)". Tests: runCLI now points the logger at io.Discard after each run, so other tests' log lines cannot land in a finished run's output. Model: opus-4-8 (implementation); opus-5-5 (rework)
This commit was merged in pull request #107.
This commit is contained in:
+125
-3
@@ -5,6 +5,7 @@ import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"math/rand"
|
||||
"os"
|
||||
"sync"
|
||||
@@ -14,6 +15,7 @@ import (
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
urfcli "github.com/urfave/cli/v2"
|
||||
"sneak.berlin/go/mfer/internal/log"
|
||||
"sneak.berlin/go/mfer/mfer"
|
||||
)
|
||||
|
||||
@@ -27,6 +29,7 @@ const (
|
||||
testManifest = "/manifest.mf"
|
||||
testFlagBase = "--base"
|
||||
testFlagNoExtra = "--no-extra-files"
|
||||
testFlagVersion = "--version"
|
||||
)
|
||||
|
||||
var errSimulatedWrite = errors.New("simulated write failure")
|
||||
@@ -39,12 +42,20 @@ var errSimulatedWrite = errors.New("simulated write failure")
|
||||
var runMu sync.Mutex
|
||||
|
||||
// runCLI invokes RunWithOptions while holding runMu so parallel tests
|
||||
// capture their own output.
|
||||
// capture their own output. Before releasing the lock it points the
|
||||
// process-global logger at io.Discard: other tests log outside the lock
|
||||
// (manifest loads, scans), and those lines must not land in this run's
|
||||
// buffers once it has returned and its test is reading them.
|
||||
func runCLI(opts *RunOptions) int {
|
||||
runMu.Lock()
|
||||
defer runMu.Unlock()
|
||||
|
||||
return RunWithOptions(opts)
|
||||
exitCode := RunWithOptions(opts)
|
||||
|
||||
log.SetOutput(io.Discard, io.Discard)
|
||||
log.Init()
|
||||
|
||||
return exitCode
|
||||
}
|
||||
|
||||
func TestMain(m *testing.M) {
|
||||
@@ -102,7 +113,7 @@ func TestVersionCommand(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
opts := testOpts([]string{testApp, "version"}, fs)
|
||||
opts := testOpts([]string{testApp, cmdVersion}, fs)
|
||||
|
||||
exitCode := runCLI(opts)
|
||||
|
||||
@@ -113,6 +124,117 @@ func TestVersionCommand(t *testing.T) {
|
||||
assert.Contains(t, stdout, "abc123")
|
||||
}
|
||||
|
||||
// TestVFlagCollision covers the -v/--verbose vs --version flag interaction
|
||||
// (issue #64). Verbose owns -v; version answers to --version and -V. None of
|
||||
// these invocations may produce a parser error, and the two ways of asking
|
||||
// for the version must print the same thing.
|
||||
func TestVFlagCollision(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Invocations that must print the version and exit 0.
|
||||
versionCases := map[string][]string{
|
||||
"long version flag": {testApp, testFlagVersion},
|
||||
"short version flag": {testApp, "-V"},
|
||||
"verbose then version": {testApp, "-v", testFlagVersion},
|
||||
"long verbose and version": {testApp, "--verbose", testFlagVersion},
|
||||
}
|
||||
|
||||
for name, args := range versionCases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
opts := testOpts(args, afero.NewMemMapFs())
|
||||
exitCode := runCLI(opts)
|
||||
|
||||
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
|
||||
assert.Contains(t, testStdout(t, opts), mfer.Version)
|
||||
assert.NotContains(t, testStderr(t, opts), "two forms of the same flag")
|
||||
})
|
||||
}
|
||||
|
||||
// Invocations that must enable verbose and exit 0 without a parser error.
|
||||
verboseCases := map[string][]string{
|
||||
"short verbose flag": {testApp, "-v"},
|
||||
"long verbose flag": {testApp, "--verbose"},
|
||||
}
|
||||
|
||||
for name, args := range verboseCases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
opts := testOpts(args, afero.NewMemMapFs())
|
||||
exitCode := runCLI(opts)
|
||||
|
||||
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
|
||||
assert.Contains(t, testStdout(t, opts), cmdGenerate,
|
||||
"root should show help listing subcommands")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestVersionFlagAndCommandMatch asserts that "mfer --version" and
|
||||
// "mfer version" produce identical output (issue #64).
|
||||
func TestVersionFlagAndCommandMatch(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
flagOpts := testOpts([]string{testApp, testFlagVersion}, afero.NewMemMapFs())
|
||||
require.Equal(t, 0, runCLI(flagOpts))
|
||||
|
||||
cmdOpts := testOpts([]string{testApp, cmdVersion}, afero.NewMemMapFs())
|
||||
require.Equal(t, 0, runCLI(cmdOpts))
|
||||
|
||||
assert.Equal(t, testStdout(t, flagOpts), testStdout(t, cmdOpts))
|
||||
}
|
||||
|
||||
// TestRootVerbosityFlags asserts that -q and -v given before any subcommand
|
||||
// name take effect: in the root itself, and in the fetch and export
|
||||
// subcommands (issue #64). It does not run in parallel: other tests log
|
||||
// outside runMu (manifest loads, scans), and a line logged while one of these
|
||||
// runs is in progress lands in its output.
|
||||
//
|
||||
//nolint:paralleltest // see above
|
||||
func TestRootVerbosityFlags(t *testing.T) {
|
||||
t.Run("quiet with no subcommand hides the banner", func(t *testing.T) {
|
||||
loud := testOpts([]string{testApp}, afero.NewMemMapFs())
|
||||
require.Equal(t, 0, runCLI(loud))
|
||||
assert.Contains(t, testStdout(t, loud), banner)
|
||||
|
||||
quiet := testOpts([]string{testApp, "-q"}, afero.NewMemMapFs())
|
||||
require.Equal(t, 0, runCLI(quiet))
|
||||
assert.Contains(t, testStdout(t, quiet), cmdGenerate)
|
||||
assert.NotContains(t, testStdout(t, quiet), banner)
|
||||
})
|
||||
|
||||
t.Run("quiet before fetch hides the banner", func(t *testing.T) {
|
||||
opts := testOpts([]string{testApp, "-q", cmdFetch}, afero.NewMemMapFs())
|
||||
|
||||
assert.Equal(t, 1, runCLI(opts))
|
||||
assert.Contains(t, testStderr(t, opts), errURLRequired.Error())
|
||||
assert.NotContains(t, testStdout(t, opts), banner)
|
||||
})
|
||||
|
||||
t.Run("verbose twice before fetch enables debug logging", func(t *testing.T) {
|
||||
opts := testOpts([]string{testApp, "-v", "-v", cmdFetch}, afero.NewMemMapFs())
|
||||
|
||||
assert.Equal(t, 1, runCLI(opts))
|
||||
assert.Contains(t, testStderr(t, opts), "fetchManifestOperation()")
|
||||
})
|
||||
|
||||
t.Run("quiet before export hides the manifest summary", func(t *testing.T) {
|
||||
fs := afero.NewMemMapFs()
|
||||
manifest := buildTestManifest(t, map[string][]byte{"a.txt": []byte("a")})
|
||||
require.NoError(t, afero.WriteFile(fs, testManifest, manifest, 0o644))
|
||||
|
||||
loud := testOpts([]string{testApp, cmdExport, testManifest}, fs)
|
||||
require.Equal(t, 0, runCLI(loud))
|
||||
assert.Contains(t, testStderr(t, loud), "loaded manifest")
|
||||
|
||||
quiet := testOpts([]string{testApp, "-q", cmdExport, testManifest}, fs)
|
||||
require.Equal(t, 0, runCLI(quiet))
|
||||
assert.NotContains(t, testStderr(t, quiet), "loaded manifest")
|
||||
})
|
||||
}
|
||||
|
||||
func TestHelpCommand(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user