Unit tests for platform-core
#17
Conversation
Add 58 new tests across platform-core covering previously uncovered code paths: - challenge.rs: Test ChallengeConfig::with_mechanism builder method - crypto.rs: Add tests for seed fallback, sign_bytes, sign_data/hash_data serialization paths - error.rs: Add conversion tests for io::Error, bincode::Error (serialization failures), and serde_json::Error, plus error display formatting - message.rs: Test TaskProgressMessage, ChallengeContainerConfig validation (docker_image, timeouts, CPU/memory limits), MechanismWeightConfig builders, and development mode check - schema_guard.rs: Test schema description methods, error formatting for SchemaMismatch and MissingVersion, const_hash determinism, and migration path verification - state.rs: Test default_timestamp, production_default, total_stake calculation (active-only), max_validators enforcement, and claim_job exhaustion - state_versioning.rs: Test ValidatorInfoLegacy migration, ChainStateV2 migration, version error paths (too old, deserialization failures, unknown versions) - types.rs: Add tests for Hotkey SS58 conversion (roundtrip and invalid input), ChallengeId::from_uuid/default uniqueness
📝 WalkthroughWalkthroughAdds extensive unit tests across eight core modules (challenge, crypto, error, message, schema_guard, state, state_versioning, types). All changes are test-only under #[cfg(test)]; no production code or public API modifications. Changes
Sequence Diagram(s)(omitted — changes are test-only and do not introduce new multi-component control flows) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/crypto.rs (1)
321-335: Remove the hard-coded “production sudo mnemonic” from tests (secret exposure).
Having what’s labeled as a production mnemonic in-repo is effectively publishing a private key. Replace with a non-production test vector (or derive from a public key only), and rotate any real secrets immediately if this mnemonic was ever used.
🧹 Nitpick comments (1)
crates/core/src/message.rs (1)
1548-1702: Make env-var-sensitive tests deterministic (DEVELOPMENT_MODE can break whitelist/validate tests).
Several tests in this module assumeDEVELOPMENT_MODEis unset/false; if CI sets it, whitelist/validation assertions will flip. Also,test_is_development_modecurrently doesn’t assert behavior.Suggestion: add a small helper that locks + sets/unsets
DEVELOPMENT_MODE, runs the assertion, then restores the previous value; use it in whitelist/validate tests and intest_is_development_mode.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
crates/core/src/challenge.rscrates/core/src/crypto.rscrates/core/src/error.rscrates/core/src/message.rscrates/core/src/schema_guard.rscrates/core/src/state.rscrates/core/src/state_versioning.rscrates/core/src/types.rs
🧰 Additional context used
🧬 Code graph analysis (2)
crates/core/src/types.rs (1)
crates/challenge-sdk/src/types.rs (5)
test_challenge_id_from_uuid(472-477)from_uuid(16-18)test_challenge_id_default(313-317)default(27-29)default(78-87)
crates/core/src/state.rs (3)
crates/consensus/src/stake_governance.rs (1)
block_height(349-351)crates/core/src/state_versioning.rs (1)
create_test_state(305-308)crates/core/src/types.rs (5)
new(70-72)new(118-120)new(141-150)new(199-204)new(239-249)
🔇 Additional comments (14)
crates/core/src/challenge.rs (1)
234-241: LGTM! Well-focused test for the builder method.The test correctly validates that
with_mechanismsets the mechanism_id while preserving default values for other configuration fields.crates/core/src/state.rs (5)
454-460: LGTM! Validates timestamp initialization.The test appropriately checks that
default_timestamp()returns a timestamp close to the current time with a reasonable 5-second tolerance.
462-468: LGTM! Validates production defaults.The test correctly verifies that
production_default()initializes with block_height 0, subnet_id 100, and a non-zero sudo key.
470-505: LGTM! Comprehensive stake calculation tests.These two tests effectively validate:
- Total stake sums correctly across all active validators
- Inactive validators are properly excluded from the total
The approach of manually inserting an inactive validator (line 500) is appropriate since
add_validatorwould reject it.
507-529: LGTM! Validates max validator limit enforcement.The test properly verifies that exceeding
max_validatorsreturns aConsensuserror, ensuring the system enforces capacity limits.
531-549: LGTM! Thorough job claiming edge case coverage.The test validates:
- Claiming returns
Nonewhen no jobs exist- Claiming succeeds when a job is available
- Claiming returns
Nonewhen all jobs are already assignedcrates/core/src/state_versioning.rs (3)
439-482: LGTM! Validates legacy migration behavior.These tests effectively verify:
ValidatorInfoLegacymigrates correctly withx25519_pubkeydefaulting toNoneChainStateV2migrates to current format while preservingregistered_hotkeys
484-517: LGTM! Validates smart deserialization strategies.These tests confirm that
deserialize_state_smartcorrectly handles:
- Direct V2 state deserialization
- Versioned (current format) deserialization
519-593: LGTM! Comprehensive error path coverage.These tests validate proper error handling for:
- Version too old (below MIN_SUPPORTED_VERSION)
- V1/V2 deserialization failures with invalid data
- Unknown version numbers
- Complete deserialization failure with incompatible format
The error assertions properly verify both the error type and message content.
crates/core/src/error.rs (1)
69-110: LGTM! Comprehensive error handling validation.The tests effectively cover:
Fromtrait implementations forio::Error,bincode::Error, andserde_json::Error- Proper error variant mapping (Internal, Serialization)
Displaytrait output for various error typesThe bincode error test (lines 83-90) is particularly well-crafted, creating a realistic error by serializing to a buffer that's too small.
crates/core/src/types.rs (3)
491-499: LGTM! Validates SS58 address roundtrip.The test correctly verifies that a
Hotkeycan be converted to SS58 format and back, preserving the original value.
501-505: LGTM! Validates SS58 error handling.The test properly confirms that
from_ss58returnsNonefor invalid or empty addresses.
507-521: LGTM! Validates ChallengeId behavior.These tests effectively verify:
from_uuidpreserves the UUID valueDisplayformatting matches the UUID string representationdefault()generates unique IDs (important for avoiding collisions)crates/core/src/schema_guard.rs (1)
374-493: Added schema-guard tests look solid and low-flake.
Good coverage for formatting/determinism/registry invariants without being overly brittle.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.