Skip to content

Mock Maven in NPath unit tests#1230

Open
somepatt wants to merge 1 commit into
cqfn:masterfrom
somepatt:fix-702-mock-mvn-in-tests
Open

Mock Maven in NPath unit tests#1230
somepatt wants to merge 1 commit into
cqfn:masterfrom
somepatt:fix-702-mock-mvn-in-tests

Conversation

@somepatt

@somepatt somepatt commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the Maven subprocess in test/metrics/npath/test_all_types.py with a stub that writes PMD XML fixtures
  • keep coverage of parsed low, medium, high, and PMD error results without requiring a local mvn
  • preserve the missing-file failure path as a real filesystem check

Problem

The top-level NPath tests exercise NPathMetric.value() by actually invoking mvn pmd:pmd, which makes them integration tests rather than unit tests and requires Maven to be installed locally.

Fix

Patch the NPath metric module’s temporary directory, UUID, and subprocess.run() in the affected tests. The stubbed subprocess writes a synthetic target/pmd.xml report, so the tests still execute the real parsing path without spawning Maven.

Verification

  • TMPDIR=/Users/iliaprokofev/.tmp-codex-aibolit python3 -B -m pytest test/metrics/npath/test_all_types.py -q
  • python3 -B -m flake8 --max-line-length=120 test/metrics/npath/test_all_types.py
  • TMPDIR=/Users/iliaprokofev/.tmp-codex-aibolit python3 -B -m pylint test/metrics/npath/test_all_types.py

Closes #702

Summary by CodeRabbit

  • Tests
    • Updated NPath metric tests to use a stubbed report instead of relying on an external execution path.
    • Added coverage for medium-score cases and improved assertions for low, high, and invalid-format inputs.
    • Clarified the missing-file test with a more explicit exception check.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The NPath metric tests now stub Maven/PMD execution, generate temporary pmd.xml reports for multiple fixtures, and update the missing-file assertion in the same test module.

Changes

NPath metric test isolation

Layer / File(s) Summary
Stub PMD execution and score assertions
test/metrics/npath/test_all_types.py
The tests monkeypatch Maven/PMD execution to write a fake pmd.xml, then assert parsed NPath values for the incorrect-format, low-score, medium-score, and high-score fixtures, with the missing-file assertion updated in the same module.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through tests with a whisker of glee,
and tucked a fake pmd.xml under the tree.
No mvn needed for the score parade,
just bunny-safe checks that were swiftly made.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: mocking Maven in NPath unit tests.
Linked Issues check ✅ Passed The PR removes real mvn execution from the four affected NPath tests by stubbing subprocess and exercising parsing paths.
Out of Scope Changes check ✅ Passed The added helper and fixture updates stay focused on the NPath test mocking change.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
test/metrics/npath/test_all_types.py (1)

17-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Stub correctly mirrors the real value() contract. gettempdirstr(tmp_path) and uuid4().hex'fake-run' reproduce os.path.join(tempfile.gettempdir(), uuid.uuid4().hex), and fake_run writes to {cwd}/target/pmd.xml, matching the subprocess.run(..., cwd=root) call.

One small robustness note: fake_run reads kwargs['cwd'], which only works because the production call passes cwd as a keyword. If that call ever switches to positional, this stub will KeyError instead of failing clearly.

🤖 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 `@test/metrics/npath/test_all_types.py` around lines 17 - 36, The `fake_run`
stub in `test_all_types.py` assumes `subprocess.run` always receives `cwd` as a
keyword argument, so it can raise an unhelpful `KeyError` if the call shape
changes. Update `fake_run` to handle the current `npath_main.subprocess.run(...,
cwd=root)` usage via the `cwd` keyword while remaining defensive by deriving the
target directory from the passed arguments in a safe way, keeping the stub
aligned with `_patch_maven_run` and `npath_main`.
🤖 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.

Nitpick comments:
In `@test/metrics/npath/test_all_types.py`:
- Around line 17-36: The `fake_run` stub in `test_all_types.py` assumes
`subprocess.run` always receives `cwd` as a keyword argument, so it can raise an
unhelpful `KeyError` if the call shape changes. Update `fake_run` to handle the
current `npath_main.subprocess.run(..., cwd=root)` usage via the `cwd` keyword
while remaining defensive by deriving the target directory from the passed
arguments in a safe way, keeping the stub aligned with `_patch_maven_run` and
`npath_main`.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ec71d674-63ec-432a-8921-b9a7166b2395

📥 Commits

Reviewing files that changed from the base of the PR and between 6d5c676 and 6f6fff2.

📒 Files selected for processing (1)
  • test/metrics/npath/test_all_types.py

@somepatt

Copy link
Copy Markdown
Contributor Author

@yegor256 please review this PR.
It removes the real Maven dependency from the NPath unit tests while keeping the PMD XML parsing path covered.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some unit tests depend on actually running mvn

1 participant