Skip to content

chore: cleanup second precision and test setup for file backend - #1429

Merged
ferhatelmas merged 1 commit into
masterfrom
ferhat/file-cleanup
Sep 22, 2026
Merged

ferhatelmas merged 1 commit into
masterfrom
ferhat/file-cleanup

Conversation

@ferhatelmas

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

refactor

What is the current behavior?

Second truncation for HTTP date is duplicated.
Setup is duplicated.

What is the new behavior?

Extract helpers.

Additional context

Related to #1407, #1422

Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
@ferhatelmas
ferhatelmas requested a review from a team as a code owner September 22, 2026 12:37
Copilot AI lite review requested due to automatic review settings September 22, 2026 12:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 35728327056

Coverage decreased (-0.005%) to 83.392%

Details

  • Coverage decreased (-0.005%) from the base build.
  • Patch coverage: 6 of 6 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 14201
Covered Lines: 12270
Line Coverage: 86.4%
Relevant Branches: 8697
Covered Branches: 6825
Branch Coverage: 78.48%
Branches in Coverage %: Yes
Coverage Strength: 3090.75 hits per line

💛 - Coveralls

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good — this is a mechanical refactor with no behavior change.

What was reviewed:

  • The new toSeconds helper drops the old explicit invalid-date guard; confirmed this is safe because <=/> comparisons against NaN always evaluate to false in both the read-path and copy-precondition checks, reproducing the old "undefined skip" behavior exactly. The existing tests (ignores an invalid if-modified-since date, ignores invalid precondition dates) still exercise this and pass.
  • The file.test.ts refactor introducing useFileBackend(): verified it preserves setup/teardown semantics — only STORAGE_FILE_BACKEND_PATH is stubbed, which is fine since config resolution treats FILE_STORAGE_BACKEND_PATH as a fallback only — and no test cases were dropped, just de-duplicated.
Extended reasoning...

The diff touches only src/storage/backend/file.ts (extracting a shared toSeconds helper for HTTP-date precondition comparisons) and its test file (a mechanical setup/teardown de-duplication via a useFileBackend() helper, no test cases added or removed). No security-sensitive surface beyond precondition/date-comparison logic, which I traced by hand: removing the old undefined-guard in toSeconds is behavior-preserving because NaN comparisons resolve to false either way, and this is backed by pre-existing tests covering invalid If-Modified-Since and invalid copy-precondition dates. The test refactor's env-var handling (only stubbing STORAGE_FILE_BACKEND_PATH) is also safe since it takes precedence over the FILE_STORAGE_BACKEND_PATH fallback in config.ts. No bug hunter findings, no open third-party objections, and the change is small, self-contained, and verified equivalent.

@ferhatelmas
ferhatelmas merged commit 7bd0ffe into master Sep 22, 2026
40 of 42 checks passed
@ferhatelmas
ferhatelmas deleted the ferhat/file-cleanup branch September 22, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants