refactor: finish the jsonutil dedupe; back up config before auto-upgrade

Duplicated helpers
  cmd/rip had its own copies of six helpers that already existed in
  internal/jsonutil: asString (StringFromAny), searchInt (IntFromAny),
  nestedSearchInt and nestedSearchString (NestedAny/NestedString),
  firstNonEmpty (FirstNonEmpty) and firstPositiveInt. firstPositiveInt was
  duplicated a third and fourth time in provider/yandex and provider/beatport,
  so it moves into jsonutil alongside the rest. jsonutil is back to 100%
  coverage.

  searchBool is deliberately left alone, with a comment saying why: it accepts
  only a real bool, and switching it to jsonutil.BoolFromAny would start
  coercing strings and numbers, changing how search results get labelled.

config auto-upgrade is no longer destructive
  UpgradeOutdated re-serializes the whole config from the parsed struct, so any
  key this version does not know about is dropped, and it runs automatically
  from run() without prompting. It now copies the original to
  <path>.bak-<unixtime> (0600, since it holds the same credentials) and
  validates the result before writing. It returns the backup path so the CLI
  can tell the user where it went. The comment states plainly that this is a
  version stamp rather than a migration framework.

cmd/rip tests
  The command bodies are testable now that they are functions returning exit
  codes rather than arms of a switch that called os.Exit. Covers the usage
  errors for every command, the quality floor on `rip id`, config path output,
  the --version flag, and that applyQualityOverride reaches all six sources and
  clamps only the two with a floor. cmd/rip coverage 17% -> 26%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
JorenandClaude Opus 5 committed 2026-10-03 17:26:24 +02:00
1 parent b5d4d5688d
commit 26fb466afc
13 files changed
+447 -211

No files matched your search

+248
View File
@@ -0,0 +1,248 @@
package main
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
"streamrip-go/internal/config"
)
// testConfig returns a config rooted in a temp dir, so command functions can
// run without touching the user's real config or download folder.
func testConfig(t *testing.T) *config.Config {
t.Helper()
tmp := t.TempDir()
d := config.DefaultConfigData()
d.Downloads.Folder = filepath.Join(tmp, "downloads")
d.Database.DownloadsEnabled = false
d.Database.FailedDownloadsEnabled = false
d.CLI.ProgressBars = false
return &config.Config{Path: filepath.Join(tmp, "config.toml"), File: d, Session: d}
}
// The command functions became testable when main() was split into run() int
// plus one function per command; previously they were arms of a 1196-line
// switch that called os.Exit directly.
func TestCommandsRejectMissingArguments(t *testing.T) {
tests := []struct {
name string
run func(ctx context.Context, cfg *config.Config, gopts globalOptions, args []string) int
args []string
}{
{"url with no args", cmdURL, nil},
{"file with no args", cmdFile, nil},
{"config with no args", cmdConfig, nil},
{"database with no args", cmdDatabase, nil},
{"database without browse", cmdDatabase, []string{"downloads"}},
{"database browse without table", cmdDatabase, []string{"browse"}},
{"id with too few args", cmdID, []string{"qobuz", "album"}},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := tc.run(context.Background(), testConfig(t), globalOptions{}, tc.args)
if got != 2 {
t.Fatalf("exit code = %d, want 2 (usage error)", got)
}
})
}
}
func TestCmdDatabaseRejectsUnknownTable(t *testing.T) {
if got := cmdDatabase(context.Background(), testConfig(t), globalOptions{}, []string{"browse", "bogus"}); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
}
func TestCmdConfigRejectsUnknownSubcommand(t *testing.T) {
if got := cmdConfig(context.Background(), testConfig(t), globalOptions{}, []string{"bogus"}); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
}
func TestCmdConfigPathPrintsPath(t *testing.T) {
cfg := testConfig(t)
out := captureStdout(t, func() {
if got := cmdConfig(context.Background(), cfg, globalOptions{}, []string{"path"}); got != 0 {
t.Errorf("exit code = %d, want 0", got)
}
})
if !strings.Contains(out, cfg.Path) {
t.Fatalf("output %q does not contain the config path %q", out, cfg.Path)
}
}
func TestCmdConfigPathRejectsExtraArgument(t *testing.T) {
if got := cmdConfig(context.Background(), testConfig(t), globalOptions{}, []string{"path", "extra"}); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
}
func TestCmdFileReportsMissingFile(t *testing.T) {
missing := filepath.Join(t.TempDir(), "nope.txt")
if got := cmdFile(context.Background(), testConfig(t), globalOptions{}, []string{missing}); got != 1 {
t.Fatalf("exit code = %d, want 1 (read error)", got)
}
}
func TestCmdFileRejectsUnknownOption(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "urls.txt")
if err := os.WriteFile(path, []byte("https://play.qobuz.com/album/abc\n"), 0o600); err != nil {
t.Fatal(err)
}
if got := cmdFile(context.Background(), testConfig(t), globalOptions{}, []string{path, "--bogus"}); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
}
func TestCmdURLRejectsInvalidURL(t *testing.T) {
// Nothing resolvable is queued, so this reports "nothing to rip" and exits 0.
out := captureStdout(t, func() {
if got := cmdURL(context.Background(), testConfig(t), globalOptions{}, []string{"not-a-url"}); got != 0 {
t.Errorf("exit code = %d, want 0", got)
}
})
if !strings.Contains(out, "nothing to rip") {
t.Fatalf("output %q should report that nothing was queued", out)
}
}
func TestCmdIDRejectsQualityBelowSourceFloor(t *testing.T) {
// Qobuz has no quality 0.
if got := cmdID(context.Background(), testConfig(t), globalOptions{}, []string{"qobuz", "album", "abc", "0"}); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
if got := cmdID(context.Background(), testConfig(t), globalOptions{}, []string{"beatport", "album", "abc", "0"}); got != 2 {
t.Fatalf("beatport exit code = %d, want 2", got)
}
}
func TestCmdIDRejectsUnknownOption(t *testing.T) {
if got := cmdID(context.Background(), testConfig(t), globalOptions{}, []string{"qobuz", "album", "abc", "--bogus"}); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
}
func TestSmokeCommandsRejectMissingArguments(t *testing.T) {
tests := []struct {
name string
run func(ctx context.Context, cfg *config.Config, gopts globalOptions, args []string) int
args []string
}{
{"soundcloud", cmdSoundcloudSmoke, nil},
{"qobuz", cmdQobuzSmoke, nil},
{"qobuz convert", cmdQobuzConvertRipSmoke, []string{"id"}},
{"qobuz search", cmdQobuzSearchSmoke, nil},
{"tidal search", cmdTidalSearchSmoke, nil},
{"tidal metadata", cmdTidalMetadataSmoke, []string{"track"}},
{"tidal video", cmdTidalVideoSmoke, nil},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := tc.run(context.Background(), testConfig(t), globalOptions{}, tc.args); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
})
}
}
// captureStdout swaps os.Stdout for a pipe while fn runs. The commands print
// user-facing output with fmt.Println rather than taking a writer.
func captureStdout(t *testing.T, fn func()) string {
t.Helper()
orig := os.Stdout
r, w, err := os.Pipe()
if err != nil {
t.Fatal(err)
}
os.Stdout = w
done := make(chan string, 1)
go func() {
var sb strings.Builder
buf := make([]byte, 4096)
for {
n, readErr := r.Read(buf)
if n > 0 {
sb.Write(buf[:n])
}
if readErr != nil {
break
}
}
done <- sb.String()
}()
fn()
_ = w.Close()
os.Stdout = orig
out := <-done
_ = r.Close()
return out
}
func TestVersionOutput(t *testing.T) {
var sb strings.Builder
printVersion(&sb)
out := sb.String()
if !strings.HasPrefix(out, "rip ") {
t.Errorf("version output should start with \"rip \": %q", out)
}
for _, want := range []string{"go:", "target:"} {
if !strings.Contains(out, want) {
t.Errorf("version output missing %q: %q", want, out)
}
}
}
func TestParseGlobalArgsVersionFlag(t *testing.T) {
for _, flag := range []string{"--version", "-V"} {
opts, err := parseGlobalArgs([]string{flag})
if err != nil {
t.Fatalf("parseGlobalArgs(%q) error = %v", flag, err)
}
if !opts.version {
t.Errorf("parseGlobalArgs(%q) did not set version", flag)
}
}
}
func TestApplyQualityOverrideCoversEverySource(t *testing.T) {
cfg := testConfig(t)
applyQualityOverride(cfg, 4)
got := map[string]int{
"qobuz": cfg.Session.Qobuz.Quality,
"tidal": cfg.Session.Tidal.Quality,
"deezer": cfg.Session.Deezer.Quality,
"soundcloud": cfg.Session.Soundcloud.Quality,
"yandex": cfg.Session.Yandex.Quality,
"beatport": cfg.Session.Beatport.Quality,
}
for source, q := range got {
if q != 4 {
t.Errorf("%s quality = %d, want 4; -q must reach every source", source, q)
}
}
}
func TestApplyQualityOverrideClampsSourceFloor(t *testing.T) {
cfg := testConfig(t)
applyQualityOverride(cfg, 0)
if cfg.Session.Qobuz.Quality != 1 {
t.Errorf("qobuz quality = %d, want it clamped to 1", cfg.Session.Qobuz.Quality)
}
if cfg.Session.Beatport.Quality != 1 {
t.Errorf("beatport quality = %d, want it clamped to 1", cfg.Session.Beatport.Quality)
}
if cfg.Session.Tidal.Quality != 0 {
t.Errorf("tidal quality = %d, want 0 preserved", cfg.Session.Tidal.Quality)
}
}