[GitHub] [nifi] MikeThomsen commented on a change in pull request #5368: NIFI-9141 Refactored nifi-provenance-repository-bundle using JUnit 5

[GitBox <gi...@apache.org>]

MikeThomsen commented on a change in pull request #5368:
URL: https://github.com/apache/nifi/pull/5368#discussion_r702448118



##########
File path: nifi-nar-bundles/nifi-provenance-repository-bundle/nifi-persistent-provenance-repository/src/test/java/org/apache/nifi/provenance/ITestPersistentProvenanceRepository.java
##########
@@ -1307,13 +1186,12 @@ public boolean isAuthorized(ProvenanceEventRecord event, NiFiUser user) {
         assertEquals(7, events.size());
         final List<Long> eventIds = events.stream().map(event -> event.getEventId()).sorted().collect(Collectors.toList());
         for (int i = 0; i < 7; i++) {
-            Assert.assertEquals(i + 3, eventIds.get(i).intValue());
+            assertEquals(i + 3, eventIds.get(i).intValue());
         }
     }
 
-    @Test(timeout = 10000)
+    @Test

Review comment:
       We should probably put an @Timeout here to keep the behavior consistent.

##########
File path: nifi-nar-bundles/nifi-provenance-repository-bundle/nifi-persistent-provenance-repository/src/test/java/org/apache/nifi/provenance/ITestPersistentProvenanceRepository.java
##########
@@ -504,7 +485,6 @@ public void testIndexOnRolloverWithImmenseAttribute() throws IOException {
 
     @Test
     public void testIndexOnRolloverAndSubsequentSearch() throws IOException, InterruptedException, ParseException {
-        assumeFalse(isWindowsEnvironment());

Review comment:
       Shouldn't these have `@DisabledOnOs(OS.WINDOWS)` on them?

##########
File path: nifi-nar-bundles/nifi-provenance-repository-bundle/nifi-persistent-provenance-repository/src/test/java/org/apache/nifi/provenance/ITestPersistentProvenanceRepository.java
##########
@@ -1250,17 +1135,11 @@ public void authorize(ProvenanceEventRecord event, NiFiUser user) {
 
         repo.waitForRollover();
 
-        try {
-            repo.getEvent(0L, null);
-            Assert.fail("getEvent() did not throw an Exception");
-        } catch (final Exception e) {
-            Assert.assertSame(expectedException, e);
-        }
+        assertThrows(expectedException.getClass(), () -> repo.getEvent(0, null));
     }
 
     @Test
     public void testNotAuthorizedGetEventRange() throws IOException {
-        assumeFalse(isWindowsEnvironment());

Review comment:
       Why no DisabledOnOs?




-- 
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-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org