refactor(#4862): remove unnecessary FpDefault references from tests by volodya-lombrozo · Pull Request #4872 · objectionary/eo

volodya-lombrozo · 2026-02-10T11:22:30Z

This PR removes unnecessary references to FpDefault in the eo-maven-plugin test code, resolving puzzle 4851-2fc47960.

Fixes #4862

Summary by CodeRabbit

Tests
- Removed obsolete internal test cases and helper scaffolding, simplifying the test suite while keeping the core cache-behavior test intact.
Refactor
- Pruned internal footprint/caching implementation details; no public API or user-visible behavior changes.

coderabbitai · 2026-02-10T11:22:52Z

📝 Walkthrough

Walkthrough

This change removes FpDefault and three related footprint classes plus a test file, deleting their implementations and many test helpers; no public API declarations were altered. Remaining tests and other code are left intact. (50 words)

Changes

Cohort / File(s)	Summary
Tests removed `eo-maven-plugin/src/test/java/org/eolang/maven/FpDefaultTest.java`	Deleted the test file and its private helpers (cache helpers, footprint helpers, existence helpers, makeOlder overloads, and private `Cache` inner class). One existing test and setup remain elsewhere.
FpDefault implementation `eo-maven-plugin/src/main/java/org/eolang/maven/FpDefault.java`	Removed the package-private `FpDefault` class and all its constructors; eliminates composed footprint logic for generated content, cache paths, SNAPSHOT/version handling, and update/copy decisions.
Existence guard removed `eo-maven-plugin/src/main/java/org/eolang/maven/FpExistedSource.java`	Removed `FpExistedSource`, which validated source existence and threw if missing, and delegated to wrapped footprints.
Conditional-age wrapper removed `eo-maven-plugin/src/main/java/org/eolang/maven/FpIfOlder.java`	Removed `FpIfOlder`, which composed conditional footprint behavior based on target existence and age (wrapping `FpIfTargetExists` / `FpIfTargetOlder`).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

refactor(#4851): replace FpDefault with Cache in caching logic #4861: Refactor replacing FpDefault with new cache/placement approach — touches same footprint/cache logic.
refactor(#4862): remove unnecessary FpDefault references from tests #4868: Removes FpDefault-related tests and references — overlaps test deletions.
test(#4743): enable tests related to caching for '0.0.0' and 'SNAPSHOT' versions in FpDefaultTest #4768: Updates cache-related tests that exercised FpDefault behavior — strong test-level relation.

Suggested labels

core

Suggested reviewers

yegor256
maxonfjvipon

Poem

🐇
I nibbled old footprints in the night,
Hid the worn paths out of sight,
Cache crumbs swept, helpers tucked away,
One brave test still hops to play,
A lighter burrow, fresh daylight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions removing 'FpDefault references from tests', but the changeset removes entire classes (FpDefault, FpExistedSource, FpIfOlder) and their test files from the main and test source code, not just test references.	Update the title to accurately reflect that entire classes and test files are being removed, such as 'refactor(`#4862`): Remove unused FpDefault, FpExistedSource, and FpIfOlder classes' or 'refactor(`#4862`): Clean up unused footprint strategy classes'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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

No actionable comments were generated in the recent 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 and usage tips.}

github-actions · 2026-02-10T11:25:12Z

🚀 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`	202.401	164.465	-37.936	-18.74%	ms/op	Average Time

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

Copilot

Pull request overview

This PR updates eo-maven-plugin test code to reduce/remove usages of FpDefault by deleting several FpDefaultTest test cases and related helper utilities.

Changes:

Removed two FpDefaultTest cases that exercised caching behavior for 1.0-SNAPSHOT and 0.0.0.
Removed multiple private helper methods and an inner Cache helper class previously used by those tests.

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

Copilot · 2026-02-10T11:27:35Z

@@ -206,66 +103,4 @@ private static void makeOlder(final Path file, final long time) throws IOExcepti
            FileTime.fromMillis(System.currentTimeMillis() + time)


makeOlder(...) currently sets the last-modified time to System.currentTimeMillis() + time, which makes the file newer (in the future), not older as the method name/Javadoc claim. Either subtract the offset (to truly make it older) or rename/update the Javadoc so the helper’s intent is unambiguous.

Suggested change

FileTime.fromMillis(System.currentTimeMillis() + time)

FileTime.fromMillis(System.currentTimeMillis() - time)

Copilot · 2026-02-10T11:27:35Z

@@ -27,57 +27,6 @@
 @ExtendWith(MktmpResolver.class)
 final class FpDefaultTest {


The PR description/linked issue indicates the puzzle is resolved when the TODO in FpDefault.java is removed, but this PR only removes some test usages and still leaves FpDefault referenced (and the source TODO remains). Either update the PR scope/description or include the corresponding production-code change so the referenced puzzle is actually addressed.

Copilot · 2026-02-10T11:27:35Z

 @ExtendWith(MktmpResolver.class)
 final class FpDefaultTest {

-    @Test
-    void usesCacheEvenItIsSnapshot(@Mktmp final Path temp) throws Exception {
-        final Path source = FpDefaultTest.existedSource(temp);
-        final Path target = FpDefaultTest.notExistedTarget(temp);
-        final String version = "1.0-SNAPSHOT";
-        final String hash = "snapshothash";
-        final Cache cache = FpDefaultTest.notExistedCache(temp, version, hash);
-        FpDefaultTest.existedFile(cache.path(), FpDefaultTest.cacheContent());
-        FpDefaultTest.makeOlder(cache.path());
-        new FpDefault(
-            src -> FpDefaultTest.footprintContent(),
-            temp.resolve("cache"),
-            version,
-            hash,
-            Path.of("")
-        ).apply(source, target);
-        MatcherAssert.assertThat(
-            "We expect that cache is used even for SNAPSHOT version",
-            new TextOf(target).asString(),
-            Matchers.allOf(
-                Matchers.equalTo(FpDefaultTest.cacheContent()),
-                Matchers.not(Matchers.equalTo(FpDefaultTest.footprintContent()))
-            )
-        );
-    }
-
-    @Test
-    void cachesEvenItIsZeroVersion(@Mktmp final Path temp) throws Exception {
-        final Path source = FpDefaultTest.existedSource(temp);
-        final Path target = FpDefaultTest.notExistedTarget(temp);
-        final String version = "0.0.0";
-        final String hash = "zerohash";
-        final Cache cache = FpDefaultTest.notExistedCache(temp, version, hash);
-        FpDefaultTest.existedFile(cache.path(), FpDefaultTest.cacheContent());
-        FpDefaultTest.makeOlder(cache.path());
-        new FpDefault(
-            src -> FpDefaultTest.footprintContent(),
-            temp.resolve("cache"),
-            version,
-            hash,
-            Path.of("")
-        ).apply(source, target);
-        MatcherAssert.assertThat(
-            "We expect that cache is used even for 0.0.0 version",
-            new TextOf(target).asString(),
-            Matchers.allOf(
-                Matchers.equalTo(FpDefaultTest.cacheContent()),
-                Matchers.not(Matchers.equalTo(FpDefaultTest.footprintContent()))
-            )
-        );
-    }

    /**
     * We should use the transpilation cache if it existed before transpilation.


This change removes the only enabled FpDefault tests from this class; the remaining test in FpDefaultTest is @Disabled, leaving no active coverage for FpDefault behavior. If FpDefault is still expected to work (even as test-only code), consider keeping at least one enabled regression test or deleting the entire class along with FpDefault itself.

Copilot · 2026-02-10T11:27:36Z

            FileTime.fromMillis(System.currentTimeMillis() + time)
        );
    }


makeOlder uses Files.setLastModifiedTime(...) on the provided path; if callers pass a directory (as the current test does), it won’t change the cache file’s timestamp that the caching logic compares. Consider either enforcing/validating that file is the actual cache file path (or updating helper name/docs/call sites accordingly) so the test setup matches what the production code checks.

…rom FpDefaultTest

volodya-lombrozo · 2026-02-10T11:58:53Z

@yegor256 could you have a look, please?

…nd FpIfOlder classes

0crat · 2026-02-12T13:32:17Z

@volodya-lombrozo Thanks for the contribution! You've earned +4 points for this: +16 as a basis; -4 for too many hits-of-code (485 >= 100); -8 for way too many hits-of-code (485 >= 400). Please, keep them coming. Your running score is +448; don't forget to check your Zerocracy account too).

Copilot AI review requested due to automatic review settings February 10, 2026 11:22

Copilot started reviewing on behalf of volodya-lombrozo February 10, 2026 11:23 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

refactor(objectionary#4862): Remove unused tests and helper methods f…

5912526

…rom FpDefaultTest

volodya-lombrozo force-pushed the 4862-remove-fpdefault branch from 45e3f98 to 5912526 Compare February 10, 2026 11:29

volodya-lombrozo requested a review from yegor256 February 10, 2026 12:29

chore(objectionary#4862): Remove unused FpDefault, FpExistedSource, a…

9f5c1c4

…nd FpIfOlder classes

yegor256 merged commit f6c8e22 into objectionary:master Feb 11, 2026
25 of 26 checks passed

		@@ -206,66 +103,4 @@ private static void makeOlder(final Path file, final long time) throws IOExcepti
		FileTime.fromMillis(System.currentTimeMillis() + time)

		@@ -27,57 +27,6 @@
		@ExtendWith(MktmpResolver.class)
		final class FpDefaultTest {

Uh oh!

Conversation

volodya-lombrozo commented Feb 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

github-actions Bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Performance Analysis

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

volodya-lombrozo commented Feb 10, 2026

Uh oh!

Uh oh!

0crat commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

volodya-lombrozo commented Feb 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 10, 2026 •

edited

Loading

github-actions Bot commented Feb 10, 2026 •

edited

Loading