Skip to content

test: cover move operation behavior - #2076

Closed
vitali87 wants to merge 1 commit into
pr-split/split-pr1549-source/pr-6from
pr-split/split-pr1549-source/pr-9
Closed

vitali87 wants to merge 1 commit into
pr-split/split-pr1549-source/pr-6from
pr-split/split-pr1549-source/pr-9

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Cover move application, cycle refusal, aliases, import presence, and shared fixtures.

Files changed

  • codebase_rag/tests/move_helpers.py
  • codebase_rag/tests/test_move_operation.py
  • codebase_rag/tests/test_move_import_presence.py

Diff stats

+330 additions, -0 deletions (330 LOC)

Dependencies

This PR depends on: pr-6

Dependency graph

Merge in this order:

pr-1: feat: add move public API and constants
├── pr-2: feat: parse move spans and imports
│   └── pr-5: feat: retarget move imports safely
│       └── pr-6: feat: implement move refactoring operation
│           ├── pr-7: feat: expose move through CLI
│           │   └── pr-11: docs: document move operation
│           ├── pr-8: feat: expose move through MCP
│           │   ├── pr-10: test: cover move MCP wiring
│           │   └── pr-11: docs: document move operation
│           └── pr-9: test: cover move operation behavior  <-- this PR
│               └── pr-10: test: cover move MCP wiring
├── pr-3: feat: expose import graph sites for move
│   └── pr-5: feat: retarget move imports safely
│       └── pr-6: feat: implement move refactoring operation
│           ├── pr-7: feat: expose move through CLI
│           │   └── pr-11: docs: document move operation
│           ├── pr-8: feat: expose move through MCP
│           │   ├── pr-10: test: cover move MCP wiring
│           │   └── pr-11: docs: document move operation
│           └── pr-9: test: cover move operation behavior  <-- this PR
│               └── pr-10: test: cover move MCP wiring
└── pr-4: feat: detect move contract deltas
    └── pr-5: feat: retarget move imports safely
        └── pr-6: feat: implement move refactoring operation
            ├── pr-7: feat: expose move through CLI
            │   └── pr-11: docs: document move operation
            ├── pr-8: feat: expose move through MCP
            │   ├── pr-10: test: cover move MCP wiring
            │   └── pr-11: docs: document move operation
            └── pr-9: test: cover move operation behavior  <-- this PR
                └── pr-10: test: cover move MCP wiring

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^feat/.*
  • ^fix/.*

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e6a70200-67a5-44f8-b958-919d2c6e6ff6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

Safe to merge: the exercised move flow preserves the module-qualified caller and its runtime result.

What we checked:

  • The module-qualified move validation was executed using the fixture where pkg/c.py calls pkg.util.helper(['q']) through a module-qualified reference. T-Rex
  • The before-move state showed pkg.c.via_module() returned q. T-Rex
  • After moving pkg.util.helper to pkg.core, pkg.c.py imported pkg.core and called pkg.core.helper(['q']); pkg.c.via_module() still returned q. T-Rex
  • The move report confirmed the change was applied and listed the importers (pkg/a.py and pkg/b.py). T-Rex
  • Artifacts documenting the module-qualified move validation and its related logs were created for reviewer inspection. T-Rex

Summary

  • Adds shared fixtures and focused coverage for move operations and import-binding detection.
  • Verified that moving pkg.util.helper to pkg.core updates the module-qualified caller in pkg/c.py and preserves pkg.c.via_module() behavior.

No merge-blocking issues were found.

Reviews (1) · Last reviewed commit: "test: cover move operation behavior"

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Add coverage for pkg.util.helper retargeting and correct the misleading test comment.

Review effort: Lite
Findings: None

What changed in this PR

Adds tests for move refactoring behavior, including aliases, cycles, import handling, and shared fixtures.

Changes:

  • Adds reusable move-operation fixtures and helpers.
  • Covers successful moves, refusal paths, aliases, cycles, and existing modules.
  • Tests module import detection edge cases.
File Summary
codebase_rag/​tests/​test_move_operation.py End-to-end move behavior tests; missing assertions for attribute-call retargeting and import updates, plus a misleading comment.
codebase_rag/​tests/​test_move_import_presence.py Tests module import-presence detection.
codebase_rag/​tests/​move_helpers.py Shared fixtures and repository helpers.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

vitali87 commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded: this is part of the split of #1549, which was closed without merging, and it targets the stale feat/change-signature branch rather than main. The change_signature base was rebuilt on main as #1912 (now stacked as #2161–#2169). The branch is kept, so the move-op work can be rebased onto main if it's picked up again.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants