Skip to content

test(memstore): add tests for Store operations and Entry conversions - #2607

Open
Qalipso wants to merge 1 commit into
kataras:mainfrom
Qalipso:memstore_test_coverage
Open

test(memstore): add tests for Store operations and Entry conversions#2607
Qalipso wants to merge 1 commit into
kataras:mainfrom
Qalipso:memstore_test_coverage

Conversation

@Qalipso

@Qalipso Qalipso commented Jul 28, 2026

Copy link
Copy Markdown

core/memstore is at 11.0% statement coverage. It is a self-contained, dependency-free package, so it is straightforward to cover.

This adds tests for:

  • the Store surface: Set, Get, GetDefault, GetEntry, GetEntryAt, Exists, Remove, Reset, Len, Visit, GetOrSet
  • insert vs update semantics of Save (the returned boolean)
  • immutability: SetImmutable entries reject a plain Set but accept another SetImmutable
  • Entry conversions and their error paths: StringDefault, StringTrim, IntDefault, BoolDefault, TimeDefault, WeekdayDefault, Float64Default
  • ErrEntryNotFound formatting and As matching, including via errors.As
  • the sized integer getters, including the out-of-range rejection in the unsigned variants

Coverage goes from 11.0% to 59.2%. No non-test code is changed; go vet and go test -race pass.


While writing these I ran into what looks like a bug in UintDefault, so I left it uncovered rather than encode the current behaviour in a test:

var maxValue uint64 = math.MaxUint32
if x64 {
    maxValue = math.MaxUint64
}
...
case int:
    if vv < 0 || vv > int(maxValue) {
        return def, e.notFound(uintType)
    }

On a 64-bit platform maxValue is math.MaxUint64, so the conversion int(maxValue) wraps to -1. The comparison becomes vv > -1, which is true for every positive int, so UintDefault returns the default and a not-found error for any plain int value:

var p memstore.Store
p.Set("n", 8)
p.GetUintDefault("n", 0) // returns 0, not 8

The sized variants (Uint8Default and friends) use literal bounds and are not affected. Happy to send a separate PR for it if you'd like — I did not want to mix a behaviour change into a test-only PR.

core/memstore was at 11.0% statement coverage. This covers the Store
CRUD surface (Set/Get/GetEntry/GetEntryAt/Remove/Reset/Len/Visit/
GetOrSet), immutability semantics, the Entry type conversions and their
error paths, ErrEntryNotFound matching, and the sized integer and float
getters including out-of-range rejection.

Coverage goes from 11.0% to 59.2%. No non-test code is changed.
@Qalipso
Qalipso requested a review from kataras as a code owner July 28, 2026 15:56
@CLAassistant

CLAassistant commented Jul 28, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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