Skip to content

refactor(#4780): remove unnecessary ContainsFiles.java class and update simian.yml#4782

Merged
yegor256 merged 1 commit into
objectionary:masterfrom
volodya-lombrozo:4780-remove-contains-files
Dec 30, 2025
Merged

refactor(#4780): remove unnecessary ContainsFiles.java class and update simian.yml#4782
yegor256 merged 1 commit into
objectionary:masterfrom
volodya-lombrozo:4780-remove-contains-files

Conversation

@volodya-lombrozo

@volodya-lombrozo volodya-lombrozo commented Dec 29, 2025

Copy link
Copy Markdown
Member

This PR removes the unnecessary ContainsFiles class and updates the workflow configuration in simian.yml.

Fixes #4780

Summary by CodeRabbit

  • Refactor
    • Enhanced static code analysis coverage to detect additional code duplication patterns.
    • Consolidated test infrastructure by removing duplicate testing utilities and updating test assertions with standard file matchers.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 29, 2025 13:19
@github-actions github-actions Bot added the core label Dec 29, 2025
@coderabbitai

coderabbitai Bot commented Dec 29, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR removes a duplicate test utility class ContainsFiles from the integration tests and updates related tests to use standard FileMatchers instead. It also removes this class from the Simian code duplication scanner's exclusion list.

Changes

Cohort / File(s) Summary
Workflow Configuration
\.github/workflows/simian\.yml
Removed ContainsFiles.java from the Simian scan exclusion list, expanding the set of files scanned for code duplication.
Test Utility Removal
eo-integration-tests/src/test/java/org/eolang/maven/ContainsFiles\.java
Deleted the entire ContainsFiles class—a package-private TypeSafeMatcher<Path> that checked if paths contained files matching glob patterns.
Test Update
eo-integration-tests/src/test/java/org/eolang/maven/MjAssembleIT\.java
Replaced ContainsFiles matcher usage with standard FileMatchers.anExistingFile(), renamed helper method from appendItself to registerAssemble, and added import for org.hamcrest.io.FileMatchers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested labels

core

Suggested reviewers

  • yegor256

Poem

🐰 A duplicate matcher hops away,
FileMatchers now lead the test play,
Simian scans with expanded sight,
The code breathes easier, cleaner, bright!

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main changes: removing ContainsFiles.java class and updating simian.yml workflow configuration.
Linked Issues check ✅ Passed The PR fully addresses issue #4780 by removing the ContainsFiles class and its puzzle text, updating all references in MjAssembleIT to use FileMatchers instead.
Out of Scope Changes check ✅ Passed All changes are directly related to resolving issue #4780: removing ContainsFiles class, updating test references, and adjusting simian.yml scanning configuration.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e82e0e4 and 64606d3.

📒 Files selected for processing (3)
  • .github/workflows/simian.yml
  • eo-integration-tests/src/test/java/org/eolang/maven/ContainsFiles.java
  • eo-integration-tests/src/test/java/org/eolang/maven/MjAssembleIT.java
💤 Files with no reviewable changes (1)
  • eo-integration-tests/src/test/java/org/eolang/maven/ContainsFiles.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: CodeQL analysis (java)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: snippets
  • GitHub Check: mvn (ubuntu-24.04, 23)
  • GitHub Check: mvn (macos-15, 23)
  • GitHub Check: mvn (ubuntu-24.04, 17)
  • GitHub Check: mvn (windows-2022, 23)
  • GitHub Check: benchmark
  • GitHub Check: qulice
  • GitHub Check: pr-size
  • GitHub Check: ort
  • GitHub Check: trufflehog
  • GitHub Check: integration
🔇 Additional comments (5)
.github/workflows/simian.yml (1)

24-24: LGTM! Exclusion removed for deleted file.

The removal of the ContainsFiles.java exclusion is correct since the file has been deleted. This appropriately broadens the Simian duplication scanner's coverage.

eo-integration-tests/src/test/java/org/eolang/maven/MjAssembleIT.java (4)

18-18: LGTM! Import added for new FileMatchers usage.

The import is necessary for the FileMatchers.anExistingFile() matcher used in the updated assertion on line 88.


42-42: LGTM! Method rename improves clarity.

The rename from appendItself to registerAssemble better reflects the Maven goals being executed ("register", "assemble") and improves code readability.

Also applies to: 83-83


87-88: LGTM! Assertion updated to use standard FileMatchers.

The replacement of ContainsFiles with FileMatchers.anExistingFile() is appropriate. The path construction target/eo/1-parse/one/main.xmir correctly follows Maven's convention for the parse phase output, matching the package name "one" and source file "main.eo".


107-110: LGTM! Method signature reflects its purpose.

The method rename from appendItself to registerAssemble makes the code more self-documenting and aligns with the Maven goals being executed.


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.

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 removes the duplicate ContainsFiles class from the eo-integration-tests module and updates related test code to use the standard Hamcrest FileMatchers instead. This resolves issue #4780 by eliminating code duplication, as the same class already exists in the eo-maven-plugin module.

Key Changes:

  • Removed the duplicate ContainsFiles.java class from eo-integration-tests
  • Replaced glob-based file matching with direct file existence checking using FileMatchers.anExistingFile()
  • Updated Simian workflow configuration to remove the now-unnecessary exclusion for ContainsFiles.java

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
eo-integration-tests/src/test/java/org/eolang/maven/ContainsFiles.java Deleted duplicate helper class that was redundant with the version in eo-maven-plugin
eo-integration-tests/src/test/java/org/eolang/maven/MjAssembleIT.java Updated test to use standard FileMatchers.anExistingFile() instead of custom ContainsFiles matcher, and renamed appendItself method to the more descriptive registerAssemble
.github/workflows/simian.yml Removed the exclusion for ContainsFiles.java since the duplicate no longer exists

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

@github-actions

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 157.813 200.834 43.021 27.26% ms/op Average Time

⚠️ Performance loss: benchmarks.XmirBench.xmirToEO is slower by 43.021 ms/op (27.26%)

@volodya-lombrozo

Copy link
Copy Markdown
Member Author

@yegor256 could you take a look, please?

@yegor256 yegor256 merged commit 0bfd390 into objectionary:master Dec 30, 2025
32 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unnecessary {@link ContainsFiles} in ContainsFiles.java Needs Removal

3 participants