Skip to content

#1230: Mock Maven in NPath unit tests#1247

Merged
yegor256 merged 1 commit into
cqfn:masterfrom
Beeeeeeee-c:fix-issue-1230-mock-npath-maven
Jul 14, 2026
Merged

#1230: Mock Maven in NPath unit tests#1247
yegor256 merged 1 commit into
cqfn:masterfrom
Beeeeeeee-c:fix-issue-1230-mock-npath-maven

Conversation

@Beeeeeeee-c

@Beeeeeeee-c Beeeeeeee-c commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Tests
    • Expanded automated coverage for metric compatibility checks by filtering out excluded metrics and validating AST-related signatures on the remaining set.
    • Improved NPath metric test coverage with mocked report generation, including error handling and multiple complexity scenarios.
    • Re-enabled previously skipped pipeline and stats tests, and updated one expected output to match current results.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR updates several test files: it removes an xfail marker and adds metric-exclusion filtering to an AST compatibility test, mocks subprocess.run in NPath tests using new PMD-report-writing helpers, and removes skip decorators (unskipping tests) in the recommend pipeline and stats test suites, updating one expected hash value.

Changes

Test Suite Fixes and Unskipping

Layer / File(s) Summary
AST compatibility test filtering
test/config/test_config.py
Removes pytest/xfail usage and filters patterns_config['metrics'] by excluded codes from metrics_exclude before asserting ast: AST signature compliance.
NPath tests mock PMD subprocess output
test/metrics/npath/test_all_types.py
Adds subprocess import and helper functions (_write_pmd_report, _write_pmd_error_report, _write_pmd_complexity_report) to synthesize PMD XML; patches subprocess.run in testIncorrectFormat, testLowScore, testHighScore, and testMediumScore.
Unskip flaky recommend and stats pipeline tests
test/recommend/test_recommend_pipeline.py, test/stats/test_stats.py
Removes unused skip import and @skip(...) decorators, enabling previously skipped tests, and updates the expected MD5 hash for test_text_format_sort_by_code_line.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

  • cqfn/aibolit#814: Both PRs modify the AST interface contract tests in test/config/test_config.py around the ast: AST compliance checks.
  • cqfn/aibolit#832: Both PRs modify test/config/test_config.py's AST-compatibility assertions, removing xfail/pytest usage.
  • cqfn/aibolit#866: Both PRs modify the NPath test suite in test/metrics/npath/test_all_types.py regarding PMD-derived complexity values.

Suggested reviewers: ivanovmg, yegor256

🚥 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 is concise and matches the main change in the NPath tests, which now mock Maven/PMD execution.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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)

28-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fake subprocess.run stub tightly bound to production call signature.

fake_run(args, cwd, check, **kwargs) requires cwd and check to be passed as they currently are in aibolit/metrics/npath/main.py (subprocess.run(['mvn', 'pmd:pmd'], cwd=root, check=True, ...)). If that call signature ever changes (e.g., drops check or passes cwd positionally in a different order), these mocks will raise a TypeError instead of a meaningful assertion failure.

♻️ Optional: decouple stub from exact kwarg names
-    def fake_run(args, cwd, check, **kwargs):
+    def fake_run(*args, cwd, **kwargs):
🤖 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 28 - 52, The fake
subprocess.run helpers in _write_pmd_error_report and
_write_pmd_complexity_report are too tightly coupled to the current call
signature, so make fake_run accept arbitrary positional and keyword arguments
instead of requiring cwd and check. Update the stub to extract only the values
it needs from kwargs and avoid assuming the exact argument order used by
aibolit/metrics/npath/main.py, so the tests fail with assertions rather than
TypeError when the production call changes.
🤖 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 28-52: The fake subprocess.run helpers in _write_pmd_error_report
and _write_pmd_complexity_report are too tightly coupled to the current call
signature, so make fake_run accept arbitrary positional and keyword arguments
instead of requiring cwd and check. Update the stub to extract only the values
it needs from kwargs and avoid assuming the exact argument order used by
aibolit/metrics/npath/main.py, so the tests fail with assertions rather than
TypeError when the production call changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 83159d80-c236-42b7-a5e7-12ed1e1e99b8

📥 Commits

Reviewing files that changed from the base of the PR and between 0b0555f and a63232c.

⛔ Files ignored due to path filters (1)
  • test/stats/results_test.csv is excluded by !**/*.csv
📒 Files selected for processing (4)
  • test/config/test_config.py
  • test/metrics/npath/test_all_types.py
  • test/recommend/test_recommend_pipeline.py
  • test/stats/test_stats.py

@yegor256 yegor256 merged commit 936c96e into cqfn:master Jul 14, 2026
17 checks passed
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.

3 participants