Skip to content

test(storage): cover disk storage initialization#4195

Open
piotr-roslaniec wants to merge 1 commit into
mainfrom
test/storage-initialization
Open

test(storage): cover disk storage initialization#4195
piotr-roslaniec wants to merge 1 commit into
mainfrom
test/storage-initialization

Conversation

@piotr-roslaniec

@piotr-roslaniec piotr-roslaniec commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

What

Adds a test suite for pkg/storage, which previously had 0% coverage. Raises package coverage to ~88%.

The tests focus on the behavior that gives the package its reason to exist: separating sensitive keystore data (whose loss is "a serious protocol violation" per the code) from restart-recoverable work data into two sibling directories, and rooting each persistence handle under the correct one.

Covered:

  • Initialize creates the keystore and work directory layout under the configured root.
  • Restart idempotency (Initialize re-run against existing directories does not error).
  • Error path when the storage root's parent is missing (EnsureDirectoryExists uses single-level os.Mkdir).
  • Key store vs. work persistence each land under the correct root and never leak into the other's directory (this is the assertion that would catch a broken separation).
  • Nested-target error path (missing intermediate parent).

Verification

go test ./pkg/storage/ passes (6 tests); gofmt -l, go vet, and staticcheck -SA1019 (the client-lint CI gate) all clean.

Note for reviewers: the test file is force-added

.gitignore has an unanchored storage/ rule (added in 2026, intended for the client's runtime storage directory) that also matches the pkg/storage/ source package. storage.go predates that rule (added 2022) and is grandfathered in as tracked, so it wasn't caught; a brand-new file here is. The file was added with git add -f and is now tracked normally (CI checks out tracked files regardless of .gitignore).

I kept this PR surgical and did not change the shared ignore rule. If you'd prefer a permanent fix, a one-line follow-up would anchor the rule to the runtime dir (storage/ -> /storage/) or add an exception (!/pkg/storage/). Happy to open that separately.

Summary by CodeRabbit

  • Tests
    • Added coverage for storage directory initialization and persistence setup.
    • Verified creation of keystore and work directories, repeated initialization, and correct directory placement.
    • Added validation for expected errors when required parent or intermediate directories are missing.

pkg/storage had no test coverage. Add tests for the disk storage
bootstrapper, focused on the behavior that gives the package its
purpose: separating sensitive keystore data from restart-recoverable
work data into two sibling directories, and rooting each persistence
handle under the correct one.

Covers directory layout, restart idempotency, the missing-parent error
path (EnsureDirectoryExists creates a single level only), and that key
store and work persistence never leak into each other's directory.
Raises package coverage from 0% to ~88%.

The file is force-added: .gitignore's unanchored 'storage/' rule (added
2026, for the runtime storage directory) also matches the pkg/storage/
source package. storage.go predates that rule and is grandfathered in;
force-add is the surgical way to track a new file here without changing
shared ignore semantics.
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6f072fb6-9463-4d4e-8ffb-56ead162a9c5

📥 Commits

Reviewing files that changed from the base of the PR and between 5245f66 and d23908f.

📒 Files selected for processing (1)
  • pkg/storage/storage_test.go

📝 Walkthrough

Walkthrough

Adds storage package tests covering root directory creation, idempotent initialization, missing-parent errors, persistence path routing, and missing intermediate-directory errors.

Changes

Storage initialization tests

Layer / File(s) Summary
Root directory initialization
pkg/storage/storage_test.go
Adds filesystem assertion helpers and tests that Initialize creates keystore and work, is idempotent, and errors when the root parent is missing.
Persistence directory routing
pkg/storage/storage_test.go
Tests that key store and work persistence use their respective directory trees and error when intermediate directories are absent.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding tests for disk storage initialization behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/storage-initialization

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant