roadSurfer commented on code in PR #297:
URL: https://github.com/apache/maven-enforcer/pull/297#discussion_r1915149223
##########
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:
This is a quick negative check to ensure the file is detected and the test
does not pass by fluke. It is consistent with checks in the other tests (throws
an exception with a message).
And I am sorry, but I do no understand how `assumeFalse` is helpful here. If
the file is not detected as being present and an exception thrown with a
message, then that should be a hard failure as the rule is not working
according to spec.
I can see a mix of checkin/not-checking for an unexpected exception, I can
add and explicit check for one and add a `fail` if you would like? I don't
think it would be appropriate to update every test though, just one directly
required by this fix.
--
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
