[ 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)