Skip to content

test: raise btree/common coverage 85.3% -> 87.7%, fix stale Codecov slug - #307

Merged
gerardrecinto merged 3 commits into
masterfrom
feat/coverage-to-100-core-packages
Sep 10, 2026
Merged

gerardrecinto merged 3 commits into
masterfrom
feat/coverage-to-100-core-packages

Conversation

@gerardrecinto

Copy link
Copy Markdown
Collaborator

Summary

Progress toward 100% coverage on the four packages Codecov measures (btree, inmemory, fs, common), plus a real bug fix found along the way.

  • Fixed: go.yml uploaded coverage to slug: SharedCode/sop (the pre-rebrand repo name) instead of SharedCode/joltrin; the README badge had the same stale link. Every other badge in the README already pointed at joltrin - this one didn't get updated. Badge may have been broken/pointing at the wrong Codecov project regardless of the coverage percentage.
  • Added tests for previously-0%-covered code: btree.Cursor's untested delegate methods (AddIfNotExist, Upsert, Update, UpdateKey, Remove, FindWithID, FindInDescendingOrder, UpdateCurrentItem, UpdateCurrentKey, GetCurrentItem, GetCurrentItemNoLock), the matching btreeWithTransaction wrapper methods (not-begun/non-writer/delegated-error/success paths for each), common.Transaction.GetStores/CommitMaxDuration, and common.CursorOnOpenedBtree/OpenBtreeCursor (both fully untested).
  • Coverage on the measured scope: 85.3% -> 87.7%.

Honest status on "100%"

Not there. What's left is concentrated in two places:

  1. Genuinely hard, concurrency-coordinator internals in common/transactionlogger.go (e.g. resurrectPriorityLogLocks at 36.4%, waitForCoordinatorRelease at 53.8%) and common/twophasecommittransaction2.go (e.g. processPriorityRollbackOnRestart at 22.2%) - these need real understanding of the two-phase-commit recovery paths to test correctly, not quick wins. Rushing these risks shallow tests that pass without actually proving the recovery logic works.
  2. Defensive if err != nil branches in fs/ (file I/O, erasure coding) that are borderline untestable without fault-injection scaffolding that doesn't exist yet.

I stopped at a clean, fully-green checkpoint rather than pushing further into #1 without slowing down to do it right. Happy to continue in a follow-up if you want to keep going.

Test plan

  • go test ./inmemory/... ./btree/... ./common/... ./fs/... -count=1 - all green
  • Verified the specific new/changed tests individually before the full-suite run
  • CI: ci.yml / go.yml / security.yml / codeql.yml / e2e.yml

go.yml uploaded coverage to slug: SharedCode/sop and the README badge
linked the same old project - a leftover from the joltrin rebrand that
every other badge in the README already picked up. The badge was
pointing at a stale/wrong Codecov project regardless of the actual
coverage percentage.
…ates

Cursor.AddIfNotExist/Upsert/Update/UpdateKey/Remove/FindWithID/
FindInDescendingOrder/UpdateCurrentItem/UpdateCurrentKey/GetCurrentItem/
GetCurrentItemNoLock, and the matching btreeWithTransaction wrapper
methods (not-begun, non-writer, delegated-error, and success paths),
were previously untested. Raises this repo's core-package coverage
from 85.3% to 87.0%.
…tree cursor openers

GetStores and CommitMaxDuration were only ever exercised through their
underlying dependency directly, never through the wrapper method
itself. CursorOnOpenedBtree and OpenBtreeCursor (both 0%) needed
their nil-transaction/not-begun/empty-name/not-found precondition
branches plus the already-open and fetch-from-StoreRepository success
paths.
@gerardrecinto
gerardrecinto merged commit 5c5ec9d into master Sep 10, 2026
19 checks passed
@gerardrecinto
gerardrecinto deleted the feat/coverage-to-100-core-packages branch September 10, 2026 09:00
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