elharo commented on code in PR #297:
URL: https://github.com/apache/maven-enforcer/pull/297#discussion_r1937651407
##########
enforcer-rules/src/site/apt/requireFilesDontExist.apt.vm:
##########
@@ -26,7 +26,10 @@
Require Files Don't Exist
This rule checks that the specified list of files do not exist.
Review Comment:
delete "list of"
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -105,19 +109,62 @@ void testEmptyFileListAllowNull() {
}
@Test
- void testFileDoesNotExist() throws EnforcerRuleException, IOException {
+ void testDeletedFileDetected() throws EnforcerRuleException, IOException {
File f = File.createTempFile("junit", null, temporaryFolder);
+ rule.setFilesList(Collections.singletonList(f));
+
+ // Check the file is detected as being present
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assumeFalse(e.getMessage() == null);
+
f.delete();
assertFalse(f.exists());
- rule.setFilesList(Collections.singletonList(f));
+ // Rule should now pass as the file was deleted
+ rule.execute();
+ }
+
+ @Test
+ void testSymbolicLinkDeletedDetected() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
+
+ rule.setFilesList(Collections.singletonList(linkFile));
+ // Check the link is detected as being present
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assumeFalse(e.getMessage() == null);
Review Comment:
assertNotNull
##########
enforcer-rules/src/site/apt/requireFilesExist.apt.vm:
##########
@@ -27,22 +27,25 @@ Require Files Exist
This rule checks that the specified list of files exist.
Review Comment:
ditto
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -105,19 +109,62 @@ void testEmptyFileListAllowNull() {
}
@Test
- void testFileDoesNotExist() throws EnforcerRuleException, IOException {
+ void testDeletedFileDetected() throws EnforcerRuleException, IOException {
File f = File.createTempFile("junit", null, temporaryFolder);
+ rule.setFilesList(Collections.singletonList(f));
+
+ // Check the file is detected as being present
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assumeFalse(e.getMessage() == null);
+
f.delete();
assertFalse(f.exists());
- rule.setFilesList(Collections.singletonList(f));
+ // Rule should now pass as the file was deleted
+ rule.execute();
+ }
+
+ @Test
+ void testSymbolicLinkDeletedDetected() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
+
+ rule.setFilesList(Collections.singletonList(linkFile));
+ // Check the link is detected as being present
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assumeFalse(e.getMessage() == null);
+
+ linkFile.delete();
+
+ // Rule should now pass as the target was deleted
+ rule.execute();
+ }
+
+ @Test
+ void testSymbolicLinkTargetDeletedDetected() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
+
+ // Check the target is detected as being present
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assumeFalse(e.getMessage() == null);
Review Comment:
assertNotNull
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -107,12 +110,55 @@ void testEmptyFileListAllowNull() {
@Test
void testFileDoesNotExist() throws EnforcerRuleException, IOException {
File f = File.createTempFile("junit", null, temporaryFolder);
+ rule.setFilesList(Collections.singletonList(f));
+
+ // Check the file is detected as being present
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assertNotNull(e.getMessage());
+
f.delete();
assertFalse(f.exists());
Review Comment:
The wrong one was changed. Line 122 should be changed or better yet deleted.
The point of the test is not whether f.delete() succeeds. You're not testing
the delete method in java.io.File.
##########
enforcer-rules/src/site/apt/requireFilesDontExist.apt.vm:
##########
@@ -26,7 +26,10 @@
Require Files Don't Exist
This rule checks that the specified list of files do not exist.
-
+
+ The mounted filesystem(s) will dictate the case-sensitivity rules when
checking files. If checks are required that
Review Comment:
delete "mounted" and consider getting rid of the parentheses or just
changing this to "The filesystem" as is every file is on a single filesystem.
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesDontExist.java:
##########
@@ -105,19 +109,62 @@ void testEmptyFileListAllowNull() {
}
@Test
- void testFileDoesNotExist() throws EnforcerRuleException, IOException {
+ void testDeletedFileDetected() throws EnforcerRuleException, IOException {
File f = File.createTempFile("junit", null, temporaryFolder);
+ rule.setFilesList(Collections.singletonList(f));
+
+ // Check the file is detected as being present
+ EnforcerRuleException e = assertThrows(EnforcerRuleException.class,
rule::execute);
+ assumeFalse(e.getMessage() == null);
Review Comment:
This should be an assertNotNull
##########
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/files/TestRequireFilesExist.java:
##########
@@ -50,12 +56,34 @@ void testFileExists() throws Exception {
}
@Test
- void testFileOsIndependentExists() {
- rule.setFilesList(Collections.singletonList(new File("POM.xml")));
+ void testSymbolicLinkExists() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
- EnforcerRuleException e = assertThrows(EnforcerRuleException.class, ()
-> rule.execute());
+ rule.setFilesList(Collections.singletonList(linkFile));
- assertNotNull(e.getMessage());
+ rule.execute();
+ }
+
+ @Test
+ void testSymbolicLinkTargetDoesNotExist() throws Exception {
+ File canonicalFile = File.createTempFile("canonical_", null,
temporaryFolder);
+ File linkFile = Files.createSymbolicLink(
+ Paths.get(temporaryFolder.getAbsolutePath(),
"symbolic.link"),
+ Paths.get(canonicalFile.getAbsolutePath()))
+ .toFile();
+ canonicalFile.delete();
+ rule.setFilesList(Collections.singletonList(linkFile));
+
+ try {
+ rule.execute();
+ fail("Should have received an exception");
+ } catch (Exception e) {
Review Comment:
which exception should be thrown here? Be specific.
##########
enforcer-rules/src/site/apt/requireFilesSize.apt.vm:
##########
@@ -27,26 +27,29 @@ Require File Size
This rule checks that the specified list of files exist and are within the
specified size range.
+ The mounted filesystem(s) will dictate the case-sensitivity rules when
checking files. If you require checks that your
Review Comment:
ditto
--
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: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
