ci: exclude infrastructure files from code coverage#3379
Conversation
Exclude binary entrypoints, gRPC/HTTP server setup, external client wrappers, streaming plumbing, runtime context providers, and replay/debugging tooling from Codecov coverage metrics. These are infrastructure/glue files that are not unit-testable and inflate uncovered line counts (~3,500 lines total). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughExpanded the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codecov.yml:
- Around line 55-64: Remove the broad exclusions that hide testable business
logic: update .codecov.yml to stop excluding
"packages/rs-drive-abci/src/query/service.rs",
"packages/rs-dapi/src/clients/core_client.rs", and
"packages/rs-dapi/src/services/streaming_service/**" (keep excluding true infra
like main.rs, replay/**, server/**) or alternatively refactor the testable parts
(e.g., atomic block height guards and QueryError::NotServiceable in service.rs,
CORE_RPC_GUARD_PERMITS and permit deadline handling in core_client.rs, and the
worker/lifecycle coordination code in streaming_service) into separate modules
and only exclude the thin infrastructure wrappers; ensure the exclusion list
only omits non-testable shells, not modules with retry/backoff,
semaphore/concurrency, or worker coordination logic.
| # Infrastructure and glue code — binary entrypoints, gRPC/HTTP server setup, | ||
| # external client wrappers, streaming plumbing, runtime context providers, | ||
| # and replay/debugging tooling that are not unit-testable | ||
| - "packages/rs-drive-abci/src/main.rs" | ||
| - "packages/rs-drive-abci/src/query/service.rs" | ||
| - "packages/rs-dapi/src/server/**" | ||
| - "packages/rs-dapi/src/clients/core_client.rs" | ||
| - "packages/rs-sdk-trusted-context-provider/**" | ||
| - "packages/rs-dapi/src/services/streaming_service/**" | ||
| - "packages/rs-drive-abci/src/replay/**" |
There was a problem hiding this comment.
Reconsider excluding files with non-trivial business logic.
The comment describes these as "infrastructure and glue code" that is "not unit-testable," but the relevant code snippets suggest otherwise for several entries:
-
service.rs(line 59): Contains critical distributed systems correctness patterns—atomic block height guards to prevent race conditions, retry logic with exponential backoff (up to 3 retries, 10ms sleep), andQueryError::NotServiceableerror handling for platform saturation. These are testable and verify correctness invariants. -
core_client.rs(line 61): Implements semaphore-based concurrency limiting (CORE_RPC_GUARD_PERMITS), timeout handling to prevent blocking, and permit deadline tracking with explicit release to prevent resource leaks. These availability patterns warrant coverage. -
streaming_service/**(line 63): Contains background worker lifecycle management, masternode list synchronization logic, and multi-worker coordination—beyond simple wiring.
While main.rs, replay/**, and server/** setup may be reasonable exclusions, consider either:
- Keeping coverage for files with business logic (
service.rs,core_client.rs,streaming_service/**) - Refactoring to extract testable logic into separate modules before excluding the infrastructure shell
Excluding these files could mask test coverage gaps in critical correctness and availability patterns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codecov.yml around lines 55 - 64, Remove the broad exclusions that hide
testable business logic: update .codecov.yml to stop excluding
"packages/rs-drive-abci/src/query/service.rs",
"packages/rs-dapi/src/clients/core_client.rs", and
"packages/rs-dapi/src/services/streaming_service/**" (keep excluding true infra
like main.rs, replay/**, server/**) or alternatively refactor the testable parts
(e.g., atomic block height guards and QueryError::NotServiceable in service.rs,
CORE_RPC_GUARD_PERMITS and permit deadline handling in core_client.rs, and the
worker/lifecycle coordination code in streaming_service) into separate modules
and only exclude the thin infrastructure wrappers; ensure the exclusion list
only omits non-testable shells, not modules with retry/backoff,
semaphore/concurrency, or worker coordination logic.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The PR correctly excludes several infrastructure paths from coverage, but the streaming_service/** wildcard is overly broad — it catches bloom.rs (499 lines of pure BIP37 protocol logic with 13 unit tests) and three other files with test suites. The comment on line 55 claiming these files are 'not unit-testable' is inaccurate: 5 of 7 newly excluded paths contain #[cfg(test)] modules. No true blockers since this is coverage configuration and tests still run, but narrowing the globs would improve metric accuracy.
Reviewed commit: cd7ae69
🟡 3 suggestion(s) | 💬 2 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.codecov.yml`:
- [SUGGESTION] line 63: streaming_service/** glob excludes well-tested protocol logic — enumerate files instead
The wildcard `packages/rs-dapi/src/services/streaming_service/**` catches 8 files totaling ~3,700 lines, but 4 of those files have `#[cfg(test)]` modules with 23+ tests:
- **bloom.rs** (499 lines, 13 tests at line 111): Pure BIP37 bloom filter logic — `matches_transaction()`, `extract_pushdatas()`, `bloom_flags_from_int()`. This is not streaming plumbing; it's core protocol logic that determines which transactions a client receives.
- **subscriber_manager.rs** (341 lines, 3 tests at line 198): `FilterType::matches_event()` dispatches to filter-specific matching logic.
- **transaction_stream.rs** (1,263 lines, 4+ tests at line 1103): Stream processing with dedup and merkle-block construction.
- **zmq_listener.rs** (729 lines, 3 tests at line 687): `parse_zmq_message()` and `split_tx_and_lock()` are pure parsing functions.
Excluding these hides coverage they already provide and silently excludes any new files added to this directory. Enumerate the genuinely untestable files instead.
- [SUGGESTION] lines 55-57: Comment says 'not unit-testable' but 5 of 7 paths have #[cfg(test)] modules
The comment on lines 55-57 says these files are 'not unit-testable', but verified `#[cfg(test)]` modules exist in:
1. `main.rs` — line 413, 1 test (DB corruption verification)
2. `server/grpc.rs` — line 225, 1 test (timeout service)
3. `streaming_service/bloom.rs` — line 111, 13 tests
4. `rs-sdk-trusted-context-provider/src/provider.rs` — line 815, 9 tests
5. `replay/` — lines 87/312/554/1069, 93 tests across 4 files
Only `query/service.rs` and `clients/core_client.rs` have no unit tests. A more accurate comment would describe these as infrastructure code primarily exercised through integration tests, not as untestable.
- [SUGGESTION] line 62: rs-sdk-trusted-context-provider/** excludes crate with 9 unit tests and pure validation logic
This pattern excludes the entire crate (~1,107 source lines). `lib.rs` contains `get_quorum_base_url()` (lines 21-62), a pure function with devnet name validation (empty check, alphanumeric+hyphen check, leading/trailing hyphen check) and network dispatch. `provider.rs` (964 lines) has 9 unit tests starting at line 815 covering URL generation, devnet name constraints, domain resolution, and provider initialization.
While much of `provider.rs` does require network calls (quorum fetching, masternode discovery, HTTP clients), the pure helpers in `lib.rs` and the cache management logic are genuinely testable. Consider excluding only the files with heavy external dependencies rather than the entire crate, or accept that this is a pragmatic tradeoff.
| - "packages/rs-dapi/src/server/**" | ||
| - "packages/rs-dapi/src/clients/core_client.rs" | ||
| - "packages/rs-sdk-trusted-context-provider/**" | ||
| - "packages/rs-dapi/src/services/streaming_service/**" |
There was a problem hiding this comment.
🟡 Suggestion: streaming_service/ glob excludes well-tested protocol logic — enumerate files instead**
The wildcard packages/rs-dapi/src/services/streaming_service/** catches 8 files totaling ~3,700 lines, but 4 of those files have #[cfg(test)] modules with 23+ tests:
- bloom.rs (499 lines, 13 tests at line 111): Pure BIP37 bloom filter logic —
matches_transaction(),extract_pushdatas(),bloom_flags_from_int(). This is not streaming plumbing; it's core protocol logic that determines which transactions a client receives. - subscriber_manager.rs (341 lines, 3 tests at line 198):
FilterType::matches_event()dispatches to filter-specific matching logic. - transaction_stream.rs (1,263 lines, 4+ tests at line 1103): Stream processing with dedup and merkle-block construction.
- zmq_listener.rs (729 lines, 3 tests at line 687):
parse_zmq_message()andsplit_tx_and_lock()are pure parsing functions.
Excluding these hides coverage they already provide and silently excludes any new files added to this directory. Enumerate the genuinely untestable files instead.
💡 Suggested change
| - "packages/rs-dapi/src/services/streaming_service/**" | |
| - "packages/rs-dapi/src/services/streaming_service/mod.rs" | |
| - "packages/rs-dapi/src/services/streaming_service/block_header_stream.rs" | |
| - "packages/rs-dapi/src/services/streaming_service/masternode_list_stream.rs" | |
| - "packages/rs-dapi/src/services/streaming_service/masternode_list_sync.rs" |
source: ['claude', 'codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.codecov.yml`:
- [SUGGESTION] line 63: streaming_service/** glob excludes well-tested protocol logic — enumerate files instead
The wildcard `packages/rs-dapi/src/services/streaming_service/**` catches 8 files totaling ~3,700 lines, but 4 of those files have `#[cfg(test)]` modules with 23+ tests:
- **bloom.rs** (499 lines, 13 tests at line 111): Pure BIP37 bloom filter logic — `matches_transaction()`, `extract_pushdatas()`, `bloom_flags_from_int()`. This is not streaming plumbing; it's core protocol logic that determines which transactions a client receives.
- **subscriber_manager.rs** (341 lines, 3 tests at line 198): `FilterType::matches_event()` dispatches to filter-specific matching logic.
- **transaction_stream.rs** (1,263 lines, 4+ tests at line 1103): Stream processing with dedup and merkle-block construction.
- **zmq_listener.rs** (729 lines, 3 tests at line 687): `parse_zmq_message()` and `split_tx_and_lock()` are pure parsing functions.
Excluding these hides coverage they already provide and silently excludes any new files added to this directory. Enumerate the genuinely untestable files instead.
| # Infrastructure and glue code — binary entrypoints, gRPC/HTTP server setup, | ||
| # external client wrappers, streaming plumbing, runtime context providers, | ||
| # and replay/debugging tooling that are not unit-testable |
There was a problem hiding this comment.
🟡 Suggestion: Comment says 'not unit-testable' but 5 of 7 paths have #[cfg(test)] modules
The comment on lines 55-57 says these files are 'not unit-testable', but verified #[cfg(test)] modules exist in:
main.rs— line 413, 1 test (DB corruption verification)server/grpc.rs— line 225, 1 test (timeout service)streaming_service/bloom.rs— line 111, 13 testsrs-sdk-trusted-context-provider/src/provider.rs— line 815, 9 testsreplay/— lines 87/312/554/1069, 93 tests across 4 files
Only query/service.rs and clients/core_client.rs have no unit tests. A more accurate comment would describe these as infrastructure code primarily exercised through integration tests, not as untestable.
💡 Suggested change
| # Infrastructure and glue code — binary entrypoints, gRPC/HTTP server setup, | |
| # external client wrappers, streaming plumbing, runtime context providers, | |
| # and replay/debugging tooling that are not unit-testable | |
| # Infrastructure and glue code — binary entrypoints, gRPC/HTTP server setup, | |
| # external client wrappers, streaming plumbing, runtime context providers, | |
| # and replay/debugging tooling. Most lines require external dependencies | |
| # (Core RPC, Tenderdash, network) and are primarily exercised through | |
| # integration tests rather than unit tests. |
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.codecov.yml`:
- [SUGGESTION] lines 55-57: Comment says 'not unit-testable' but 5 of 7 paths have #[cfg(test)] modules
The comment on lines 55-57 says these files are 'not unit-testable', but verified `#[cfg(test)]` modules exist in:
1. `main.rs` — line 413, 1 test (DB corruption verification)
2. `server/grpc.rs` — line 225, 1 test (timeout service)
3. `streaming_service/bloom.rs` — line 111, 13 tests
4. `rs-sdk-trusted-context-provider/src/provider.rs` — line 815, 9 tests
5. `replay/` — lines 87/312/554/1069, 93 tests across 4 files
Only `query/service.rs` and `clients/core_client.rs` have no unit tests. A more accurate comment would describe these as infrastructure code primarily exercised through integration tests, not as untestable.
| - "packages/rs-drive-abci/src/query/service.rs" | ||
| - "packages/rs-dapi/src/server/**" | ||
| - "packages/rs-dapi/src/clients/core_client.rs" | ||
| - "packages/rs-sdk-trusted-context-provider/**" |
There was a problem hiding this comment.
🟡 Suggestion: rs-sdk-trusted-context-provider/ excludes crate with 9 unit tests and pure validation logic**
This pattern excludes the entire crate (~1,107 source lines). lib.rs contains get_quorum_base_url() (lines 21-62), a pure function with devnet name validation (empty check, alphanumeric+hyphen check, leading/trailing hyphen check) and network dispatch. provider.rs (964 lines) has 9 unit tests starting at line 815 covering URL generation, devnet name constraints, domain resolution, and provider initialization.
While much of provider.rs does require network calls (quorum fetching, masternode discovery, HTTP clients), the pure helpers in lib.rs and the cache management logic are genuinely testable. Consider excluding only the files with heavy external dependencies rather than the entire crate, or accept that this is a pragmatic tradeoff.
source: ['claude', 'codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.codecov.yml`:
- [SUGGESTION] line 62: rs-sdk-trusted-context-provider/** excludes crate with 9 unit tests and pure validation logic
This pattern excludes the entire crate (~1,107 source lines). `lib.rs` contains `get_quorum_base_url()` (lines 21-62), a pure function with devnet name validation (empty check, alphanumeric+hyphen check, leading/trailing hyphen check) and network dispatch. `provider.rs` (964 lines) has 9 unit tests starting at line 815 covering URL generation, devnet name constraints, domain resolution, and provider initialization.
While much of `provider.rs` does require network calls (quorum fetching, masternode discovery, HTTP clients), the pure helpers in `lib.rs` and the cache management logic are genuinely testable. Consider excluding only the files with heavy external dependencies rather than the entire crate, or accept that this is a pragmatic tradeoff.
| - "packages/rs-dapi/src/clients/core_client.rs" | ||
| - "packages/rs-sdk-trusted-context-provider/**" | ||
| - "packages/rs-dapi/src/services/streaming_service/**" | ||
| - "packages/rs-drive-abci/src/replay/**" |
There was a problem hiding this comment.
💬 Nitpick: replay/ has 93 unit tests across ~3,900 lines but is genuinely debug tooling**
Both agents flagged this, but after verification: the replay directory is a self-contained debug/replay utility for replaying captured ABCI requests from logs. It is NOT core platform logic. The 93 tests across 4 files (mod.rs: 30, cli.rs: 13, runner.rs: 46, log_ingest.rs: 4) cover CLI parsing, request validation, progress formatting, and log deserialization.
While excluding 93 tests from coverage reporting is notable, the exclusion is defensible since this is debug tooling that doesn't affect platform behavior. The main concern is that the /** wildcard would silently exclude any future files added to this directory.
source: ['claude', 'codex']
| # external client wrappers, streaming plumbing, runtime context providers, | ||
| # and replay/debugging tooling that are not unit-testable | ||
| - "packages/rs-drive-abci/src/main.rs" | ||
| - "packages/rs-drive-abci/src/query/service.rs" |
There was a problem hiding this comment.
💬 Nitpick: query/service.rs has ~170 lines of concurrency logic but no unit tests
Verified: handle_blocking_query (lines 91-258, 1,029-line file) implements a double-check loop with committed_block_height_guard, sleep-based polling with retry counters, and saturation detection returning QueryError::NotServiceable. The pure helpers query_error_into_status (lines 1014-1025) and error_into_status (line 1027) are trivially testable. This file genuinely has no #[cfg(test)] module, so the exclusion is defensible — but the pure helper functions at the bottom could reasonably be unit tested.
source: ['claude']
2 participants
Issue being fixed or feature implemented
Infrastructure and glue files (binary entrypoints, server setup, external client wrappers, streaming plumbing, context providers, replay tooling) inflate uncovered line counts by ~3,500 lines. These files are not unit-testable by nature and should not count toward coverage metrics.
What was done?
Added 7 new entries to the
ignore:section in.codecov.yml:packages/rs-drive-abci/src/main.rspackages/rs-drive-abci/src/query/service.rspackages/rs-dapi/src/server/**packages/rs-dapi/src/clients/core_client.rspackages/rs-sdk-trusted-context-provider/**packages/rs-dapi/src/services/streaming_service/**packages/rs-drive-abci/src/replay/**How Has This Been Tested?
Configuration-only change. Codecov will apply the updated ignore rules on the next coverage upload.
Breaking Changes
None.
Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit