feat: Add unit tests for JSONSummaryReporter#152
Open
Ayush-Patel-56 wants to merge 1 commit intokrkn-chaos:mainfrom
Open
feat: Add unit tests for JSONSummaryReporter#152Ayush-Patel-56 wants to merge 1 commit intokrkn-chaos:mainfrom
Ayush-Patel-56 wants to merge 1 commit intokrkn-chaos:mainfrom
Conversation
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoAdd unit tests for JSONSummaryReporter class
WalkthroughsDescription• Added comprehensive unit tests for JSONSummaryReporter class • Validates run metadata, duration, and fitness statistics accuracy • Tests generation-wise fitness progression and top-10 ranking logic • Verifies results.json file persistence and content correctness Diagramflowchart LR
A["Test Setup<br/>Population & Results"] -->|validates| B["JSONSummaryReporter"]
B -->|generates| C["Summary Data"]
C -->|checks| D["Metadata & Stats"]
C -->|checks| E["Fitness Progression"]
C -->|checks| F["Best Scenarios Ranking"]
B -->|saves| G["results.json File"]
File Changes1. tests/unit/reporter/test_json_summary_reporter.py
|
Ayush-Patel-56
force-pushed
the
fix/issue-149-json-reporter-tests
branch
from
f6313ba to
18f88df
Compare
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Code Review by Qodo
1. generate_summary assertions incomplete
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
56
to
66
| summary = reporter.generate_summary() | ||
|
|
||
| assert summary["run_id"] == "test-run" | ||
| assert summary["duration_seconds"] == 100.0 | ||
| assert summary["summary"]["total_scenarios_executed"] == 6 | ||
| assert summary["summary"]["best_fitness_score"] == 60.0 | ||
|
|
||
| assert summary["fitness_progression"][0]["average"] == 10.0 | ||
| assert summary["fitness_progression"][1]["average"] == 50.0 | ||
| assert summary["fitness_progression"][1]["best"] == 60.0 | ||
|
|
There was a problem hiding this comment.
1. generate_summary assertions incomplete 📎 Requirement gap ✓ Correctness
• The test_generate_summary_content test asserts only a small subset of the required generate_summary() output (e.g., run_id, duration_seconds, a couple summary stats, and a few progression values). • This does not validate required run metadata (e.g., seed, timestamps), config summary, and the broader statistics/schema expectations, so regressions to results.json structure or key computed fields could still pass CI. • As written, the test suite does not meet the checklist’s requirement to assert the unified summary dictionary’s required keys and correct values.
Agent Prompt
## Issue description
`generate_summary()` unit tests do not validate the required output schema and key computed fields described in the compliance checklist.
## Issue Context
Downstream consumers rely on `results.json` schema stability. Partial assertions (only a few keys) can allow breaking changes to pass CI.
## Fix Focus Areas
- tests/unit/reporter/test_json_summary_reporter.py[34-66]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
81
to
86
| best = reporter.generate_summary()["best_scenarios"] | ||
| assert len(best) == 10 | ||
| assert best[0]["rank"] == 1 | ||
| assert best[0]["fitness_score"] == 140.0 | ||
| assert best[0]["parameters"]["end"] == 10 | ||
|
|
There was a problem hiding this comment.
2. Top-10 ranking checks partial 📎 Requirement gap ✓ Correctness
• The best-scenarios test checks only the list length and the first entry’s rank/fitness, but it does not validate full sorting, rank increments, truncation correctness across all returned entries, or required fields for each item. • This leaves gaps where incorrect ordering, missing fields, or wrong ranks for entries 2–10 would not be caught.
Agent Prompt
## Issue description
`_build_best_scenarios()` behavior (sorting/ranking/truncation and required fields) is only partially validated.
## Issue Context
Incorrect ordering or missing fields in `best_scenarios` can break downstream consumers even if the top item looks correct.
## Fix Focus Areas
- tests/unit/reporter/test_json_summary_reporter.py[67-86]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
87
to
97
| def test_empty_population(self, minimal_config): | ||
| reporter = JSONSummaryReporter( | ||
| run_uuid="empty", | ||
| config=minimal_config, | ||
| seen_population={}, | ||
| best_of_generation=[], | ||
| ) | ||
| summary = reporter.generate_summary() | ||
| assert summary["summary"]["total_scenarios_executed"] == 0 | ||
| assert summary["best_scenarios"] == [] | ||
|
|
There was a problem hiding this comment.
3. Missing minimal/zero-fitness edge cases 📎 Requirement gap ⛯ Reliability
• The tests include an empty population case, but do not cover the other required edge cases: single-generation best_of_generation and explicit zero-fitness scenarios. • Without these cases, failures or malformed output in minimal/degenerate runs can slip through CI.
Agent Prompt
## Issue description
Required edge cases (single generation and zero fitness) are not covered.
## Issue Context
Minimal or degenerate runs are common in testing and early stopping; output must remain stable and error-free.
## Fix Focus Areas
- tests/unit/reporter/test_json_summary_reporter.py[87-97]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
98
to
111
| def test_save_json_file(self, minimal_config, temp_output_dir): | ||
| reporter = JSONSummaryReporter( | ||
| run_uuid="save-test", | ||
| config=minimal_config, | ||
| seen_population={}, | ||
| best_of_generation=[], | ||
| ) | ||
| reporter.save(temp_output_dir) | ||
|
|
||
| path = os.path.join(temp_output_dir, "results.json") | ||
| assert os.path.exists(path) | ||
| with open(path, "r") as f: | ||
| content = json.load(f) | ||
| assert content["run_id"] == "save-test" |
There was a problem hiding this comment.
4. save() test not consistency-checking 📎 Requirement gap ✓ Correctness
• The save() test verifies results.json exists and is parseable, but it does not assert the file content matches (or is consistent with) generate_summary() for the same inputs. • This can miss regressions where save() writes a different structure/content than generate_summary() returns.
Agent Prompt
## Issue description
`save()` test does not verify that the written `results.json` content matches `generate_summary()` output.
## Issue Context
`save()` is the persistence path; mismatches between in-memory summary and serialized file can break pipelines.
## Fix Focus Areas
- tests/unit/reporter/test_json_summary_reporter.py[98-111]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+45
to
+54
| reporter = JSONSummaryReporter( | ||
| run_uuid="test-run", | ||
| config=minimal_config, | ||
| seen_population=pop, | ||
| best_of_generation=best, | ||
| start_time=now, | ||
| end_time=now + datetime.timedelta(seconds=100), | ||
| completed_generations=2, | ||
| seed=123 | ||
| ) |
There was a problem hiding this comment.
5. Ga metadata always empty 🐞 Bug ✓ Correctness
• The new unit tests pass end_time and completed_generations into JSONSummaryReporter, so duration/generation metadata looks correct in tests. • In the real code path (GeneticAlgorithm.save()), end_time remains None and completed_generations remains 0 because they are initialized but never updated in the generation loop or stop logic. • As a result, results.json will report duration_seconds=0.0 and generations_completed=0 for real runs, which breaks correctness of run reporting/analytics.
Agent Prompt
## Issue description
`GeneticAlgorithm` passes `end_time` and `completed_generations` into `JSONSummaryReporter`, but these fields are never updated during the algorithm run. This causes `results.json` to report incorrect run metadata (duration stays 0, generations_completed stays 0) in real executions.
## Issue Context
The new unit tests for `JSONSummaryReporter` pass explicit `start_time/end_time/completed_generations`, so they don’t exercise the real producer of these values (`GeneticAlgorithm`).
## Fix Focus Areas
- krkn_ai/algorithm/genetic.py[120-190]
- krkn_ai/algorithm/genetic.py[260-280]
- krkn_ai/algorithm/genetic.py[682-700]
- krkn_ai/reporter/json_summary_reporter.py[66-70]
## Implementation notes
- Set `self.completed_generations` when a generation completes (e.g., after evaluation, or when stopping).
- Set `self.end_time` when stopping is triggered (ideally timezone-aware and consistent with `self.start_time`).
- Consider setting `self.seed = self.config.seed` (or pass `config.seed`) so seed is recorded.
- Add an integration-style test that runs a minimal/mocked `GeneticAlgorithm.simulate()` then `save()` and asserts `results.json` has non-zero duration and correct generations_completed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Ayush-Patel-56
force-pushed
the
fix/issue-149-json-reporter-tests
branch
2 times, most recently
from
e4f4083 to
ae5eacc
Compare
Signed-off-by: Ayush-Patel-56 <ayushpatel2731@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Closes #149
PR Type
Tests, Bug Fixes
Description
Added unit test coverage for JSONSummaryReporter and addressed a few reporting bugs found during development.
The changes provide 100% coverage for the reporter logic, including ranking and fitness progression. I also updated GeneticAlgorithm to correctly populate
end_timeandcompleted_generationsin the final output, as these were previously remaining at their default/empty values. Finally, I standardized all timestamps to UTC across the algorithm and runner to avoidTypeErrorrisks with naive/aware datetime mixing.Diagram Walkthrough
graph TD A[GeneticAlgorithm] -->|updates| B[Metadata] B -->|reports| C[JSONSummaryReporter] C -->|generates| D[results.json] E[KrknRunner] -->|calculates| F[Fitness UTC] F -->|stores| BFile Walkthrough
test_json_summary_reporter.py
Add unit tests for JSONSummaryReportertests/unit/reporter/test_json_summary_reporter.py
Testing
python -m pytest tests/unit/reporter/test_json_summary_reporter.pyruffandmypy.