Skip to content

fix: Split transformation_test.exs into access_test.exs and collection_test.exs (#244)#247

Merged
andreasronge merged 2 commits intomainfrom
claude/issue-244-20251211-143003
Dec 11, 2025
Merged

fix: Split transformation_test.exs into access_test.exs and collection_test.exs (#244)#247
andreasronge merged 2 commits intomainfrom
claude/issue-244-20251211-143003

Conversation

@andreasronge
Copy link
Copy Markdown
Owner

Summary

Reorganize test files to align with lib module structure:

  • Created test/ptc_runner/json/operations/access_test.exs with all access operation tests (first, last, nth, take, drop, distinct, get, sort_by)
  • Moved collection operation tests (filter, map, select, reject) to existing collection_test.exs
  • Deleted test/ptc_runner/json/operations/transformation_test.exs

Test Plan

  • All 64 tests continue to pass
  • Test structure now mirrors lib/ptc_runner/json/operations module organization
  • Verified with mix test - all tests pass
  • Pre-commit checks pass

Closes #244

…n_test.exs (#244)

Reorganize test files to align with lib module structure:
- Created test/ptc_runner/json/operations/access_test.exs with all access operation tests (first, last, nth, take, drop, distinct, get, sort_by)
- Moved collection operation tests (filter, map, select, reject) to existing collection_test.exs
- Deleted test/ptc_runner/json/operations/transformation_test.exs

All 64 tests continue to pass. Test structure now mirrors lib/ptc_runner/json/operations module organization.

🤖 Generated with Claude Code

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

PR Review: fix: Split transformation_test.exs into access_test.exs and collection_test.exs (#244)

Summary

This PR reorganizes the test files to align with the lib module structure by renaming transformation_test.exs to access_test.exs and moving collection operation tests (filter, map, select, reject) to collection_test.exs. The reorganization is clean and all tests pass.

What's Good

  • Test file structure now mirrors lib module structure (access.exaccess_test.exs, collection.excollection_test.exs)
  • All 1119 tests continue to pass
  • The transformation_test.exs file was correctly deleted (renamed)
  • Tests were moved verbatim without logic changes as specified in the issue
  • Clear organization with comments identifying each operation type

Issues (Must Fix)

  1. Duplicate E2E test - access_test.exs:696-715 and collection_test.exs:562-581
    • Problem: Both files contain essentially identical E2E tests with the same implementation:
      • access_test.exs: "first/last/nth work in realistic data processing pipeline"
      • collection_test.exs: "E2E: filter/reject work in realistic data processing pipeline"
    • Impact: Code duplication; running the same test twice wastes CI time
    • Fix: Remove one of the duplicate tests. Since the test uses reject (collection operation) and first (access operation), it could reasonably live in either file - choose one location and remove the other.
    • Complexity: Mechanical (delete one test)

Suggestions (Optional)

None - the PR cleanly accomplishes its stated goal.

Security

No concerns.

Documentation

No updates needed - this is a pure test reorganization.

Verdict

Request Changes - The duplicate E2E test should be removed before merge. This is a simple fix: delete either test "first/last/nth work in realistic data processing pipeline" from access_test.exs:697-715 OR test "E2E: filter/reject work in realistic data processing pipeline" from collection_test.exs:563-581.

@github-actions github-actions bot added the auto-triage-pending Auto-triage in progress label Dec 11, 2025
@andreasronge
Copy link
Copy Markdown
Owner Author

@claude please fix these issues from PR review:

Issue 1: Remove duplicate E2E test

Location: test/ptc_runner/json/operations/access_test.exs:696-715
Problem: Duplicate E2E test appears in both access_test.exs and collection_test.exs with identical implementation. The same test exists at collection_test.exs:562-581.
Fix: Remove the E2E test from access_test.exs (lines 696-715, including the comment line 696). Keep the version in collection_test.exs since the test primarily validates the reject operation (a collection operation) with first as a supporting operation.

Please fix this issue in a SINGLE commit. Make minimal changes to resolve the issue.

@andreasronge
Copy link
Copy Markdown
Owner Author

Auto-Triage Summary

Decisions Made

# Issue Decision Action
1 Duplicate E2E test in access_test.exs and collection_test.exs FIX_NOW Included in fix request above

Rationale

The duplicate test is a mechanical fix directly in-scope for this PR (test reorganization). The test was accidentally copied to both files during the split. Since the test primarily validates the reject operation (collection), it should remain in collection_test.exs and be removed from access_test.exs.

Status

  • FIX_NOW items: 1 (batched in single fix comment above)
  • Issues created: 0
  • Items ignored: 0

The automated fix will remove the duplicate test and the PR will be ready to merge once the fix is applied and checks pass.

@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 11, 2025
The E2E test "first/last/nth work in realistic data processing pipeline"
was present in both access_test.exs and collection_test.exs. Since the
test primarily validates the `reject` operation (a collection operation),
keep the version in collection_test.exs and remove the duplicate from
access_test.exs.

Fixes PR review issue in #247.

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

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@andreasronge
Copy link
Copy Markdown
Owner Author

Fix Complete

Issue 1: Remove duplicate E2E test ✅

Status: Fixed

Changes Made:

  • Removed duplicate E2E test from test/ptc_runner/json/operations/access_test.exs:696-715
  • Kept the test in collection_test.exs as it primarily validates the reject operation (a collection operation)

Verification:

  • All 1118 tests pass
  • Code formatting compliant
  • Credo checks pass
  • Pre-commit checks pass

The PR is now ready for merge.

@github-actions
Copy link
Copy Markdown
Contributor

PR Review: Split transformation_test.exs into access_test.exs and collection_test.exs

Summary

Clean test reorganization that aligns test file structure with lib module structure. The PR correctly splits tests for operations defined in access.ex vs collection.ex into their respective test files.

What's Good

  • Correct alignment: Tests are split according to actual lib module structure:
    • access.ex ops (get, first, last, nth, take, drop, distinct, sort_by) → access_test.exs
    • collection.ex ops (filter, map, select, reject) → collection_test.exs
  • Test preservation: All 93 tests pass (53 in access_test, 40 in collection_test)
  • Meaningful E2E test rename: The E2E test was renamed from "first/last/nth work in realistic data processing pipeline" to "E2E: filter/reject work in realistic data processing pipeline" which better reflects its content (it tests reject + first, not just access operations)
  • Net-zero changes: 199 additions / 199 deletions confirms this is a pure reorganization with no logic changes

Issues (Must Fix)

None.

Suggestions (Optional)

None. The PR scope is appropriate - adding describe blocks or other structural improvements would be out of scope for this refactoring PR.

Security

No concerns - this is test-only code reorganization.

Documentation

No updates needed - this affects only test organization.

Verdict

Approve - Clean, correct refactoring that improves maintainability by aligning test files with lib module structure. All tests pass.

@github-actions github-actions bot added the auto-triage-pending Auto-triage in progress label Dec 11, 2025
@andreasronge andreasronge added the ready-to-merge PR is ready to be merged label Dec 11, 2025
@andreasronge
Copy link
Copy Markdown
Owner Author

Auto-Triage Summary

Review Status

The PR has already been fully triaged and fixed by the repository owner:

  1. Initial Review (Request Changes) - Found 1 duplicate E2E test
  2. Fix Applied - Owner posted @claude fix request and duplicate was removed
  3. Final Review (Approve) - No remaining issues

Decisions Made

# Issue Decision Action
1 Duplicate E2E test FIX_NOW ✅ Already fixed by owner

Status

  • FIX_NOW items: 0 (issue already fixed)
  • Issues created: 0
  • Items ignored: 0

Outcome

✅ All checks pass
✅ Review verdict: Approve
✅ No remaining issues
✅ Ready to merge - added ready-to-merge label

Auto-merge will handle merging once all required checks are complete.

@github-actions github-actions bot added auto-triage-cycle-2 Auto-triage cycle 2 and removed auto-triage-cycle-1 Auto-triage cycle 1 auto-triage-pending Auto-triage in progress labels Dec 11, 2025
@andreasronge andreasronge enabled auto-merge (squash) December 11, 2025 14:42
@andreasronge andreasronge merged commit f0dc31e into main Dec 11, 2025
4 checks passed
@andreasronge andreasronge deleted the claude/issue-244-20251211-143003 branch December 11, 2025 14:42
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-2 Auto-triage cycle 2 ready-to-merge PR is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[From PR #243] Split large transformation_test.exs into smaller test files

1 participant