Skip to content

refactor: extract textFromContentStream into handler methods, remove dead code, add reference snapshot tests #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ymettier merged 4 commits into from
Jul 30, 2026
Merged

refactor: extract textFromContentStream into handler methods, remove dead code, add reference snapshot tests #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8f37dc
docs: add nolint exception for fontwidths_std.go
ymettier Jul 30, 2026
2915983
refactor: extract textFromContentStream into handler methods
ymettier Jul 30, 2026
d90a52e
test: add reference snapshot tests for PDF text extraction
ymettier Jul 30, 2026
8b6476e
refactor: remove dead FromCtx/WithCtx, extract withArgs helper in tests
ymettier Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Fileganizer is a Go CLI tool that processes documents through a pipeline: text e
- Group imports: stdlib first, third-party second, internal (`fileganizer/...`) last.
- Flags (like `-c` or `-f`) are never constants. When the linter complains, add `//nolint`.
- No global or function-scoped `//nolint`. Only line-scoped `//nolint` is allowed.
- Exception: `pdftotext/fontwidths_std.go` may use declaration-scoped `//nolint:gochecknoglobals,dupl` — the font metric data is inherently repetitive and the global map is intentional.
- Keep the whole code simple and stupid (KISS). No over-engineering, no unnecessary abstractions.
- Copyright header on every source file. For `.go` files:
```go
Expand Down
17 changes: 0 additions & 17 deletions logger/logger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package logger

import (
"context"
"io"
"log/slog"
"os"
Expand All @@ -14,8 +13,6 @@ import (
"gopkg.in/natefinch/lumberjack.v2"
)

type ctxKey struct{}

var (
mu sync.RWMutex
logger *Logger
Expand Down Expand Up @@ -144,17 +141,3 @@ func Reset(opts *LogOptions) {
}
logger = newLogger(opts)
}

// FromCtx returns the Logger associated with the ctx. If no logger
// is associated, the default logger is returned.
func FromCtx(ctx context.Context) *Logger {
if l, ok := ctx.Value(ctxKey{}).(*Logger); ok {
return l
}
return Get()
}

// WithCtx returns a copy of ctx with the Logger attached.
func WithCtx(ctx context.Context, l *Logger) context.Context {
return context.WithValue(ctx, ctxKey{}, l)
}
14 changes: 0 additions & 14 deletions logger/logger_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,6 @@ func TestResetClosesPreviousWriter(t *testing.T) {
assert.NotNil(t, l2.closer)
}

func TestWithCtx_FromCtx(t *testing.T) {
l := newLogger(nil)
ctx := WithCtx(context.Background(), l)

extracted := FromCtx(ctx)
assert.Same(t, l, extracted)
}

func TestFromCtx_NoLogger(t *testing.T) {
resetGlobal()
l := FromCtx(context.Background())
assert.NotNil(t, l) // returns the default logger
}

func TestNewLogger_InvalidLevelEnv(t *testing.T) {
os.Setenv("FILEGANIZER_LOGGING_LEVEL", "BOGUS")
defer os.Unsetenv("FILEGANIZER_LOGGING_LEVEL")
Expand Down
106 changes: 26 additions & 80 deletions main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,33 @@ import (
"fileganizer/testutil"
)

func TestFileInvoice(t *testing.T) {
func withArgs(t *testing.T, args ...string) {
t.Helper()
oldArgs := os.Args
defer func() { os.Args = oldArgs }() // os.Args is a "global variable", so keep the state from before the test, and restore it after.
os.Args = append([]string{"./fileganizer"}, args...)
t.Cleanup(func() { os.Args = oldArgs })
}

os.Args = []string{"./fileganizer", "-c", "testdata/config.invoice.yaml", "-f", "testdata/invoice.txt"} //nolint:goconst
func TestFileInvoice(t *testing.T) {
withArgs(t, "-c", "testdata/config.invoice.yaml", "-f", "testdata/invoice.txt")

output, err := run()
assert.Nil(t, err)
assert.Contains(t, output, "Invoice Summary\n date: 2014-03-27\n number: 001\n")
}

func TestFileInvoiceEnv(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Setenv("SOMEVAR", "magic")
defer os.Unsetenv("SOMEVAR")
os.Args = []string{"./fileganizer", "-c", "testdata/config.invoice-env.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.invoice-env.yaml", "-f", "testdata/invoice.txt")

output, err := run()
assert.Nil(t, err)
assert.Contains(t, output, "Invoice magic Summary\n date: 2014-03-27\n number: 001\n")
}

func TestBuiltinExtractUnsupportedMIME(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.broken.mime.yaml", "-f", "testdata/minimal.wav"}
withArgs(t, "-c", "testdata/config.broken.mime.yaml", "-f", "testdata/minimal.wav")

_, err := run()
assert.Error(t, err)
Expand All @@ -62,34 +60,25 @@ func TestDetectFileType_ReadError(t *testing.T) {
}

func TestPDFBuiltinExtractor(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.pdfBuiltin.yaml", "-f", "pdftotext/testdata/forged-invoice.pdf"}
withArgs(t, "-c", "testdata/config.pdfBuiltin.yaml", "-f", "pdftotext/testdata/forged-invoice.pdf")

output, err := run()
assert.Nil(t, err)
assert.Contains(t, output, "Invoice INV-2024-001")
}

func TestPDFBuiltinExtractorEnv(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Setenv("COMPANY", "ACME Corp")
defer os.Unsetenv("COMPANY")
os.Args = []string{"./fileganizer", "-c", "testdata/config.pdfBuiltinEnv.yaml", "-f", "pdftotext/testdata/forged-invoice.pdf"}
withArgs(t, "-c", "testdata/config.pdfBuiltinEnv.yaml", "-f", "pdftotext/testdata/forged-invoice.pdf")

output, err := run()
assert.Nil(t, err)
assert.Contains(t, output, "Invoice INV-2024-001 from ACME Corp")
}

func TestFileNonMatchingPattern(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.invoice-nomatch.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.invoice-nomatch.yaml", "-f", "testdata/invoice.txt")

output, err := run()
assert.Nil(t, err)
Expand All @@ -98,105 +87,75 @@ func TestFileNonMatchingPattern(t *testing.T) {
}

func TestFileBrokenTemplate(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.broken.template.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.broken.template.yaml", "-f", "testdata/invoice.txt")

output, err := run()
assert.Nil(t, err)
assert.Contains(t, output, "Invoice Summary\n date: 2014-03-27\n number: 001\n")
}

func TestFileRunMode(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.invoice-run.yaml", "-f", "testdata/invoice.txt", "-r"}
withArgs(t, "-c", "testdata/config.invoice-run.yaml", "-f", "testdata/invoice.txt", "-r")

output, err := run()
assert.Nil(t, err)
assert.Contains(t, output, "run mode works")
}

func TestFileFrenchMonths(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.invoice-french.yaml", "-f", "testdata/invoice-french.txt"}
withArgs(t, "-c", "testdata/config.invoice-french.yaml", "-f", "testdata/invoice-french.txt")

output, err := run()
assert.Nil(t, err)
assert.Contains(t, output, "08-27-2014")
}

func TestRunMissingConfigFile(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/nonexistent.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/nonexistent.yaml", "-f", "testdata/invoice.txt")

_, err := run()
assert.Error(t, err)
assert.Contains(t, err.Error(), "nonexistent.yaml")
}

func TestRunMissingInputFile(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.invoice.yaml", "-f", "testdata/nonexistent.txt"}
withArgs(t, "-c", "testdata/config.invoice.yaml", "-f", "testdata/nonexistent.txt")

_, err := run()
assert.Error(t, err)
}

func TestRunTextOutputFlag(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.invoice.yaml", "-f", "testdata/invoice.txt", "-t"}
withArgs(t, "-c", "testdata/config.invoice.yaml", "-f", "testdata/invoice.txt", "-t")

output, err := run()
assert.NoError(t, err)
assert.Contains(t, output, "Invoice")
}

func TestRunBrokenGrokPattern(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.broken.grok.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.broken.grok.yaml", "-f", "testdata/invoice.txt")

_, err := run()
assert.Error(t, err)
}

func TestFileRunModeFails(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.broken.run.yaml", "-f", "testdata/invoice.txt", "-r"}
withArgs(t, "-c", "testdata/config.broken.run.yaml", "-f", "testdata/invoice.txt", "-r")

_, err := run()
assert.Error(t, err)
}

func TestRunBrokenGrokPatternDefinition(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.broken.regex.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.broken.regex.yaml", "-f", "testdata/invoice.txt")

_, err := run()
assert.Error(t, err)
}

func TestRunVersionFlag(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-V"}
withArgs(t, "-V")

_, err := run()
assert.NoError(t, err)
Expand All @@ -217,10 +176,7 @@ func TestBSBStatements(t *testing.T) {

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.bsb.yaml", "-f", tc.pdf}
withArgs(t, "-c", "testdata/config.bsb.yaml", "-f", tc.pdf)

output, err := run()
assert.Nil(t, err)
Expand All @@ -230,21 +186,15 @@ func TestBSBStatements(t *testing.T) {
}

func TestExtractTextMimeNotInConfig(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.broken.mime.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.broken.mime.yaml", "-f", "testdata/invoice.txt")

_, err := run()
assert.Error(t, err)
assert.Contains(t, err.Error(), "no textExtractor configured for MIME type")
}

func TestProcessFileDescriptionsNoMatch(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

os.Args = []string{"./fileganizer", "-c", "testdata/config.nomatch.yaml", "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.nomatch.yaml", "-f", "testdata/invoice.txt")

output, err := run()
assert.Nil(t, err)
Expand All @@ -264,11 +214,7 @@ func TestExtractTextUnsupportedType(t *testing.T) {
}

func TestFileInvoiceWithCatCommand(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()

configFile := "testdata/config.invoice-cat.yaml"
os.Args = []string{"./fileganizer", "-c", configFile, "-f", "testdata/invoice.txt"}
withArgs(t, "-c", "testdata/config.invoice-cat.yaml", "-f", "testdata/invoice.txt")

output, err := run()
assert.Nil(t, err)
Expand Down
Loading