[MSHADE-322][MSHADE-306] fixing Tibor's review comments and ensure ma…

…ster tests are deterministic
apache · Aug 14, 2019 · 57d78de · 57d78de
1 parent fbefa16
commit 57d78de
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 10 deletions.
diff --git a/src/main/java/org/apache/maven/plugins/shade/resource/properties/SortedProperties.java b/src/main/java/org/apache/maven/plugins/shade/resource/properties/SortedProperties.java
@@ -23,16 +23,19 @@
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Enumeration;
-import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
 
 /**
- * Properties instance sorting its keys on iterations.
+ * Internal Properties instance sorting its keys on iterations for store() usages.
+ * It ensures properties persistence is deterministic.
+ *
+ * IMPORTANT: this only overrides methods used accross JVM in store() so ordering is not guaranteed for other cases.
  */
 public class SortedProperties extends Properties
 {
@@ -48,7 +51,7 @@ public int compare( Map.Entry<Object, Object> o1, Map.Entry<Object, Object> o2 )
                 return String.valueOf( o1.getKey() ).compareTo( String.valueOf( o2.getKey() ) );
             }
         } );
-        return new HashSet<>( entries );
+        return new LinkedHashSet<>( entries );
     }
 
     @Override

diff --git a/src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java b/src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java
@@ -65,7 +65,7 @@ public void testOverlappingResourcesAreLogged() throws IOException, MojoExecutio
             @Override
             public void debug( final String s, final Throwable throwable )
             {
-                debugMessages.add(s);
+                debugMessages.add( s.replace( '\\', '/' ).trim() );
             }
 
             @Override
@@ -77,7 +77,7 @@ public void info( final String s, final Throwable throwable )
             @Override
             public void warn( final String s, final Throwable throwable )
             {
-                warnMessages.add(s);
+                warnMessages.add( s.replace( '\\', '/' ).trim() );
             }
 
             @Override
@@ -101,7 +101,7 @@ public Logger getChildLogger( final String s )
 
         // we will shade two jars and expect to see META-INF/MANIFEST.MF overlaps, this will always be true
         // but this can lead to a broken deployment if intended for OSGi or so, so even this should be logged
-        final Set<File> set = new LinkedHashSet<File>();
+        final Set<File> set = new LinkedHashSet<>();
         set.add( new File( "src/test/jars/test-project-1.0-SNAPSHOT.jar" ) );
         set.add( new File( "src/test/jars/plexus-utils-1.4.1.jar" ) );
 
@@ -115,13 +115,12 @@ public Logger getChildLogger( final String s )
 
         final String failureWarnMessage = warnMessages.toString();
         assertTrue(failureWarnMessage, warnMessages.contains(
-                "plexus-utils-1.4.1.jar, test-project-1.0-SNAPSHOT.jar define 1 overlapping resources: "));
-        assertTrue(failureWarnMessage, warnMessages.contains("  - META-INF/MANIFEST.MF"));
+                "plexus-utils-1.4.1.jar, test-project-1.0-SNAPSHOT.jar define 1 overlapping resources:"));
+        assertTrue(failureWarnMessage, warnMessages.contains("- META-INF/MANIFEST.MF"));
 
         final String failureDebugMessage = debugMessages.toString();
         assertTrue(failureDebugMessage, debugMessages.contains(
-                "We have a duplicate META-INF/MANIFEST.MF in src/test/jars/plexus-utils-1.4.1.jar"
-                        .replace('/', File.separatorChar)));
+                "We have a duplicate META-INF/MANIFEST.MF in src/test/jars/plexus-utils-1.4.1.jar" ));
     }
 
     public void testShaderWithDefaultShadedPattern()