Skip to content

Conversation

@jworth008
Copy link

@jworth008 jworth008 commented Dec 8, 2025

Pull Request: Add Comprehensive Unit Tests for Timelock Module

Bug

Missing unit test coverage for bittensor/core/timelock.py. While integration tests exist in tests/integration_tests/test_timelock.py, there are no unit tests that can run without network access to Drand. This leaves the module's logic untested in isolation and makes CI slower due to network-dependent tests.

Description of the Change

This PR adds a comprehensive unit test suite (tests/unit_tests/test_timelock.py) with 47 unit tests covering all aspects of the timelock module using mocked dependencies for fast, isolated testing.

Test Coverage Added:

TLE Constants (3 tests)

  • TLE_ENCRYPTED_DATA_SUFFIX type and value validation
  • Module __all__ exports verification

encrypt() Function (12 tests)

  • String to bytes conversion
  • Bytes data passthrough
  • Default and custom block_time handling
  • Fast-blocks mode (block_time=0.25)
  • Integer and float block_time support
  • Return type validation
  • Empty string/bytes handling
  • Unicode string support
  • Large n_blocks values

decrypt() Function (8 tests)

  • Return type validation (bytes by default)
  • None return before reveal round
  • no_errors flag handling
  • return_str flag handling
  • Unicode result decoding

wait_reveal_and_decrypt() Function (9 tests)

  • Explicit reveal_round parameter
  • Automatic round parsing from encrypted data
  • Waiting loop behavior
  • return_str and no_errors parameter passing
  • Error handling for invalid/malformed data
  • Immediate return when past reveal round

Round Parsing (4 tests)

  • Valid encrypted data parsing
  • Little-endian format verification
  • Maximum value (2^64 - 1) handling
  • Zero value handling

Edge Cases (7 tests)

  • Zero and negative n_blocks
  • Float and very small block_time
  • Large data handling
  • Exact round match behavior
  • Binary data with null bytes

Module Imports (5 tests)

  • Function importability verification
  • Constant exportability

Alternate Designs

Alternative approaches considered:

  1. Extend existing integration tests - Rejected because integration tests require network access to Drand and are slower (~3s per test vs 0.06s for all unit tests)
  2. Fewer tests with broader scope - Rejected to ensure comprehensive coverage of all code paths and parameter combinations
  3. No mocking, use real Drand - Rejected because unit tests should be fast, isolated, and not depend on external services

The chosen approach provides thorough unit-level testing while maintaining test independence and fast execution.

Possible Drawbacks

  • Adds ~500 lines of test code to maintain
  • Uses mocking which may need updates if internal implementation changes
  • Some overlap with existing integration tests (different testing approach, same functionality)

Verification Process

  1. Ran the full test suite:
pytest tests/unit_tests/test_timelock.py -v

Result: 47 passed in 0.06s

  1. Verified lint compliance:
ruff check tests/unit_tests/test_timelock.py

Result: All checks passed!

  1. Verified format compliance:
ruff format --check tests/unit_tests/test_timelock.py

Result: 1 file already formatted

  1. Test Categories Verified:
  • ✅ encrypt() input handling and parameter passing
  • ✅ decrypt() return types and error handling
  • ✅ wait_reveal_and_decrypt() waiting logic and round parsing
  • ✅ Edge cases and boundary conditions
  • ✅ Module exports and imports

Release Notes

N/A - This change adds test coverage only and does not affect user-facing functionality.

Branch Acknowledgement

[x] I am acknowledging that I am opening this branch against staging

@jworth008
Copy link
Author

@basfroman Hope you had a great weekend. Could you please check the PR and give me your feedback when you are available?

@basfroman
Copy link
Collaborator

@basfroman Hope you had a great weekend. Could you please check the PR and give me your feedback when you are available?

All the coverage you're providing in this PR is already available/covered in the current SDKv10 codebase and in bittensor-drand one.
Conclusion: this PR is redundant.
Pls close this PR.

@jworth008 jworth008 closed this Dec 8, 2025
@jworth008 jworth008 reopened this Dec 9, 2025
@jworth008 jworth008 closed this by deleting the head repository Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants