Skip to content

fix(#4863): resolve failed tests in MjLintTest.java related to caching#4864

Merged
yegor256 merged 1 commit into
objectionary:masterfrom
volodya-lombrozo:4863-fix-mjlint-tests
Feb 9, 2026
Merged

fix(#4863): resolve failed tests in MjLintTest.java related to caching#4864
yegor256 merged 1 commit into
objectionary:masterfrom
volodya-lombrozo:4863-fix-mjlint-tests

Conversation

@volodya-lombrozo

@volodya-lombrozo volodya-lombrozo commented Feb 6, 2026

Copy link
Copy Markdown
Member

This PR resolves the puzzle 4851-f3d235c3 by enabling tests in MjLintTest.java related to caching.

Fixes #4863

Summary by CodeRabbit

  • Tests
    • Re-enabled previously disabled linting tests.
    • Switched staleness checks to use embedded time-attributes instead of external timestamps.
    • Updated cache-related tests to verify cached artifact paths and to persist input content before validation.
    • Clarified assertion messages and reorganized test content for more accurate, reliable cache and verification testing.

Copilot AI review requested due to automatic review settings February 6, 2026 10:34
@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Re-enabled tests in MjLintTest.java and refactored test logic to stop using FileTime-based staleness checks, switching to XMIR time-attribute checks and a Cache/CachePath-based caching verification flow.

Changes

Cohort / File(s) Summary
Test Refactoring
eo-maven-plugin/src/test/java/org/eolang/maven/MjLintTest.java
Removed @Disabled annotations. Replaced FileTime/last-modified checks with XMIR time-attribute XPath assertions. Reworked cache-related test flows to use Cache/CachePath and a content-supplying lambda; updated assertions and test organization accordingly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

core

Suggested reviewers

  • yegor256

Poem

🐇✨ I hopped through tests that once were stalled,
XMIR times now checked—no clocks involved.
Cache paths snug, content fed on cue,
Green lights dance where red once grew. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: enabling and fixing failed tests in MjLintTest.java related to caching, which directly matches the changeset.
Linked Issues check ✅ Passed The PR successfully resolves issue #4863 by removing the puzzle text and re-enabling the disabled tests in MjLintTest.java, meeting the stated completion criterion.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the disabled tests and refactoring the caching mechanism within MjLintTest.java, with no unrelated modifications introduced.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@github-actions

github-actions Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

🚀 Performance Analysis

All benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information.

Click to see the detailed report
Test Base Score PR Score Change % Change Unit Mode
benchmarks.XmirBench.xmirToEO 183.304 178.659 -4.645 -2.53% ms/op Average Time

✅ Performance gain: benchmarks.XmirBench.xmirToEO is faster by 4.645 ms/op (2.53%)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjLintTest.java`:
- Around line 241-248: The closing parenthesis for the new XMLDocument(...)
expression in MjLintTest is on the same line as the surrounding Matcher call
which trips Qulice's BracketsStructureCheck; move the final ")" that closes new
XMLDocument(...) to its own line so the XMLDocument constructor call ends with a
lone closing parenthesis line (adjust the block containing new
XMLDocument(cache.resolve(MjLint.CACHE)...resolve("foo/x/main.xmir"))
accordingly) ensuring proper indentation and keeping the surrounding
Matchers.is(...) unchanged.

Comment thread eo-maven-plugin/src/test/java/org/eolang/maven/MjLintTest.java

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR resolves puzzle 4851-f3d235c3 by re-enabling and fixing three disabled test methods in MjLintTest.java that were broken due to changes in the caching logic. The tests have been updated to work with the new Cache and CachePath API introduced in recent refactoring.

Changes:

  • Re-enabled three previously disabled caching-related tests by removing @Disabled annotations
  • Updated test implementation to use new caching mechanism (Cache, CachePath, Xnav classes)
  • Changed skipsAlreadyLinted test to verify caching by checking XML @time attribute instead of file modification time
  • Removed TODO puzzle comment that documented the disabled tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@volodya-lombrozo

Copy link
Copy Markdown
Member Author

@yegor256 could you have a look, please?

@volodya-lombrozo

Copy link
Copy Markdown
Member Author

@yegor256 friendly reminder

@yegor256 yegor256 merged commit 9e9a2a9 into objectionary:master Feb 9, 2026
26 checks passed
@0crat

0crat commented Feb 9, 2026

Copy link
Copy Markdown

@volodya-lombrozo Hey! Nice work on your contribution – you've snagged +16 points this time! 🎉 The policy rewards solid contributions like yours, and you're doing great by keeping your code contributions within that sweet spot (avoiding those pesky deductions for oversized changes or missing reviews). Your running score is sitting pretty at +390, so keep the momentum going! Don't forget to peek at your Zerocracy account when you get a chance.

1 similar comment
@0crat

0crat commented Feb 9, 2026

Copy link
Copy Markdown

@volodya-lombrozo Hey! Nice work on your contribution – you've snagged +16 points this time! 🎉 The policy rewards solid contributions like yours, and you're doing great by keeping your code contributions within that sweet spot (avoiding those pesky deductions for oversized changes or missing reviews). Your running score is sitting pretty at +390, so keep the momentum going! Don't forget to peek at your Zerocracy account when you get a chance.

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.

Failed Tests in MjLintTest.java Need Resolution

4 participants