Refactor: DRY/KISS cleanup and add test suite by icco · Pull Request #83 · icco/etu

icco · 2026-02-11T16:24:43Z

Summary

Extract shared helpers (parsePaths, detectMIME, notesToPosts, formatDuration, isInteractive, printPostPlain) to eliminate duplicated code across main.go, client/client.go, and client/grpc.go
Simplify list.go: named constants for magic numbers, cleaner style selection in render, use math.Min with named constants
Add 28 tests across 4 test files covering all pure/unit-testable functions in the codebase

Test plan

go build ./... passes
go test ./... passes (28 tests, 0 failures)
Manual smoke test of etu create, etu list, etu search, etu last, etu random, etu timesince

🤖 Generated with Claude Code

- Extract parsePaths, formatDuration, isInteractive, printPostPlain helpers in main.go - Extract detectMIME helper in client/client.go to deduplicate MIME detection - Extract notesToPosts helper in client/grpc.go to deduplicate note conversion - Simplify list.go: use builtin min(), named constants, cleaner style selection - Add initial main_test.go with tests for parsePaths, truncate, formatDuration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- client/client_test.go: detectMIME, LoadImageUploads, LoadAudioUploads, Validate, cache round-trip - client/grpc_test.go: apiKeyCreds, noteToPost, notesToPosts, getGRPCClients - client/config_test.go: ConfigDir, ConfigPath, SaveConfig, loadConfigFromFile, LoadConfig with env - Revert builtin min() to math.Min per preference for standard library Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

On Linux, os.UserConfigDir checks XDG_CONFIG_HOME before HOME. Setting only HOME left tests sharing the same config/cache dir, causing cross-test pollution in CI. Add setTestHome helper that clears XDG_CONFIG_HOME so $HOME/.config is used consistently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Tests already run in the dedicated Test job. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Refactors the CLI and client code to reduce duplication (DRY/KISS) by extracting shared helper functions, while adding a new unit test suite to cover pure/unit-testable logic.

Changes:

Extracts shared helpers into reusable functions (e.g., path parsing, duration formatting, MIME detection, proto-note conversion).
Simplifies list rendering/layout logic by replacing magic numbers with named constants and streamlining style selection.
Adds new unit tests across main and client packages; updates dependencies accordingly.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`main.go`	Replaces duplicated path parsing and duration formatting with helpers; centralizes non-interactive output behavior.
`list.go`	Introduces named constants for list sizing and simplifies render style selection and height calculation.
`client/client.go`	Extracts MIME detection helper and reuses `notesToPosts` for consistent proto→model conversion.
`client/grpc.go`	Adds `notesToPosts` helper and ensures nil notes are filtered via `noteToPost`.
`main_test.go`	Adds tests for `parsePaths`, `truncate`, and `formatDuration`.
`client/helpers_test.go`	Adds a shared test helper for isolating config/cache paths.
`client/client_test.go`	Adds tests for validation, MIME detection, uploads, and cache round-trips.
`client/config_test.go`	Adds tests for config dir/path behavior and env/file loading logic.
`client/grpc_test.go`	Adds tests for API key creds, proto→post conversion, and gRPC client validation.
`go.mod` / `go.sum`	Bumps backend dependency and makes protobuf a direct dependency (used by tests).
`.github/workflows/ci.yml`	Removes redundant test step from the build job (tests still run in the dedicated test job).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T16:37:27Z

main_test.go

+		{"empty", "", 0},
+		{"single path", "/tmp/test.jpg", 1},
+		{"multiple paths", "/tmp/a.jpg\n/tmp/b.png", 2},
+		{"blank lines", "/tmp/a.jpg\n\n/tmp/b.png\n", 2},
+		{"quoted paths", `"/tmp/my file.jpg"`, 1},
+		{"single quoted", `'/tmp/my file.jpg'`, 1},
+		{"whitespace only", "   \n  \n  ", 0},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := parsePaths(tt.input)
+			if len(got) != tt.want {
+				t.Errorf("parsePaths(%q) returned %d paths, want %d", tt.input, len(got), tt.want)
+			}
+			for _, p := range got {
+				if !filepath.IsAbs(p) {
+					t.Errorf("parsePaths(%q) returned non-absolute path: %s", tt.input, p)
+				}
+			}
+		})
+	}
+}
+
+func TestParsePathsStripsQuotes(t *testing.T) {
+	got := parsePaths(`"/tmp/test.jpg"`)
+	if len(got) != 1 {
+		t.Fatalf("expected 1 path, got %d", len(got))
+	}
+	if got[0] != "/tmp/test.jpg" {
+		t.Errorf("expected /tmp/test.jpg, got %s", got[0])
+	}


These tests hard-code Unix-style absolute paths (e.g. /tmp/...) and TestParsePathsStripsQuotes asserts the exact string "/tmp/test.jpg". Since this repo ships Windows/darwin builds, this makes the test suite OS-dependent and likely to fail on non-Unix platforms. Consider generating paths with t.TempDir()/filepath.Join and asserting properties (IsAbs, base name, cleaned path) rather than a fixed /tmp path.

Copilot · 2026-02-11T16:37:28Z

client/helpers_test.go

+import "testing"
+
+// setTestHome isolates config/cache paths to a temp directory.
+// On Linux, os.UserConfigDir checks XDG_CONFIG_HOME before HOME,
+// so we must clear it to ensure $HOME/.config is used.
+func setTestHome(t *testing.T) {
+	t.Helper()
+	t.Setenv("HOME", t.TempDir())
+	t.Setenv("XDG_CONFIG_HOME", "")


setTestHome currently relies on setting HOME and clearing XDG_CONFIG_HOME, but os.UserConfigDir does not use HOME on all platforms (notably Windows uses APPDATA/LOCALAPPDATA). Given the project builds for windows/darwin/linux, this helper won’t reliably isolate config paths across OSes. Consider setting XDG_CONFIG_HOME to a temp dir (instead of clearing it) and also setting APPDATA/LOCALAPPDATA (and HOME) as appropriate per runtime.GOOS.

Suggested change

import "testing"

// setTestHome isolates config/cache paths to a temp directory.

// On Linux, os.UserConfigDir checks XDG_CONFIG_HOME before HOME,

// so we must clear it to ensure $HOME/.config is used.

func setTestHome(t *testing.T) {

t.Helper()

t.Setenv("HOME", t.TempDir())

t.Setenv("XDG_CONFIG_HOME", "")

import (

"runtime"

"testing"

)

// setTestHome isolates config/cache paths to a temp directory.

// On Linux, os.UserConfigDir checks XDG_CONFIG_HOME before HOME,

// and on Windows it uses APPDATA/LOCALAPPDATA, so we must ensure

// all relevant environment variables point to a temporary directory.

func setTestHome(t *testing.T) {

t.Helper()

dir := t.TempDir()

t.Setenv("HOME", dir)

t.Setenv("XDG_CONFIG_HOME", dir)

if runtime.GOOS == "windows" {

t.Setenv("APPDATA", dir)

t.Setenv("LOCALAPPDATA", dir)

}

Copilot · 2026-02-11T16:37:28Z

client/client_test.go

+	t.Run("nonexistent file", func(t *testing.T) {
+		_, err := LoadImageUploads([]string{"/nonexistent/file.png"})
+		if err == nil {
+			t.Error("expected error for nonexistent file")
+		}
+	})


This test uses a hard-coded Unix path (/nonexistent/file.png). On Windows or environments where that path could exist, the test could behave unexpectedly. Prefer using a guaranteed-missing path under t.TempDir() (e.g. join temp dir with a filename that you do not create).

client/client_test.go

icco and others added 4 commits February 11, 2026 16:15

fix: clamp negative durations in formatDuration to 0.0h

36d8d67

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

icco requested a review from Copilot February 11, 2026 16:32

icco and others added 2 commits February 11, 2026 16:32

ci: remove duplicate test step from Build job

2988543

Tests already run in the dedicated Test job. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chore: update go deps

52aca48

Copilot started reviewing on behalf of icco February 11, 2026 16:33 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

icco merged commit 3739225 into main Feb 11, 2026
7 checks passed

icco deleted the test branch February 11, 2026 17:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor: DRY/KISS cleanup and add test suite#83

Refactor: DRY/KISS cleanup and add test suite#83
icco merged 6 commits intomainfrom
test

icco commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

Uh oh!

Conversation

icco commented Feb 11, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments