Skip to content

Test/fix assertions for go127 - #17

Merged
fredbi merged 4 commits into
masterfrom
test/fix-assertions-for-go127
Aug 23, 2026
Merged

Test/fix assertions for go127#17
fredbi merged 4 commits into
masterfrom
test/fix-assertions-for-go127

Conversation

@fredbi

@fredbi fredbi commented Aug 23, 2026

Copy link
Copy Markdown
Member

Change type

Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update

Short description

Fixes

Full description

Checklist

  • I have signed all my commits with my name and email (see DCO. This does not require a PGP-signed commit
  • I have rebased and squashed my work, so only one commit remains
  • I have added tests to cover my changes.
  • I have properly enriched go doc comments in code.
  • I have properly documented any breaking change.

fredbi and others added 3 commits August 23, 2026 08:44
Before go1.27, the flate library could not produce a compressed string
that fits into a Handle. That case was not covered in go1.26.

Now some tests with extreme compressions ratios (e.g. 129 times "a") may
be inlined, with the compressed string packed into the returned handle
and zero arena consumption.

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
go1.27 rewrote compress/flate, which changed *which* encoding branch a
string lands in and made two latent bugs reachable. TestStoreAllocations
was failing on go1.27 as a result.

Two independent stdlib behaviour changes are at play:

  - highly repetitive input now deflates below 9 bytes (129 x "a": 9 -> 6),
    so it is inlined in the handle instead of going to the arena. That made
    headerInlinedCompressedString reachable for the first time.
  - incompressible input now falls back to a *stored* block at the default
    level, so the output can be larger than the input (350 -> 357, where
    go1.26 Huffman-coded it down to 251).

Fixes:

  - ensureEmptyBuffer grew the caller scratch relative to its length rather
    than its capacity, so passing a full `var buf [8]byte` as buf[:] asked
    slices.Grow for len+size and reallocated every time. It now truncates
    before growing; unpackASCII and unpackBlanks had open-coded copies of
    the same bug and route through the helper.
  - stack scratch arrays handed to a pooled reader or to the writer
    interface escaped to the heap (5 sites flagged by -gcflags=-m). They now
    borrow from poolOfBytes.
  - putCompressedString sized its scratch at len(value); it now uses
    compressBound(), which accounts for stored-block framing.
  - putCompressedString keeps the raw bytes when DEFLATE does not shrink the
    value, instead of spending more arena space AND an inflate on every read.
  - minCompressedSize 9 -> 6, re-documented as a sizing heuristic rather
    than the stdlib invariant it used to be. uncompressRatioHeuristic floors
    on compressionThreshold+1, a far better bound for inlined payloads, and
    getCompressedString no longer pre-sizes its result buffer, so a wrong
    ratio guess can never cost a second allocation.
  - uncompressStringReader returned a redeem closure capturing three
    variables, which escaped on every call. It now returns an inflateSession
    struct by value.

TestStoreAllocations keyed its expectations on the measured alloc count,
which could only fail by a wide margin. Expectations now derive from the
handle header, each subtest asserts which encoding was chosen, and a new
WriteTo group asserts that path is allocation-free.

Measured on go1.27: WriteTo 1 -> 0 allocs for inlined strings, inlined
ASCII and compressed values; AppendValueBytes 2 -> 0 for inlined-compressed;
Get 3 -> 1 for inlined-compressed.

Verified on go1.27.0 and go1.26.7 across the none/guards/poolsdebug/
writerguards build tags, plus -race and golangci-lint.

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doc: retire gocard badge
chore: schedule dependabot twice a week

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
@fredbi
fredbi force-pushed the test/fix-assertions-for-go127 branch from ef68b9c to f208731 Compare August 23, 2026 06:45
@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.18750% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.19%. Comparing base (383a198) to head (21707e7).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
json/stores/default-store/store.go 42.30% 15 Missing ⚠️
json/stores/default-store/verbatim.go 0.00% 6 Missing ⚠️

❌ Your patch check has failed because the patch coverage (67.18%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
+ Coverage   63.88%   69.19%   +5.30%     
==========================================
  Files          90       90              
  Lines        9681     9681              
==========================================
+ Hits         6185     6699     +514     
+ Misses       2994     2973      -21     
+ Partials      502        9     -493     

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

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi
fredbi merged commit 3e398ca into master Aug 23, 2026
24 of 25 checks passed
@fredbi
fredbi deleted the test/fix-assertions-for-go127 branch August 23, 2026 08:40
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