You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/09/01 09:41:47 UTC
[GitHub] [nifi] exceptionfactory commented on a change in pull request #5360: NIFI-9146 to NIFI-9148 - 4 bundles refactored for JUnit 5
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org