exceptionfactory commented on a change in pull request #5360:
URL: https://github.com/apache/nifi/pull/5360#discussion_r699619176
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestAlertHandler.java
##########
@@ -97,9 +93,9 @@ public void testAlertNoReportingContext() {
action.setAttributes(attributes);
try {
alertHandler.execute(action, metrics);
- fail();
+ Assertions.fail();
} catch (UnsupportedOperationException ex) {
- assertTrue(true);
+ Assertions.assertTrue(true);
}
Review comment:
It looks like this should be streamlined using `assertThrows`.
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestExpressionHandler.java
##########
@@ -22,25 +22,23 @@
import org.apache.nifi.rules.Action;
import org.apache.nifi.util.TestRunner;
import org.apache.nifi.util.TestRunners;
-import org.junit.Before;
-import org.junit.Test;
+import org.hamcrest.MatcherAssert;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import java.util.HashMap;
import java.util.Map;
-import static junit.framework.TestCase.assertEquals;
-import static junit.framework.TestCase.assertTrue;
import static org.hamcrest.core.IsInstanceOf.instanceOf;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
Review comment:
Recommend retaining import static and replacing references using the new
Assertions methods.
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestLogHandler.java
##########
@@ -229,12 +226,12 @@ public void testInvalidActionTypeDebug() {
try {
logHandler.execute(action, metrics);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend assertThrows
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestAlertHandler.java
##########
@@ -129,11 +125,11 @@ public void testAlertWithBulletinLevel() {
alertHandler.execute(reportingContext, action, metrics);
BulletinRepository bulletinRepository =
reportingContext.getBulletinRepository();
List<Bulletin> bulletins =
bulletinRepository.findBulletinsForController();
- assertFalse(bulletins.isEmpty());
+ Assertions.assertFalse(bulletins.isEmpty());
Bulletin bulletin = bulletins.get(0);
- assertEquals(bulletin.getCategory(), category);
- assertEquals(bulletin.getMessage(), expectedOutput);
- assertEquals(bulletin.getLevel(), severity);
+ Assertions.assertEquals(bulletin.getCategory(), category);
+ Assertions.assertEquals(bulletin.getMessage(), expectedOutput);
+ Assertions.assertEquals(bulletin.getLevel(), severity);
Review comment:
Is there a reason for switching from `import static` to the explicit
`Assertions` reference? Replacing the import static references would reduce the
number of changes significantly.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
##########
@@ -48,10 +49,10 @@ public void setup() throws Exception {
* and stores the value in an attribute of the outgoing flowfile.
* Confirms that the scripted processor transfers the incoming flowfile
with an attribute added.
*
- * @throws Exception Any error encountered while testing
+ * @Any error encountered while testing
*/
- @Test
- public void testReadFlowFileContentAndStoreInFlowFileAttribute() throws
Exception {
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandlerTest.java
##########
@@ -97,7 +97,7 @@ public void
testActionHandlerNotPropertyContextActionHandler() throws Initializa
assertEquals(42, facts.get("testFact"));
}
- @Test
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestAlertHandler.java
##########
@@ -278,13 +274,13 @@ public void testInvalidActionTypeWarn(){
action.setAttributes(attributes);
try {
alertHandler.execute(reportingContext,action, metrics);
- assertTrue(true);
+ Assertions.assertTrue(true);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestAlertHandler.java
##########
@@ -342,9 +338,9 @@ public void testValidActionType(){
action.setAttributes(attributes);
try {
alertHandler.execute(reportingContext,action, metrics);
- assertTrue(true);
+ Assertions.assertTrue(true);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-registry-bundle/nifi-registry-service/src/test/java/org/apache/nifi/schemaregistry/services/TestAvroSchemaRegistry.java
##########
@@ -62,10 +60,10 @@ public void
validateSchemaRegistrationFromrDynamicProperties() throws Exception
SchemaIdentifier schemaIdentifier =
SchemaIdentifier.builder().name(schemaName).build();
RecordSchema locatedSchema = delegate.retrieveSchema(schemaIdentifier);
- assertEquals(fooSchemaText, locatedSchema.getSchemaText().get());
+ Assertions.assertEquals(fooSchemaText,
locatedSchema.getSchemaText().get());
try {
delegate.retrieveSchema(SchemaIdentifier.builder().name("barSchema").build());
- Assert.fail("Expected a SchemaNotFoundException to be thrown but
it was not");
+ Assertions.fail("Expected a SchemaNotFoundException to be thrown
but it was not");
} catch (final SchemaNotFoundException expected) {
Review comment:
It looks like this should be changed to use
`assertThrows(SchemaNotFoundException.class)`
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestExpressionHandler.java
##########
@@ -176,12 +174,12 @@ public void testInvalidActionTypeWarning() {
action.setAttributes(attributes); try {
expressionHandler.execute(action, metrics);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestAlertHandler.java
##########
@@ -311,13 +307,13 @@ public void testInvalidActionTypeIgnore(){
action.setAttributes(attributes);
try {
alertHandler.execute(reportingContext,action, metrics);
- assertTrue(true);
+ Assertions.assertTrue(true);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestExpressionHandler.java
##########
@@ -202,12 +200,12 @@ public void testInvalidActionTypeIgnore() {
action.setAttributes(attributes); try {
expressionHandler.execute(action, metrics);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestExpressionHandler.java
##########
@@ -153,9 +151,9 @@ public void testInvalidActionTypeException() {
action.setType("FAKE");
action.setAttributes(attributes); try {
expressionHandler.execute(action, metrics);
- fail();
+ Assertions.fail();
} catch (UnsupportedOperationException ex) {
- assertTrue(true);
+ Assertions.assertTrue(true);
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestAlertHandler.java
##########
@@ -249,7 +245,7 @@ public void testInvalidActionTypeException(){
action.setAttributes(attributes);
try {
alertHandler.execute(reportingContext, action, metrics);
- fail();
+ Assertions.fail();
} catch (UnsupportedOperationException ex) {
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestLogHandler.java
##########
@@ -161,9 +158,9 @@ public void testInvalidActionTypeException() {
action.setAttributes(attributes);
try {
logHandler.execute(action, metrics);
- fail();
+ Assertions.fail();
} catch (UnsupportedOperationException ex) {
- assertTrue(true);
+ Assertions.assertTrue(true);
Review comment:
Recommend assertThrows
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestLogHandler.java
##########
@@ -194,12 +191,12 @@ public void testInvalidActionTypeWarning() {
try {
logHandler.execute(action, metrics);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend assertThrows
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestLogHandler.java
##########
@@ -16,32 +16,29 @@
*/
package org.apache.nifi.rules.handlers;
-import junit.framework.TestCase;
import org.apache.commons.lang3.StringUtils;
import org.apache.nifi.logging.ComponentLog;
import org.apache.nifi.reporting.InitializationException;
import org.apache.nifi.rules.Action;
import org.apache.nifi.util.TestRunner;
import org.apache.nifi.util.TestRunners;
-import org.junit.Before;
-import org.junit.Test;
+import org.hamcrest.MatcherAssert;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
Review comment:
Recommend import static methods.
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestExpressionHandler.java
##########
@@ -226,9 +224,9 @@ public void testValidActionType() {
action.setAttributes(attributes);
try {
expressionHandler.execute(action, metrics);
- assertTrue(true);
+ Assertions.assertTrue(true);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend `assertThrows`
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/processors/script/ExecuteScriptGroovyTest.groovy
##########
@@ -150,7 +142,10 @@ class ExecuteScriptGroovyTest extends BaseScriptTest {
}
}
- @Ignore("This test fails intermittently when the serial execution happens
faster than pooled")
+ @EnabledIfSystemProperty(
+ named = "nifi.test.unstable",
+ matches = "true",
+ disabledReason = "This test fails intermittently when the serial
execution happens faster than pooled")
Review comment:
Recommend removing this test method. With Git history, if others are
interested in improving it, the previous approach is available. As it stands,
this seems equivalent to commented code, and thus not useful.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestExecuteClojure.java
##########
@@ -65,7 +64,7 @@ public void
testReadFlowFileContentAndStoreInFlowFileAttributeWithScriptFile() t
*
* @throws Exception Any error encountered while testing
*/
- @Test
+ @org.junit.jupiter.api.Test
Review comment:
This should not use the fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestLogHandler.java
##########
@@ -262,9 +259,9 @@ public void testValidActionType() {
action.setAttributes(attributes);
try {
logHandler.execute(action, metrics);
- assertTrue(true);
+ Assertions.assertTrue(true);
} catch (UnsupportedOperationException ex) {
- fail();
+ Assertions.fail();
Review comment:
Recommend assertThrows
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeGroovy.java
##########
@@ -30,33 +30,31 @@
import org.apache.nifi.util.TestRunner;
import org.apache.nifi.util.TestRunners;
import org.apache.nifi.util.security.MessageDigestUtils;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Set;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
public class TestInvokeGroovy extends BaseScriptTest {
-
- @Before
+ @BeforeEach
public void setup() throws Exception {
super.setupInvokeScriptProcessor();
}
/**
* Tests a script that has a Groovy Processor that that reads the first
line of text from the flowfiles content and stores the value in an attribute of
the outgoing flowfile.
*
- * @throws Exception Any error encountered while testing
*/
- @Test
- public void testReadFlowFileContentAndStoreInFlowFileAttribute() throws
Exception {
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation should not be necessary.
##########
File path:
nifi-nar-bundles/nifi-rules-action-handler-bundle/nifi-rules-action-handler-service/src/test/java/org/apache/nifi/rules/handlers/TestRecordSinkHandler.java
##########
@@ -30,8 +30,10 @@
import org.apache.nifi.serialization.record.RecordSet;
import org.apache.nifi.util.TestRunner;
import org.apache.nifi.util.TestRunners;
-import org.junit.Before;
-import org.junit.Test;
+import org.hamcrest.MatcherAssert;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
Review comment:
Recommend import static methods.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
##########
@@ -105,10 +106,10 @@ public void testScriptDefinedAttribute() throws Exception
{
* stores the value in an attribute of the outgoing flowfile.
* Confirms that the scripted processor can return relationships defined
in it.
*
- * @throws Exception Any error encountered while testing
+ * @Any error encountered while testing
*/
- @Test
- public void testScriptDefinedRelationship() throws Exception {
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeGroovy.java
##########
@@ -108,10 +105,9 @@ public void testScriptDefinedAttribute() throws Exception {
* Tests a script that has a Groovy Processor that that reads the first
line of text from the flowfiles content and
* stores the value in an attribute of the outgoing flowfile.
*
- * @throws Exception Any error encountered while testing
*/
- @Test
- public void testScriptDefinedRelationship() throws Exception {
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
##########
@@ -70,10 +71,10 @@ public void
testReadFlowFileContentAndStoreInFlowFileAttribute() throws Exceptio
* stores the value in an attribute of the outgoing flowfile.
* Confirms that the scripted processor can return property descriptors
defined in it.
*
- * @throws Exception Any error encountered while testing
+ * @Any error encountered while testing
*/
- @Test
- public void testScriptDefinedAttribute() throws Exception {
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
##########
@@ -140,28 +141,26 @@ public void testScriptDefinedRelationship() throws
Exception {
* Tests a script that throws a ProcessException within.
* The expected result is that the exception will be propagated.
*
- * @throws Exception Any error encountered while testing
*/
- @Test(expected = AssertionError.class)
- public void testInvokeScriptCausesException() throws Exception {
+ @Test
+ public void testInvokeScriptCausesException() {
final TestRunner runner = TestRunners.newTestRunner(new
InvokeScriptedProcessor());
runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE,
"ECMAScript");
runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY,
getFileContentsAsString(
TEST_RESOURCE_LOCATION +
"javascript/testInvokeScriptCausesException.js")
);
runner.assertValid();
runner.enqueue("test content".getBytes(StandardCharsets.UTF_8));
- runner.run();
-
+ assertThrows(AssertionError.class, () -> runner.run());
}
/**
* Tests a script that routes the FlowFile to failure.
*
- * @throws Exception Any error encountered while testing
+ * @Any error encountered while testing
*/
- @Test
- public void testScriptRoutesToFailure() throws Exception {
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
##########
@@ -178,10 +177,10 @@ public void testScriptRoutesToFailure() throws Exception {
/**
* Tests an empty script with Nashorn (which throws an NPE if it is
loaded), this test verifies an empty script is not attempted to be loaded.
*
- * @throws Exception Any error encountered while testing
+ * @Any error encountered while testing
*/
- @Test
- public void testEmptyScript() throws Exception {
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandlerTest.java
##########
@@ -64,15 +64,15 @@
private Map<String, Object> facts = new HashMap<>();
private Map<String, String> attrs = new HashMap<>();
- @Before
+ @BeforeEach
public void setup() {
facts.put("predictedQueuedCount", 60);
facts.put("predictedTimeToBytesBackpressureMillis", 299999);
attrs.put("level", "DEBUG");
attrs.put("message", "Time to backpressure < 5 mins");
}
- @Test
+ @org.junit.jupiter.api.Test
Review comment:
Fully-qualified class annotation.
--
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...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
