Fix/postman row truncation#9
Merged
Merged
Conversation
… cap TestReport::record() silently drops rows once PM_MAX_ROWS (256) is reached, corrupting PostMan's summary table and verdict with no warning. This test records 300 synthetic rows directly and asserts the observed row count matches — it fails today (clamped at 256) and is expected to pass once the fixed-size PmRow array becomes a std::vector. Adds TestReport::rowCount() as a minimal test-visible accessor; no other behavior changes.
record() silently dropped rows once the 256-entry PmRow[] backing array filled up, so PostMan's summary table and verdict went blind past that point without any warning (reproducible with --gtest_repeat). print()/calcCols() were already parameterized by the real row count, not the array capacity, so switching _rows to a std::vector removes the cap without touching any rendering code. PostMan.cpp compiles under C++17 (tests/Makefile), so std::vector is unrestricted here. rowCount() now reports the vector's real size.
Records more than PM_MAX_ROWS synthetic rows and asserts the reported row count matches what was recorded. Fails against the current fixed array: record() returns early past 256 with no warning, so the delta comes back at 108 instead of 300.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PostMan's report could print "All 256 assertions passed" while hundreds of
results — including genuine failures — had been discarded without a word.
PM_MAX_ROWS = 256sized a fixed array, andrecord()returned early past it(
PostMan.cpp:154). Under--gtest_repeat=3over the full suite, 444 testsran and passed, but the table reported 256. The exit code was never wrong (it
comes from
RUN_ALL_TESTS()), so CI was honest — the report a human readswas not.
The dangerous case is worse than an undercount: a test that FAILS past row 256
has its row dropped before
passedis even inspected. Verified: green table,makeexiting with Error 1. Table and process fully desynchronised.This matters because
--gtest_repeatis this repo's main tool for ruling outflakiness. T6 was validated with it. A reporter that quietly hides rows poisons
the instrument.
Fix
PmRow[PM_MAX_ROWS]→std::vector<PmRow>, no capacity guard.print()derives every count from
_rows.size(). The macro is removed, not raised:bumping 256 to 4096 would have made the test green and left the silent-drop
behaviour intact one order of magnitude further out.
~50 net lines across
vendor/PostMan.{hpp,cpp}, plus a regression test and oneline in
tests/Makefile.Verification
fix, not by reading the diff): reported delta 108 vs 300 expected.
--gtest_repeat=3now reports 1347 = 3 × 449, exact, no clamping.EXPECT_TRUE(false)past row 256 now shows as FAIL in the table andin the aggregate.
scripts/audit.shPASSED on all three tiers; suite green.Accepted risk
The vector has no ceiling. A future bug calling
record()in a runaway loopgrows memory unbounded instead of clamping at ~20 KB. Deliberate — clamping is
what caused this bug.
Out of scope
Dockerfile:10andci.yml:53still say "138-assertion suite". Stale, but aseparate ticket.