[ 
https://issues.apache.org/jira/browse/MSHARED-1290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17778622#comment-17778622
 ] 

ASF GitHub Bot commented on MSHARED-1290:
-----------------------------------------

elharo commented on code in PR #78:
URL: https://github.com/apache/maven-filtering/pull/78#discussion_r1368496934


##########
src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java:
##########
@@ -39,40 +38,40 @@ public class PropertyUtilsTest extends TestSupport {
     private static File testDirectory = new File(getBasedir(), 
"target/test-classes/");
 
     public void testBasic() throws Exception {
-        File basicProp = new File(testDirectory, "basic.properties");
+        File basicProp = File.createTempFile("basic", ".properties");
 
-        if (basicProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(basicProp), StandardCharsets.UTF_8)) {
+                writer.write("ghost=${non_existent}\n");
+                writer.write("key=${untat_na_damgo}\n");
+                writer.write("untat_na_damgo=gani_man\n");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false, logger);

Review Comment:
   prop --> properties



##########
src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java:
##########
@@ -39,40 +38,40 @@ public class PropertyUtilsTest extends TestSupport {
     private static File testDirectory = new File(getBasedir(), 
"target/test-classes/");
 
     public void testBasic() throws Exception {
-        File basicProp = new File(testDirectory, "basic.properties");
+        File basicProp = File.createTempFile("basic", ".properties");
 
-        if (basicProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(basicProp), StandardCharsets.UTF_8)) {
+                writer.write("ghost=${non_existent}\n");
+                writer.write("key=${untat_na_damgo}\n");
+                writer.write("untat_na_damgo=gani_man\n");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false, logger);
+            assertTrue(prop.getProperty("key").equals("gani_man"));
+            assertTrue(prop.getProperty("ghost").equals("${non_existent}"));
+        } finally {
             basicProp.delete();
         }
-
-        basicProp.createNewFile();
-        try (FileWriter writer = new FileWriter(basicProp)) {
-            writer.write("ghost=${non_existent}\n");
-            writer.write("key=${untat_na_damgo}\n");
-            writer.write("untat_na_damgo=gani_man\n");
-            writer.flush();
-        }
-
-        Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false);
-        assertTrue(prop.getProperty("key").equals("gani_man"));
-        assertTrue(prop.getProperty("ghost").equals("${non_existent}"));
     }
 
     public void testSystemProperties() throws Exception {
-        File systemProp = new File(testDirectory, "system.properties");
+        File systemProp = File.createTempFile("system", ".properties");
 
-        if (systemProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(systemProp), StandardCharsets.UTF_8)) {
+                writer.write("key=${user.dir}");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(systemProp, 
false, true, logger);
+            
assertTrue(prop.getProperty("key").equals(System.getProperty("user.dir")));

Review Comment:
   assertEquals



##########
src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java:
##########
@@ -39,40 +38,40 @@ public class PropertyUtilsTest extends TestSupport {
     private static File testDirectory = new File(getBasedir(), 
"target/test-classes/");
 
     public void testBasic() throws Exception {
-        File basicProp = new File(testDirectory, "basic.properties");
+        File basicProp = File.createTempFile("basic", ".properties");
 
-        if (basicProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(basicProp), StandardCharsets.UTF_8)) {
+                writer.write("ghost=${non_existent}\n");
+                writer.write("key=${untat_na_damgo}\n");
+                writer.write("untat_na_damgo=gani_man\n");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false, logger);
+            assertTrue(prop.getProperty("key").equals("gani_man"));

Review Comment:
   assertEquals



##########
src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java:
##########
@@ -39,40 +38,40 @@ public class PropertyUtilsTest extends TestSupport {
     private static File testDirectory = new File(getBasedir(), 
"target/test-classes/");
 
     public void testBasic() throws Exception {
-        File basicProp = new File(testDirectory, "basic.properties");
+        File basicProp = File.createTempFile("basic", ".properties");
 
-        if (basicProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(basicProp), StandardCharsets.UTF_8)) {
+                writer.write("ghost=${non_existent}\n");
+                writer.write("key=${untat_na_damgo}\n");
+                writer.write("untat_na_damgo=gani_man\n");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false, logger);
+            assertTrue(prop.getProperty("key").equals("gani_man"));
+            assertTrue(prop.getProperty("ghost").equals("${non_existent}"));
+        } finally {
             basicProp.delete();
         }
-
-        basicProp.createNewFile();
-        try (FileWriter writer = new FileWriter(basicProp)) {
-            writer.write("ghost=${non_existent}\n");
-            writer.write("key=${untat_na_damgo}\n");
-            writer.write("untat_na_damgo=gani_man\n");
-            writer.flush();
-        }
-
-        Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false);
-        assertTrue(prop.getProperty("key").equals("gani_man"));
-        assertTrue(prop.getProperty("ghost").equals("${non_existent}"));
     }
 
     public void testSystemProperties() throws Exception {
-        File systemProp = new File(testDirectory, "system.properties");
+        File systemProp = File.createTempFile("system", ".properties");
 
-        if (systemProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(systemProp), StandardCharsets.UTF_8)) {
+                writer.write("key=${user.dir}");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(systemProp, 
false, true, logger);

Review Comment:
   properties



##########
src/test/java/org/apache/maven/shared/filtering/PropertyUtilsTest.java:
##########
@@ -39,40 +38,40 @@ public class PropertyUtilsTest extends TestSupport {
     private static File testDirectory = new File(getBasedir(), 
"target/test-classes/");
 
     public void testBasic() throws Exception {
-        File basicProp = new File(testDirectory, "basic.properties");
+        File basicProp = File.createTempFile("basic", ".properties");
 
-        if (basicProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(basicProp), StandardCharsets.UTF_8)) {
+                writer.write("ghost=${non_existent}\n");
+                writer.write("key=${untat_na_damgo}\n");
+                writer.write("untat_na_damgo=gani_man\n");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false, logger);
+            assertTrue(prop.getProperty("key").equals("gani_man"));
+            assertTrue(prop.getProperty("ghost").equals("${non_existent}"));
+        } finally {
             basicProp.delete();
         }
-
-        basicProp.createNewFile();
-        try (FileWriter writer = new FileWriter(basicProp)) {
-            writer.write("ghost=${non_existent}\n");
-            writer.write("key=${untat_na_damgo}\n");
-            writer.write("untat_na_damgo=gani_man\n");
-            writer.flush();
-        }
-
-        Properties prop = PropertyUtils.loadPropertyFile(basicProp, false, 
false);
-        assertTrue(prop.getProperty("key").equals("gani_man"));
-        assertTrue(prop.getProperty("ghost").equals("${non_existent}"));
     }
 
     public void testSystemProperties() throws Exception {
-        File systemProp = new File(testDirectory, "system.properties");
+        File systemProp = File.createTempFile("system", ".properties");
 
-        if (systemProp.exists()) {
+        try {
+            try (Writer writer = new OutputStreamWriter(new 
FileOutputStream(systemProp), StandardCharsets.UTF_8)) {
+                writer.write("key=${user.dir}");
+                writer.flush();
+            }
+
+            Logger logger = mock(Logger.class);
+            Properties prop = PropertyUtils.loadPropertyFile(systemProp, 
false, true, logger);
+            
assertTrue(prop.getProperty("key").equals(System.getProperty("user.dir")));
+        } finally {

Review Comment:
   perhaps just set deleteOnExit instead to avoid IOExceptions here failing the 
test





> PropertyUtils cycle detection results in false positives
> --------------------------------------------------------
>
>                 Key: MSHARED-1290
>                 URL: https://issues.apache.org/jira/browse/MSHARED-1290
>             Project: Maven Shared Components
>          Issue Type: Bug
>          Components: maven-filtering
>    Affects Versions: maven-filtering-3.2.0, maven-filtering-3.3.0, 
> maven-filtering-3.3.1
>            Reporter: Wouter Born
>            Priority: Major
>
> I upgraded the maven-assembly-plugin in one of my projects from 2.6 to 3.6.0 
> and found that {{PropertyUtils}} was logging many false positives about 
> cycles between properties.
> The cycle detection (introduced for MSHARED-417) reports false positives when 
> a variable occurs multiple times in a property, example:
> {code:java}
> depends=p1 >= ${version}, p2 >= ${version}, p3 >= ${version}
> version=1.2.3 {code}
> While writing a unit test to fix this, I also found that there are also cycle 
> false positives whenever one of the property values is the same as a property 
> name, example:
> {code:java}
> test1=${test2}
> test2=${test3}
> test3=test2{code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to