Skip to content

Improve fragmentation package test coverage#114

Merged
Vladyslav-Kuksiuk merged 5 commits into
masterfrom
improve-fragmentation-test-coverage
Jul 13, 2026
Merged

Improve fragmentation package test coverage#114
Vladyslav-Kuksiuk merged 5 commits into
masterfrom
improve-fragmentation-test-coverage

Conversation

@Vladyslav-Kuksiuk

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

This PR improves fragmentation and files packages test coverage.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk self-assigned this Jul 10, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.12%. Comparing base (62132b0) to head (5e62e7e).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
+ Coverage   82.37%   90.12%   +7.74%     
==========================================
  Files          43       43              
  Lines        2951     2946       -5     
==========================================
+ Hits         2431     2655     +224     
+ Misses        390      232     -158     
+ Partials      130       59      -71     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk marked this pull request as ready for review July 13, 2026 08:48
Expect(cache.order.Len()).Should(Equal(0))
})

It("should discard an invalid usage-order entry", func() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This spec (and the "usage order is empty" one above) exercises branches in cache.evictOldest() — the Front() == nil guard and the !isKey type-assertion fallback — that are unreachable in production: order only ever receives K-typed keys via storeLoadedPushBack(key), and evictOldest is only ever called right after a PushBack, so the list is never empty and the assertion never fails. To cover them the test has to hand-craft an impossible state (order.PushBack("not an int key")). Worth considering whether the defensive branches are dead code that should be dropped rather than propped up with tests.

Comment thread files/files.go
// It points to os.Stat in production. Tests replace it to exercise error
// handling after glob resolution, which otherwise depends on filesystem races
// between finding a path and inspecting it.
var statPath = os.Stat

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor / design: this (and makeAbsolutePath in fragmentation.go:63) introduces a mutable package-global into production code purely as a test seam. It works and is well-documented, and since Ginkgo parallelism is process-based the global mutation is safe. Just flagging the tradeoff for the record — an alternative that keeps production surface immutable would be injecting the stat/abs function through the existing struct (e.g. Resolver/Fragmentation) rather than a reassignable global. Not blocking.

@Oleg-Melnik Oleg-Melnik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Vladyslav-Kuksiuk LGTM with comments to address.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk merged commit 269cfd0 into master Jul 13, 2026
5 checks passed
@Vladyslav-Kuksiuk Vladyslav-Kuksiuk deleted the improve-fragmentation-test-coverage branch July 13, 2026 10:58
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.

2 participants