test: increase unit test coverage for core/ modules - #115
Conversation
Add 65 new tests across 4 test files covering controller.py, evaluator.py, selection.py, and version_database.py — the modules called out in TODO.md P2 'Increase unit test coverage for core/ modules'. New test files: - test_controller.py (12 tests): initialize, run_generation, select_candidates, get_state, cli_interface - test_version_database_extended.py (26 tests): experiment tracking, get_variant_metadata, get_best_variants, get_variant_statistics, export/import, edge cases - test_selection_extended.py (14 tests): custom fitness_key, empty population, elitism edge cases, custom strategies, negative/missing scores - test_evaluator_extended.py (13 tests): empty results, missing metrics, unknown strategy fallback, score bounds, feedback content Closes TODO.md item: 'Increase unit test coverage for core/ modules'
|
Warning Review limit reached
Next review available in: 39 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request adds unit tests for four EVOSEAL core modules and updates TODO tracking totals to record the increased test coverage. ChangesUnit test coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (3)
tests/unit/evoseal/test_controller.py (1)
60-66: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen the assertion for the default-score case.
This test asserts only
selected[0]["score"] == 0.8. It does not verify the full descending order or that the entry missingscoredefaults to0.0at the tail. The comment on line 63 documents the expected order0.8, 0.6, 0.0, but the assertions do not check it.Proposed stronger assertion
assert len(selected) == 3 assert selected[0]["score"] == 0.8 + assert selected[1]["score"] == 0.6 + assert selected[2].get("score", 0.0) == 0.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/evoseal/test_controller.py` around lines 60 - 66, Strengthen test_select_candidates_defaults_missing_score_to_zero by asserting the complete selected order: scores must be 0.8, 0.6, then 0.0, confirming descending ranking and the missing-score default at the tail.tests/unit/evoseal/test_selection_extended.py (1)
146-151: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify the claimed clamping behavior.
The docstring states negative scores are clamped to 0 in roulette selection, but the assertion only checks
len(selected) == 2. Add an assertion that actually confirms clamping (for example, that selection does not raise and that all individuals remain eligible), or adjust the docstring to match what the test verifies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/evoseal/test_selection_extended.py` around lines 146 - 151, Update test_roulette_with_negative_scores to verify the documented negative-score clamping behavior, not only the result length: assert selection completes without error and confirms the expected eligible individuals can be selected. Alternatively, revise the docstring to describe only the behavior the test actually checks.tests/unit/evoseal/test_version_database_extended.py (1)
19-301: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared fixture for
VersionDatabase()instantiation.
db = VersionDatabase()repeats in every one of the 24 test functions in this file. Replace it with a pytest fixture to reduce duplication and simplify future changes to database setup.Suggested fix
import pytest from evoseal.core.version_database import VersionDatabase + +@pytest.fixture +def db(): + return VersionDatabase() + # --- Experiment tracking --- -def test_add_variant_with_experiment_id(): - db = VersionDatabase() +def test_add_variant_with_experiment_id(db): db.add_variant("v1", "code", {}, 0.8, experiment_id="exp1")Apply the same pattern to the remaining test functions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/evoseal/test_version_database_extended.py` around lines 19 - 301, Introduce a pytest fixture that returns a VersionDatabase instance, then update all test functions in this file to accept the fixture and remove their local VersionDatabase() instantiations. Apply this consistently across the experiment, metadata, statistics, import/export, and edge-case tests while preserving each test’s existing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/evoseal/test_controller.py`:
- Around line 1-16: Mark the new unit test modules with pytestmark =
pytest.mark.unit after their imports: tests/unit/evoseal/test_controller.py
lines 1-16, tests/unit/evoseal/test_evaluator_extended.py lines 1-13, and
tests/unit/evoseal/test_selection_extended.py lines 1-14. Use the existing
pytest import and apply the marker at module scope in each file.
In `@tests/unit/evoseal/test_version_database_extended.py`:
- Around line 1-14: Add the repository’s unit pytest marker at module scope in
test_version_database_extended.py using pytestmark, so all tests in this
extended VersionDatabase test module are classified for marker-based CI
selection; retain the existing pytest import and test behavior.
- Around line 224-278: Update VersionDatabase.import_variants to check each
incoming variant_id against existing variants before appending or registering
experiment and lineage data, skipping duplicates and excluding them from the
returned import count. Preserve existing records and only apply experiment
tracking and lineage updates for newly imported variants, so
test_import_variants_skips_duplicates verifies the contract.
---
Nitpick comments:
In `@tests/unit/evoseal/test_controller.py`:
- Around line 60-66: Strengthen
test_select_candidates_defaults_missing_score_to_zero by asserting the complete
selected order: scores must be 0.8, 0.6, then 0.0, confirming descending ranking
and the missing-score default at the tail.
In `@tests/unit/evoseal/test_selection_extended.py`:
- Around line 146-151: Update test_roulette_with_negative_scores to verify the
documented negative-score clamping behavior, not only the result length: assert
selection completes without error and confirms the expected eligible individuals
can be selected. Alternatively, revise the docstring to describe only the
behavior the test actually checks.
In `@tests/unit/evoseal/test_version_database_extended.py`:
- Around line 19-301: Introduce a pytest fixture that returns a VersionDatabase
instance, then update all test functions in this file to accept the fixture and
remove their local VersionDatabase() instantiations. Apply this consistently
across the experiment, metadata, statistics, import/export, and edge-case tests
while preserving each test’s existing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fe01d1b6-a4dc-4786-8971-979c4a4e9d63
📒 Files selected for processing (5)
TODO.mdtests/unit/evoseal/test_controller.pytests/unit/evoseal/test_evaluator_extended.pytests/unit/evoseal/test_selection_extended.pytests/unit/evoseal/test_version_database_extended.py
| def test_import_variants_skips_duplicates(): | ||
| db = VersionDatabase() | ||
| db.add_variant("v1", "original", {}, 0.5) | ||
| export_data = { | ||
| "variants": { | ||
| "v1": { | ||
| "variant_id": "v1", | ||
| "source": "imported", | ||
| "test_results": {}, | ||
| "eval_score": 0.9, | ||
| "parent_ids": [], | ||
| "metadata": {}, | ||
| "experiment_id": None, | ||
| } | ||
| }, | ||
| "lineage": {}, | ||
| } | ||
| count = db.import_variants(json.dumps(export_data)) | ||
| assert count == 0 | ||
| assert db.get_variant("v1")["source"] == "original" | ||
|
|
||
|
|
||
| def test_import_variants_with_experiment_tracking(): | ||
| db = VersionDatabase() | ||
| export_data = { | ||
| "variants": { | ||
| "v1": { | ||
| "variant_id": "v1", | ||
| "source": "code", | ||
| "test_results": {}, | ||
| "eval_score": 0.8, | ||
| "parent_ids": [], | ||
| "metadata": {}, | ||
| "experiment_id": "exp1", | ||
| } | ||
| }, | ||
| "lineage": {}, | ||
| } | ||
| db.import_variants(json.dumps(export_data)) | ||
| assert db.get_variant_experiment("v1") == "exp1" | ||
| assert "v1" in db.get_experiment_variants("exp1") | ||
|
|
||
|
|
||
| def test_import_variants_preserves_lineage(): | ||
| db = VersionDatabase() | ||
| export_data = { | ||
| "variants": { | ||
| "v1": {"variant_id": "v1", "source": "A", "test_results": {}, "eval_score": 0.5}, | ||
| "v2": {"variant_id": "v2", "source": "B", "test_results": {}, "eval_score": 0.8}, | ||
| }, | ||
| "lineage": {"v2": ["v1"]}, | ||
| } | ||
| db.import_variants(json.dumps(export_data)) | ||
| assert db.get_lineage("v2") == ["v1"] | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect import_variants implementation for duplicate handling and experiment/lineage tracking.
ast-grep run --pattern 'def import_variants($$$) {
$$$
}' --lang python evoseal/core/version_database.pyRepository: SHA888/EVOSEAL
Length of output: 478
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)version_database\.py$|test_version_database_extended\.py$' || true
echo "== outline version_database =="
ast-grep outline evoseal/core/version_database.py --view expanded || true
echo "== import_variants occurrences =="
rg -n "def import_variants|import_variants|experiment_id|lineage" evoseal/core/version_database.py tests/unit/evoseal/test_version_database_extended.pyRepository: SHA888/EVOSEAL
Length of output: 9507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant version_database sections =="
python3 - <<'PY'
from pathlib import Path
p=Path("evoseal/core/version_database.py")
lines=p.read_text().splitlines()
for i,l in enumerate(lines,1):
if "class VersionDatabase" in l:
print(f"class at {i}")
PY
# Print line-numbered slices around likely functions/tests
for start in $(rg -n "def (import_variants|add_variant|get_variant_experiment|get_experiment_variants|get_lineage|set_lineage)|def get_variant_experiment|def get_experiment_variants|def get_lineage" evoseal/core/version_database.py tests/unit/evoseal/test_version_database_extended.py | cut -d: -f1 | sort -u); do
echo "===== $start ====="
sed -n "$((start>20?start-20:1)),$((start+80))p" evoseal/core/version_database.py tests/unit/evoseal/test_version_database_extended.py 2>/dev/null | cat -n
doneRepository: SHA888/EVOSEAL
Length of output: 291
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== version_database.py import_variants implementation =="
sed -n '272,310p' evoseal/core/version_database.py | cat -n
echo "== import-related tests =="
sed -n '180,280p' tests/unit/evoseal/test_version_database_extended.py | cat -nRepository: SHA888/EVOSEAL
Length of output: 5578
No duplicate-skip behavior for import_variants.
import_variants registers experiment_id and lineage after appending imported variants, so these tests do not exercise the claimed duplicate-skip behavior and will pass with the current implementation. Update import_variants to skip already-imported variant IDs if this contract is intended.
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 240-240: use jsonify instead of json.dumps for JSON output
Context: json.dumps(export_data)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 261-261: use jsonify instead of json.dumps for JSON output
Context: json.dumps(export_data)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 275-275: use jsonify instead of json.dumps for JSON output
Context: json.dumps(export_data)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/evoseal/test_version_database_extended.py` around lines 224 - 278,
Update VersionDatabase.import_variants to check each incoming variant_id against
existing variants before appending or registering experiment and lineage data,
skipping duplicates and excluding them from the returned import count. Preserve
existing records and only apply experiment tracking and lineage updates for
newly imported variants, so test_import_variants_skips_duplicates verifies the
contract.
Review feedback verificationI verified each review point against the actual source code ( 1. Stochastic flakiness — Not a real issueThe reviewer missed that Empirically verified: 500/500 runs passed with zero failures. The other three tests only assert 2.
|
…comment - Add seeded_rng fixture that monkeypatches secrets.SystemRandom with a deterministic random.Random(42), applied to all stochastic selection tests. - Fix misleading comment in test_tournament_custom_fitness_key: v4 is favored because it has the highest custom_score (0.5), not because eval_score equals 0.5. Also assert selected[0] is v4 (deterministic via elitism) instead of the probabilistic any() check. - Verified: IndexError for empty population is correct (secrets.SystemRandom .choice([]) raises IndexError); no change needed there. Addresses review feedback on PR #115.
|
Addressed the review feedback. Here is my point-by-point analysis: 1. Unseeded RNG in stochastic tests ✅ Fixed
2. IndexError for empty population ✅ Already correct — no change needed
3. Misleading comment on
4. Broader caveat (score formulas, defaults, exceptions taken on faith) ℹ️ Informational
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/evoseal/test_selection_extended.py`:
- Around line 120-123: Strengthen the edge-case assertions in the
oversized-selection, negative-score roulette, and missing-score tests: pass
elitism=0 where needed, then assert expected individual IDs rather than only
result length. In test_num_selected_larger_than_population, verify all selected
entries are population members and that padding repeats members; update the
roulette test to verify negative scores are clamped as documented, and the
missing-score test to verify the individual selected under the default score of
0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d285700-4cc1-4650-b4fc-a9c513eb0e06
📒 Files selected for processing (1)
tests/unit/evoseal/test_selection_extended.py
- Add pytestmark = pytest.mark.unit to all 4 new test files - Strengthen test_select_candidates_defaults_missing_score_to_zero with full top-3 order assertions (0.8, 0.6, 0.0) - Strengthen test_num_selected_larger_than_population to verify padding uses population members with repeats - Strengthen test_roulette_with_negative_scores to verify selected individuals come from the population - Strengthen test_tournament_missing_eval_score to verify v1 is selected (higher score beats missing-score default of 0)
Review feedback analysis — verified against actual sourceI verified each point from the review against 1.
|
- Remove unused import in test_version_database_extended.py - Pass explicit in test_tournament_custom_fitness_key to decouple from the DEFAULT_ELITISM constant
Review feedback addressed (c61a096)Fixed
Verified correct — no change needed
|
Review feedback — verified against sourceAddressed each point from the review against the actual 1.
|
1 participant
What
Add 65 new tests across 4 test files covering
controller.py,evaluator.py,selection.py, andversion_database.py— the modules called out in TODO.md P2 "Increase unit test coverage forcore/modules".New test files
test_controller.py(12 tests): initialize, run_generation orchestration, select_candidates sorting/truncation, get_state, cli_interface dispatchtest_version_database_extended.py(26 tests): experiment tracking (add/get by experiment), get_variant_metadata, get_best_variants with filtering/limits, get_variant_statistics (empty, same-score, per-experiment), export/import (JSON string, file, duplicates, lineage preservation, experiment tracking), edge casestest_selection_extended.py(14 tests): custom fitness_key, empty population (raises IndexError), single individual, elitism > population size, elitism=0, num_selected=0 and >population, custom strategies, negative/missing scorestest_evaluator_extended.py(13 tests): empty results, missing/partial metrics, unknown strategy fallback, all-metrics-perfect/zero, feedback content (flags low coverage/quality, clean when perfect), custom default_weights, strategy overwrite, multiple results orderingVerification
ruff format --check .✅ruff check evoseal/ tests/✅pytest tests/unit/evoseal/ -v→ 81 passed ✅ (65 new + 16 existing)Closes TODO.md item: "Increase unit test coverage for
core/modules"Summary by CodeRabbit