Skip to content

Fix #173: Split eval_test.exs into multiple focused test files#182

Merged
andreasronge merged 1 commit intomainfrom
claude/issue-173-20251209-235800
Dec 10, 2025
Merged

Fix #173: Split eval_test.exs into multiple focused test files#182
andreasronge merged 1 commit intomainfrom
claude/issue-173-20251209-235800

Conversation

@andreasronge
Copy link
Copy Markdown
Owner

Summary

Split test/ptc_runner/lisp/eval_test.exs (1078 lines, 114 tests) into 6 focused test files to improve maintainability and navigation:

  • eval_data_types_test.exs - literal, vector, map, set evaluation tests (~170 lines)
  • eval_control_flow_test.exs - variable access, and/or, if, let, where, predicates (~230 lines)
  • eval_functions_test.exs - fn, closures, destructuring, multi-arity, builtins (~385 lines)
  • eval_errors_test.exs - error propagation tests (~160 lines)
  • eval_memory_test.exs - memory threading tests (~27 lines)
  • eval_sets_test.exs - set predicates, constructor, collection operations (~70 lines)

Test plan

✅ All 114 tests pass
✅ Pre-commit checks pass
✅ Each file is self-contained with proper module setup
✅ Pattern follows established precedent from issue #95

Fixes #173

🤖 Generated with Claude Code

Splits the large eval_test.exs (1078 lines, 114 tests) into 6 focused test files:
- eval_data_types_test.exs: literal, vector, map, set evaluation
- eval_control_flow_test.exs: variable access, and/or, if, let, where, predicates
- eval_functions_test.exs: fn, closures, destructuring, multi-arity, builtins
- eval_errors_test.exs: error propagation in structures, bindings, calls
- eval_memory_test.exs: memory threading through evaluation
- eval_sets_test.exs: set predicates, constructor, collection operations

Follows the pattern from issue #95 for organizing test files by functional area.
Each file is self-contained with proper module setup and helper functions.
Test count remains at 114 tests.

Fixes #173

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

PR Review: Fix #173: Split eval_test.exs into multiple focused test files

Summary

Successfully splits the monolithic eval_test.exs (1078 lines, 114 tests) into 6 focused test files. The implementation is clean, matches the issue specification, and all tests pass.

What's Good

  • Correct file organization: Files are logically grouped by functional area (data types, control flow, functions, errors, memory, sets)
  • Test count preserved: All 114 tests maintained across the split
  • Follows established pattern: Structure mirrors the test/ptc_runner/json/operations/ pattern from issue Refactor: Split test/ptc_runner_test.exs into logical smaller files #95
  • Self-contained modules: Each file has proper setup with aliases and helpers
  • Tests pass: All 114 tests pass with mix test test/ptc_runner/lisp/eval_*.exs
  • Appropriate file sizes: Largest file is 397 lines (eval_functions_test.exs), well under the threshold

Issues (Must Fix)

None.

Suggestions (Optional)

  1. Helper function duplication - Nice-to-have
    • Observation: defp dummy_tool(_name, _args), do: :ok is duplicated in 5 files
    • Benefit: A shared test helper in test/support/ could reduce this boilerplate
    • Complexity: Low (one new helper module)
    • Recommendation: Out of scope for this PR. The duplication is minimal (~1 line each) and matches how the original file worked. Could be addressed in a future test infrastructure cleanup issue.

Security

No concerns. This is a test file reorganization with no functional changes.

Documentation

No updates needed. The test organization documentation in docs/guidelines/testing-guidelines.md:22-27 references eval_test.exs generically, which is still accurate since all the new files follow the eval_*_test.exs pattern.

Verdict

Approve. Clean implementation of a straightforward tech debt task. All acceptance criteria from issue #173 are met:

  • ✅ 6 focused test files created
  • ✅ Each file is self-contained with proper module setup
  • ✅ All 114 tests pass
  • ✅ Original eval_test.exs deleted
  • ✅ File sizes match specification (~170, ~230, ~385, ~160, ~27, ~70 lines)

@andreasronge
Copy link
Copy Markdown
Owner Author

Auto-Triage Summary

Decisions Made

# Issue Decision Action
1 Helper function duplication (dummy_tool/2 in 5 files) DEFER_ISSUE Created issue #183

Status

Reasoning

The review identified one optional suggestion about extracting the duplicated dummy_tool/2 helper to a shared test support module. The reviewer explicitly marked this as "Out of scope for this PR" and recommended deferring to a future test infrastructure cleanup.

Decision: Created issue #183 to track this tech debt. The duplication is minimal (~1 line per file) and doesn't block this PR, which successfully accomplishes its stated goal of splitting the monolithic test file.

Since there are no FIX_NOW items, this PR is ready to merge pending CI checks.

@andreasronge andreasronge added the ready-to-merge PR is ready to be merged label Dec 10, 2025
@github-actions github-actions bot added auto-triage-cycle-1 Auto-triage cycle 1 auto-triage-complete Auto-triage completed and removed auto-triage-pending Auto-triage in progress labels Dec 10, 2025
@andreasronge andreasronge enabled auto-merge (squash) December 10, 2025 00:04
@andreasronge andreasronge merged commit a2118b5 into main Dec 10, 2025
4 checks passed
@andreasronge andreasronge deleted the claude/issue-173-20251209-235800 branch December 10, 2025 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-triage-complete Auto-triage completed auto-triage-cycle-1 Auto-triage cycle 1 ready-to-merge PR is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[From PR #172] Split eval_test.exs into multiple files

1 participant