Skip to content

ci: exclude infrastructure files from code coverage #3379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
QuantumExplorer merged 1 commit into from
Mar 18, 2026
Merged

ci: exclude infrastructure files from code coverage #3379

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .codecov.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ ignore:
- "packages/rs-platform-version/**"
# Simple signer — thin test-only wrapper
- "packages/simple-signer/src/**"
# 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
Comment on lines +55 to +57
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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:

  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.

💡 Suggested change
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/main.rs"
- "packages/rs-drive-abci/src/query/service.rs"
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 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']

- "packages/rs-dapi/src/server/**"
- "packages/rs-dapi/src/clients/core_client.rs"
- "packages/rs-sdk-trusted-context-provider/**"
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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/services/streaming_service/**"
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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() 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.

💡 Suggested change
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.

- "packages/rs-drive-abci/src/replay/**"
Comment on lines +55 to +64
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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:

  1. 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), and QueryError::NotServiceable error handling for platform saturation. These are testable and verify correctness invariants.

  2. 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.

  3. 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.

Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 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']


coverage:
status:
Expand Down
Loading