chore: improve code coverage toward 90%#222
Merged
Conversation
Remove dead code: - Delete local_memtable.c/.h (428 lines, 0% coverage, never called) - Remove tp_update_metapage_stats (defined but never called) - Remove enable_bmw GUC (BMW is always-on; exhaustive fallback kept for >8 term queries) Add tests for under-tested functionality: - coverage.sql: page visualization, score/BMW stats logging, AM handler properties, dump to file - bulk_load.sql: exercises bulk load spill threshold path - vacuum_extended.sql: vacuum with segments, empty indexes, bulk deletes - binary_io.sql: bm25vector binary COPY (recv/send) - Add explicit_index to REGRESS list (was missing) Annotate unreachable defensive code with LCOV_EXCL: - segment.c: overflow guards, magic number checks, page validation - state.c: recovery failures, stale data, lock upgrade, null guards - metapage.c: null metapage, magic validation, version check Also regenerate expected output files to match current NOTICE behavior (BM25 index build started message was removed in a prior change but expected files were not updated).
The previous commit incorrectly removed "BM25 index build started" notices from expected output files because they were generated on a local build that didn't emit this notice. Regenerated using pg18-release which correctly produces the notice, matching CI behavior.
Add LCOV_EXCL annotations for code paths that cannot be reached through normal SQL tests: - score.c: exhaustive scoring fallback (>8 query terms) - scan.c: exhaustive segment scoring + defensive block load failures - state.c: registry double-failure fallbacks - query.c: PG_CATCH error cleanup Add new tests: - Segment BMW seek via ORDER BY LIMIT with spilled data - tpquery_in parsing with index:query format - BMW stats logging with segment ORDER BY LIMIT queries - Inheritance scoring via find_first_child_bm25_index
LCOV_EXCL for genuinely untestable code paths: - state.c: crash recovery (requires server restart, tested by recovery.sh), transaction abort cleanup, DSA allocation failures - hooks.c: hook chaining (requires other extensions), CustomScan handling (requires other extensions) New tests for real functional gaps: - Plain text ORDER BY (scan.c sk_subtype == TEXTOID path) - HAVING clause with BM25 operator (hooks.c havingQual path) - Build-mode spill with low memtable_spill_threshold - Double bulk-load spill with pre-existing L0 segment
- Fix inaccurate comments claiming ">8 query terms" triggers exhaustive fallback; BMW handles all term counts - Remove nested LCOV_EXCL_START inside already-excluded crash recovery function in state.c (lcov doesn't support nesting) - Fix test comments: Test 10 tests opclass resolution not tp_validate, Test 16 is GROUP BY/HAVING not havingQual
5000168 to
8d2bd4b
Compare
Install TimescaleDB in both coverage jobs and run hypertable.sh to exercise the T_CustomScan code paths in hooks.c. This allows removing the LCOV_EXCL annotations from those cases since they are now covered by tests.
The exhaustive scoring path in score.c was dead code — BMW always returns >= 0, so the fallback was never reached. Remove ~700 lines of unreachable code and associated LCOV_EXCL annotations across score.c, score.h, scan.c, and segment.h. Clean up unused includes.
Remove all source-level LCOV_EXCL_START/STOP annotations from the codebase, following TimescaleDB's approach of not using source annotations for coverage exclusion. Fix shell script cleanup functions to use `pg_ctl stop -m fast` before falling back to `-m immediate`. This ensures gcda coverage data is flushed when processes exit, so code exercised by shell tests (recovery, concurrency, segment, cic, stress) is properly counted in coverage reports.
Configure project-level and patch-level coverage targets at 85%. The project check will fail PRs that drop overall coverage below 85% (with 1% threshold for noise). The patch check requires new/changed lines to have at least 85% coverage.
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
scan.c, segment.h), local_memtable module, unused metapage functions,
enable_bmw GUC
page visualization, GUC validation), bulk_load.sql, vacuum_extended.sql,
plus inheritance and binary I/O test expansions
of not using source-level coverage exclusions
pg_ctl stop -m fastbefore falling backto
-m immediate, so gcda coverage data flushes from recovery, concurrency,segment, cic, and stress tests
code paths when TimescaleDB is available)
Testing
make format-checkpassesshell script coverage flush fix