Skip to content

Conversation

@koriyoshi2041
Copy link

Summary

This PR adds comprehensive unit tests for previously untested core modules and LLM client implementations, addressing the "Add better unit tests" item in CONTRIBUTING.md.

Changes

tests/test_comms_utils.py (34 tests)

  • LMRequest/LMResponse dataclass serialization
  • Socket send/recv protocol testing
  • Batched request handling

tests/clients/test_openai.py (23 tests)

  • Client initialization with various configurations
  • Completion with string and message list prompts
  • Usage tracking and accumulation
  • Async completion support
  • Integration tests (skipped without API key)

tests/clients/test_anthropic.py (27 tests)

  • System message extraction and handling
  • Message list format conversion
  • Usage tracking per model
  • Async completion support

tests/test_rlm_logger.py (18 tests)

  • Logger initialization and file creation
  • Metadata and iteration logging
  • JSONL format validation
  • Complex data serialization

tests/test_lm_handler.py (29 tests)

  • Client routing by depth and model name
  • Server lifecycle management
  • Context manager protocol
  • Full socket communication integration

Testing

uv run pytest tests/test_comms_utils.py tests/clients/test_openai.py \
    tests/clients/test_anthropic.py tests/test_rlm_logger.py \
    tests/test_lm_handler.py -v
# Result: 125 passed

Code Quality

  • uv run ruff check: All checks passed
  • uv run ruff format --check: All files formatted

Notes

  • Integration tests are marked with @pytest.mark.skipif and require API keys
  • Uses existing MockLM class for unit testing without API calls
  • Follows existing test style from test_local_repl.py, test_parsing.py, test_gemini.py

- test_comms_utils.py: 34 tests for socket protocol and message dataclasses
- test_openai.py: 23 tests for OpenAI client
- test_anthropic.py: 27 tests for Anthropic client
- test_rlm_logger.py: 18 tests for JSONL logging
- test_lm_handler.py: 29 tests for LM routing and server lifecycle

Total: 125 tests
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.

1 participant