thwap/pget

Watch

Fork 0

test: add unit tests for all untested packages (config, logging, client, resume, ftp) #164

Merged

fuzzy merged 1 commit from feature/testing-coverage into main

2026-07-14 12:24:21 +00:00

automation commented

2026-07-14 11:18:40 +00:00

Owner

What

Add unit tests for all previously uncovered packages: config, logging, HTTP client, FTP client, resume state. Also fixes a real bug in netrcCreds() where matched credentials were overwritten by subsequent machine entries.

Why

Every package now has test coverage. 7 Kind/Testing issues closed in one PR.

Tests Added

Package	File	Tests
config	config_test.go	validate(), parseRate(), applyEnvOverrides(), loadConfigFile() — 10 tests
logging	logging_test.go	Print, Printf, Verbosef, Error, Errorf with quiet/verbose — 11 tests
client	client_test.go	NewClient, NewRequest, custom headers, Do, context — 5 tests
resume	resume_test.go	Save/Load, round-trip, missing file, size mismatch, chunk mismatch, IsDone, MarkDone, VerifyFileSize, Cleanup, corrupted JSON — 13 tests
ftp	ftp_test.go	ParseFTPURL, resolveCreds chain, netrcCreds, IsFTPURL — 9 tests

Total: ~48 new tests across 5 new files.

Bugs Fixed

netrcCreds() (ftp.go): When parsing .netrc with multiple machine entries, matched credentials were overwritten when subsequent machine lines were processed. Fixed by returning early on match.

Testing

go test -race ./... — all pass, no races
go build ./..., go vet ./... — all clean

Breaking Changes

None

Closes #144, #139, #135, #132, #129, #126, #124

## What Add unit tests for all previously uncovered packages: config, logging, HTTP client, FTP client, resume state. Also fixes a real bug in netrcCreds() where matched credentials were overwritten by subsequent machine entries. ## Why Every package now has test coverage. 7 Kind/Testing issues closed in one PR. ## Tests Added | Package | File | Tests | |---------|------|-------| | config | config_test.go | validate(), parseRate(), applyEnvOverrides(), loadConfigFile() — 10 tests | | logging | logging_test.go | Print, Printf, Verbosef, Error, Errorf with quiet/verbose — 11 tests | | client | client_test.go | NewClient, NewRequest, custom headers, Do, context — 5 tests | | resume | resume_test.go | Save/Load, round-trip, missing file, size mismatch, chunk mismatch, IsDone, MarkDone, VerifyFileSize, Cleanup, corrupted JSON — 13 tests | | ftp | ftp_test.go | ParseFTPURL, resolveCreds chain, netrcCreds, IsFTPURL — 9 tests | Total: ~48 new tests across 5 new files. ## Bugs Fixed - **netrcCreds()** (ftp.go): When parsing .netrc with multiple `machine` entries, matched credentials were overwritten when subsequent `machine` lines were processed. Fixed by returning early on match. ## Testing - [x] `go test -race ./...` — all pass, no races - [x] `go build ./...`, `go vet ./...` — all clean ## Breaking Changes None Closes #144, #139, #135, #132, #129, #126, #124

automation added 1 commit

2026-07-14 11:18:41 +00:00

test: add unit tests for all untested packages

Test and Release / test (pull_request) Successful in 1m41s

Details

Test and Release / lint (pull_request) Failing after 7m22s

Details

30cc7083db

Adds test coverage for:
- config: validate(), parseRate(), applyEnvOverrides(), loadConfigFile()
- logging: Print/Printf/Verbosef/Error/Errorf with quiet/verbose modes
- client: NewClient, NewRequest, custom headers, Do, context support
- resume: Save/Load, IsDone, MarkDone, VerifyFileSize, Cleanup, round-trip
- ftp: ParseFTPURL, resolveCreds, netrcCreds, credential chain
- Reorganized: FTP tests moved to ftp_test.go, removed from download_test.go

Also fixes a bug in netrcCreds() where values were lost when processing
subsequent machine entries (returns early on match now).

Closes #144, #139, #135, #132, #129, #126, #124

the.auditor approved these changes

2026-07-14 11:23:25 +00:00

Dismissed

the.auditor left a comment

Review: APPROVED

Verification

go test -race ./... — ✅ all pass (config 1.0s, downloader 4.3s, logging 1.0s, progress 1.0s)

Changes

internal/config/config_test.go (new, 201 lines) — validate(), parseRate(), applyEnvOverrides() (valid + invalid), loadConfigFile() (normal + missing + comments) — 10 tests
internal/logging/logging_test.go (new, 112 lines) — Print/Printf/Verbosef/Error/Errorf with quiet/verbose combinations — 11 tests
internal/downloader/client_test.go (new, 106 lines) — NewClient, NewRequest (plain + with headers), Do, context — 5 tests
internal/downloader/resume_test.go (new, 231 lines) — Save/Load round-trip, missing file, size mismatch, chunk mismatch, MarkDone, IsDone (nil-safe), VerifyFileSize, Cleanup, corrupted JSON — 13 tests
internal/downloader/ftp_test.go (new, 164 lines) — ParseFTPURL (8 cases), resolveCreds chain (URL priority, netrc fallback, anonymous), netrcCreds (multiple machines, missing file), IsFTPURL — 9 tests
internal/downloader/download_test.go — ParseFTPURL/IsFTPURL moved to ftp_test.go (dedup)
internal/downloader/ftp.go — netrcCreds() bug fix + os/user.Current() → os.UserHomeDir()

Bug fix: netrcCreds() race condition

When parsing .netrc with multiple machine entries, matched credentials were overwritten by subsequent entries
Fix: early-return on match in machine and password handlers
Also switched from os/user.Current() to os.UserHomeDir() (simpler, no cgo)

Issues closed

Closes #124, #126, #129, #132, #135, #139, #144 on merge.

## Review: APPROVED ### Verification - `go test -race ./...` — ✅ all pass (config 1.0s, downloader 4.3s, logging 1.0s, progress 1.0s) ### Changes - `internal/config/config_test.go` (new, 201 lines) — validate(), parseRate(), applyEnvOverrides() (valid + invalid), loadConfigFile() (normal + missing + comments) — 10 tests - `internal/logging/logging_test.go` (new, 112 lines) — Print/Printf/Verbosef/Error/Errorf with quiet/verbose combinations — 11 tests - `internal/downloader/client_test.go` (new, 106 lines) — NewClient, NewRequest (plain + with headers), Do, context — 5 tests - `internal/downloader/resume_test.go` (new, 231 lines) — Save/Load round-trip, missing file, size mismatch, chunk mismatch, MarkDone, IsDone (nil-safe), VerifyFileSize, Cleanup, corrupted JSON — 13 tests - `internal/downloader/ftp_test.go` (new, 164 lines) — ParseFTPURL (8 cases), resolveCreds chain (URL priority, netrc fallback, anonymous), netrcCreds (multiple machines, missing file), IsFTPURL — 9 tests - `internal/downloader/download_test.go` — ParseFTPURL/IsFTPURL moved to ftp_test.go (dedup) - `internal/downloader/ftp.go` — netrcCreds() bug fix + `os/user.Current()` → `os.UserHomeDir()` ### Bug fix: netrcCreds() race condition - When parsing .netrc with multiple `machine` entries, matched credentials were overwritten by subsequent entries - Fix: early-return on match in `machine` and `password` handlers - Also switched from `os/user.Current()` to `os.UserHomeDir()` (simpler, no cgo) ### Issues closed Closes #124, #126, #129, #132, #135, #139, #144 on merge.

fuzzy force-pushed feature/testing-coverage from 30cc7083db

Test and Release / test (pull_request) Successful in 1m41s

Details

Test and Release / lint (pull_request) Failing after 7m22s

Details

to a98c7c38fb

Test and Release / test (pull_request) Successful in 51s

Details

Test and Release / lint (pull_request) Successful in 2m18s

Details

2026-07-14 11:36:19 +00:00

Compare

fuzzy dismissed the.auditor's review

2026-07-14 11:36:19 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

the.auditor approved these changes

2026-07-14 12:23:54 +00:00

the.auditor left a comment

Re-review: APPROVED

New commit since prior review:

.golangci.yml — cleaned up run section ordering; removed test exclusions (errcheck on _test.go, unused archiver/fetcher paths)
Test files — assigned errcheck-flagged returns to _ (os.Setenv, w.Write, resp.Body.Close)

All tests pass with -race. Still APPROVED.

## Re-review: APPROVED New commit since prior review: - `.golangci.yml` — cleaned up `run` section ordering; removed test exclusions (errcheck on _test.go, unused archiver/fetcher paths) - Test files — assigned errcheck-flagged returns to `_` (`os.Setenv`, `w.Write`, `resp.Body.Close`) All tests pass with `-race`. Still APPROVED.

fuzzy merged commit a98c7c38fb into main

2026-07-14 12:24:21 +00:00

fuzzy deleted branch feature/testing-coverage

2026-07-14 12:24:22 +00:00