You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/05/17 22:16:36 UTC

[GitHub] [maven-shared-utils] roxspring commented on a change in pull request #31: [MSHARED-681] createSymbolicLink() overwrites existing different symlinks

roxspring commented on a change in pull request #31:
URL: https://github.com/apache/maven-shared-utils/pull/31#discussion_r426311011



##########
File path: src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
##########
@@ -1475,6 +1475,30 @@ public void createAndReadSymlink()
         Files.delete( file.toPath() );
     }
 
+    @Test
+    public void createSymbolicLinkWithDifferentTargetOverwritesSymlink()
+            throws Exception
+    {
+        assumeThat( System.getProperty( "os.name" ), not( startsWith( "Windows" ) ) );

Review comment:
       An understandable concern. Can the Jenkins results be seen somewhere obvious? (it's a shame they're not linked from the PR)
   
   FWIW the condition was copied from the symlink test immediately above, which itself was moved from `Java7SupportTest` only yesterday. Prior to that it appears to have been unchanged here for 3 years: https://github.com/apache/maven-shared-utils/blob/bb2f85e98c3c651ae50b7f642500cb74f50abb0d/src/test/java/org/apache/maven/shared/utils/io/Java7SupportTest.java#L50
   
   On closer inspection it seems that `FileUtilsTest` is already making Windows related assumptions in a variety of ways:
   
   - https://github.com/apache/maven-shared-utils/blob/bb2f85e98c3c651ae50b7f642500cb74f50abb0d/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java#L446
   - https://github.com/apache/maven-shared-utils/blob/92a5ba3c431affcabb2cebd11ca17fe02b15748c/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java#L1206
   - https://github.com/apache/maven-shared-utils/blob/92a5ba3c431affcabb2cebd11ca17fe02b15748c/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java#L1461
   - https://github.com/apache/maven-shared-utils/blob/92a5ba3c431affcabb2cebd11ca17fe02b15748c/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java#L1432
   
   If there's a preferred approach to such assumptions then I'm happy to unify on that - but perhaps in a separate PR?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org