gnodet-bot commented on code in PR #2198:
URL: https://github.com/apache/maven-resolver/pull/2198#discussion_r4258744399
##########
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultInstallerTest.java:
##########
@@ -377,4 +379,90 @@ void testSetArtifactTimestamps() throws
InstallationException {
localArtifactFile.lastModified(),
"artifact timestamp was not set to src file");
}
+
+ @Test
+ void testHardLinkInstallCreatesHardLink() throws InstallationException,
IOException {
+ // default: hardLink=true — installer attempts a hard link, falls back
to copy on cross-device
+ //
+ // Use a counting PathProcessor to detect whether the fallback copy
path was taken.
+ // On same-device filesystems: hard link is created (copyWithTimestamp
NOT called).
+ // On cross-device (common in CI containers): Files.createLink throws,
copy IS taken.
+ CountingPathProcessor countingProcessor = new CountingPathProcessor();
+ installer = new DefaultInstaller(
+ countingProcessor,
+ new StubRepositoryEventDispatcher(),
+ Collections.emptyMap(),
+ Collections.emptyMap(),
+ Collections.emptyMap(),
+ new StubSyncContextFactory());
+
+ request.addArtifact(artifact);
+ installer.install(session, request);
+
+ assertTrue(localArtifactFile.exists());
+ assertEquals("artifact", TestFileUtils.readString(localArtifactFile));
+
+ // On same-device filesystems, verify a hard link was created (same
inode, no copy).
+ // On cross-device (common in CI containers where /tmp and project dir
are on different mounts),
+ // Files.createLink throws IOException and the installer falls back to
copy — both are correct.
+ Path srcPath = artifact.getFile().toPath();
+ Path dstPath = localArtifactFile.toPath();
+ try {
+ boolean sameDevice =
+ Files.getAttribute(srcPath,
"unix:dev").equals(Files.getAttribute(dstPath, "unix:dev"));
+ if (sameDevice) {
+ // hard link must have been created — same inode expected,
copy NOT taken
+ assertEquals(
+ Files.getAttribute(srcPath, "unix:ino"),
+ Files.getAttribute(dstPath, "unix:ino"),
+ "installed artifact should be a hard link to the
source on same-device FS");
+ assertEquals(
+ 0,
+ countingProcessor.copyWithTimestampCount,
+ "copyWithTimestamp must NOT be called on same-device
FS");
+ } else {
+ // cross-device: fallback to copy must have been taken
+ assertEquals(
+ 1,
+ countingProcessor.copyWithTimestampCount,
+ "copyWithTimestamp must be called as fallback on
cross-device FS");
+ }
+ } catch (UnsupportedOperationException | IllegalArgumentException
ignored) {
Review Comment:
⚠️ **Counting assertions silently skipped on non-POSIX FS (re-raised —
partially addressed)**
The `CountingPathProcessor` counting assertions (both the `assertEquals(0,
count...)` and `assertEquals(1, count...)`) are inside this `try` block. When
`Files.getAttribute(srcPath, "unix:dev")` throws
`UnsupportedOperationException` on a non-POSIX filesystem (Windows, Jimfs), the
catch fires and all counting assertions are silently skipped.
Maven-resolver CI is Linux-only, so the cross-device `else` branch now fires
correctly there ✅. But a Windows developer running locally gets zero behavioral
coverage from this test.
Fix: add a counting assertion in the `catch` block too —
`countingProcessor.copyWithTimestampCount` should be 0 or 1 regardless of FS
type:
```suggestion
} catch (UnsupportedOperationException | IllegalArgumentException
ignored) {
// non-POSIX filesystem — inode/device check not available
// but we can still assert that at most one copy was attempted
assertTrue(
countingProcessor.copyWithTimestampCount == 0
|| countingProcessor.copyWithTimestampCount == 1,
"expected 0 copies (hard link) or 1 copy (fallback),
got: "
+ countingProcessor.copyWithTimestampCount);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
