tests: close files explicitly and replace tsp-tcp test init/panics with helpers #11

Closed
opened 2026-07-21 17:20:20 +02:00 by heiko · 1 comment
Owner

Problem

Test resource/setup handling is inconsistent:

  • normalize_test.go opens each fixture but never closes it.
  • internal/tsp-tcp/tcp_test.go loads fixtures from init, panics on errors, and defers file closes inside a loop. Failures occur before a test can report useful context, and all defers wait until the entire initializer returns.

Checklist: #35 (avoid defers accumulating in loops), #48 (avoid panics for recoverable setup failures), #79 (close opened resources), #90 (use test helpers/Cleanup).

Proposed fix

  • Register t.Cleanup immediately after each os.Open in normalization tests, or use os.ReadFile where a streaming reader is not under test.
  • Load TCP fixtures from a helper taking *testing.T, use t.Helper, require.NoError, and return typed request/response fixtures.
  • Avoid package init for test data.
  • Keep fixture parsing local to the tests that need it or use a clearly failing TestMain only if shared setup is necessary.

Acceptance criteria

  • No test file descriptors are leaked.
  • Missing/corrupt fixture errors identify the test and path rather than panicking during package initialization.
  • Tests remain parallel-safe and pass with race/shuffle enabled.

Reviewed against 948b7a9 on master (Go 1.26.2).

## Problem Test resource/setup handling is inconsistent: - `normalize_test.go` opens each fixture but never closes it. - `internal/tsp-tcp/tcp_test.go` loads fixtures from `init`, panics on errors, and defers file closes inside a loop. Failures occur before a test can report useful context, and all defers wait until the entire initializer returns. Checklist: #35 (avoid defers accumulating in loops), #48 (avoid panics for recoverable setup failures), #79 (close opened resources), #90 (use test helpers/Cleanup). ## Proposed fix - Register `t.Cleanup` immediately after each `os.Open` in normalization tests, or use `os.ReadFile` where a streaming reader is not under test. - Load TCP fixtures from a helper taking `*testing.T`, use `t.Helper`, `require.NoError`, and return typed request/response fixtures. - Avoid package `init` for test data. - Keep fixture parsing local to the tests that need it or use a clearly failing `TestMain` only if shared setup is necessary. ## Acceptance criteria - No test file descriptors are leaked. - Missing/corrupt fixture errors identify the test and path rather than panicking during package initialization. - Tests remain parallel-safe and pass with race/shuffle enabled. Reviewed against `948b7a9` on `master` (Go 1.26.2).
Author
Owner

Fixture setup no longer uses init/panic, and test file resources are closed explicitly.

Closing commit: 5afb95a236d8

Fixture setup no longer uses init/panic, and test file resources are closed explicitly. Closing commit: [`5afb95a236d8`](https://forgejo.schlittermann.de/heiko/mail-seal/commit/5afb95a236d82e595096426125cf592840f9be62)
heiko closed this issue 2026-07-21 23:07:53 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
heiko/mailseal#11
No description provided.