[oschwald]

Summary

Split the test suite into unit tests and integration tests. src/reader_test.rs and the test module in src/lib.rs mixed public-API tests with tests that reach into Reader internals.

Closes #151.

Commits

1. Split tests into unit and integration tests:
   • The 17 tests that use crate internals move into the test module in src/reader.rs. Examples are buf, pointer_base, metadata_start, ipv4_start_bit_depth, validate_metadata_for_reader, and without_lookup_index.
   • The other 56 tests move into one integration test binary, tests/integration/. It has one module per area: reader, lookup, decode, within, and verify. Shared helpers are in common.rs.
   • tests/derived_newtypes.rs moves into the same binary.
   • The integration crate denies the same lints as src/lib.rs: trivial_casts, trivial_numeric_casts, and unused_import_braces. One trivial u32 cast in derived_newtypes.rs is removed to satisfy them.
   • src/reader_test.rs and the test module in src/lib.rs are removed.
   • test_lookup_network from lib.rs becomes test_lookup_networks_across_databases, because reader_test.rs had a test with the same name.
2. Narrow visibility of Reader internals: metadata_start, has_ipv4_subtree, and validate_metadata_for_reader lose pub(crate). None of them is used outside src/reader.rs. The other pub(crate) fields stay, because result.rs and within.rs use them.
3. Use test reader helpers in moved tests:
   • Tests that opened databases with Reader::open_readfile and a full path now use open_test_data_reader or open_test_reader. Both helpers name the database in the panic message.
   • 7 tests in src/reader.rs opened the fixture and then read the same file again with std::fs::read. They now copy reader.buf. One more test now opens its fixture with open_test_reader.
   • reader_with_data in derived_newtypes.rs needs the raw file bytes, which integration tests cannot get from a Reader. It keeps std::fs::read but names the database if the read fails.

Why one integration binary

Cargo builds each top-level file in tests/ as a separate crate and links each one. One binary links once, and its modules share common.rs without #[allow(dead_code)].

Notes

• The lookup-index tests stay in src/reader.rs. They call without_lookup_index, which exists only under cfg(test) or the fuzzing feature. Integration tests do not compile the library with cfg(test).
• In commits 1 and 2, the test bodies are unchanged apart from imports, helper names, the one cast, and rustfmt line wrapping.
• Not changed here: lookup_index_preserves_corrupt_tree_behavior computes (1 << bits) - 1 as a usize. That overflows when bits is 32 on a 32-bit target. The test was moved unchanged, and CI does not build 32-bit targets.

Testing

• The list of test names under --all-features matches main (191 tests), except for the one rename.
• cargo test passes with default features, with --features mmap, and with --all-features.
• precious lint --all passes.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Expanded integration coverage for IP lookups, value decoding, database metadata, verification, and network iteration across IPv4, IPv6, and mixed databases.
  • Added checks for malformed data, missing records, and resource limits, including cases where configured limits are reached.
  • Consolidated reader tests into broader integration coverage, including lookup consistency and network iteration options.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The PR reorganizes tests into topical integration modules and reader unit tests. It adds coverage for database opening, lookup, decoding, network iteration, and verification. It also narrows the visibility of several reader internals. No production behavior changes are reported.

Changes

Test suite reorganization

Layer / File(s)	Summary
Test module split and shared helpers src/lib.rs, src/reader_test.rs, tests/integration/main.rs, tests/integration/common.rs	Removes the former test modules and declares topical integration test modules. Adds shared database configurations and a helper to open test databases.
Reader metadata and lookup coverage src/reader.rs, tests/integration/reader.rs, tests/integration/lookup.rs	Narrows visibility for reader internals. Adds unit and integration coverage for metadata, database opening, IP lookups, malformed trees, and lookup results across database configurations.
Decoder and path-decoding coverage src/reader.rs, tests/integration/decode.rs	Adds coverage for typed and nested decoding, path navigation, malformed payloads, and resource limits.
Network iteration and verification coverage src/reader.rs, tests/integration/within.rs, tests/integration/verify.rs	Adds coverage for network iteration options, CIDR results, malformed trees, and verification of valid and invalid databases.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Merge Risk: Minimal · up to 37b63

This change reorganizes tests and narrows internal helper visibility without altering production behavior. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "Split tests into unit and integration tests" matches the main change. The PR moves crate-internal tests into src/reader.rs as unit tests and moves public-API tests into a tests/integration …
Linked Issues check		Issue #151 has three coding requirements. The whole-PR summary shows each one is met at head 37b6318. (1) [#151] Public-API tests move to tests/. The PR adds tests/integration/ with main.rs, `…
Out of Scope Changes check		Each change supports the test reorganization in #151. - src/lib.rs: removes the old test module only. - src/reader.rs: adds test code and helpers. Three test-only items lose pub(crate). The sum…
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

Full details: Title check

Explanation

The title "Split tests into unit and integration tests" matches the main change. The PR moves crate-internal tests into src/reader.rs as unit tests and moves public-API tests into a tests/integration binary. The title is short, specific, and states the primary change.

Full details: Linked Issues check

Explanation

Issue #151 has three coding requirements. The whole-PR summary shows each one is met at head 37b6318.

(1) [#151] Public-API tests move to tests/. The PR adds tests/integration/ with main.rs, common.rs, reader.rs, lookup.rs, decode.rs, within.rs, and verify.rs. It also moves tests/derived_newtypes.rs into the same binary. #151 allows this single-binary layout.

(2) [#151] White-box tests move to a #[cfg(test)] test module in src/reader.rs. The summary lists lookup-index equivalence, metadata and pointer validation, verification limits, and decode_path budget tests there. These tests use crate internals, as #151 requires.

(3) [#151] src/reader_test.rs is deleted, and the test module in src/lib.rs is removed. Visibility is narrowed for metadata_start, has_ipv4_subtree, and validate_metadata_for_reader, which become private. Other pub(crate) fields stay because result.rs and within.rs use them. #151 permits this with the words 'where possible'.

Limits: The assessment rests on the whole-PR summary and the PR description. I did not read the hunks directly. I did not re-run tests. The PR reports 191 tests, matching main except for one rename.

Full details: Out of Scope Changes check

Explanation

Each change supports the test reorganization in #151.

• src/lib.rs: removes the old test module only.
• src/reader.rs: adds test code and helpers. Three test-only items lose pub(crate). The summary reports no production behavior change.
• src/reader_test.rs: deleted, as #151 requires.
• tests/integration/common.rs: adds shared helpers and constants used by the moved tests.
• tests/integration/derived_newtypes.rs: uses open_test_data_reader. It also removes one node_count cast to satisfy the lint settings the PR adds to the integration crate. This is test-only.
• Rename test_lookup_network to test_lookup_networks_across_databases: resolves a name clash created by the move.
• Integration crate lint settings match src/lib.rs. This keeps the moved tests under the same lint rules.

The PR notes an existing 32-bit usize overflow in lookup_index_preserves_corrupt_tree_behavior. The test is moved unchanged. This is not a new unrelated change.

I found no change unrelated to #151. This conclusion rests on the whole-PR summary and PR description, not on a direct read of each hunk.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each lookup path,
And tests the records, leaf by leaf.
It hops through CIDRs, neat and clear,
Then checks the limits, far and near.
The tests find homes in modules new.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 013c4184-4c19-4e81-869e-7e700043d276

Commits

Reviewing files that changed from the base of the PR and between 5c5e635 and 37b6318.

Files selected for processing: 2

• src/reader.rs
• tests/integration/derived_newtypes.rs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.