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 2022/11/25 20:26:20 UTC
[GitHub] [nifi] MikeThomsen commented on a diff in pull request #6720: [NIFI-10876] Made changes to convert from JUnit 4 to JUnit 5
MikeThomsen commented on code in PR #6720:
URL: https://github.com/apache/nifi/pull/6720#discussion_r1032674876
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java:
##########
@@ -249,7 +253,7 @@ public void testUpdateGroupWithSameName() {
try {
Group group1Updated = new Group.Builder().identifier("group-id-1").name("xyz").build();
userGroupProvider.updateGroup(group1Updated);
- Assert.fail("Should have thrown exception");
+ fail("Should have thrown exception");
Review Comment:
Should use `assertThrows`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/parameter/TestStandardParameterContext.java:
##########
@@ -241,7 +242,7 @@ public void testChangingSensitivity() {
try {
context.setParameters(updatedParameters);
- Assert.fail("Succeeded in changing parameter from sensitive to non-sensitive");
+ fail("Succeeded in changing parameter from sensitive to non-sensitive");
Review Comment:
Should use `assertThrows`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/parameter/TestStandardParameterContext.java:
##########
@@ -331,17 +332,17 @@ public void testChangingNestedParameterForRunningProcessor() {
startProcessor(procNode);
try {
a.setInheritedParameterContexts(Collections.emptyList());
- Assert.fail("Was able to remove parameter while referencing processor was running");
Review Comment:
This should be replaced with something like `IllegalStateException ex = assertThrows(...`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java:
##########
@@ -119,7 +123,7 @@ public void testAddPoliciesWithSameResourceAndAction() {
try {
accessPolicyProvider.addAccessPolicy(policy2);
- Assert.fail("Should have thrown exception");
+ fail("Should have thrown exception");
Review Comment:
Should use `assertThrows`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/parameter/TestStandardParameterContext.java:
##########
@@ -231,7 +232,7 @@ public void testChangingSensitivity() {
try {
context.setParameters(updatedParameters);
- Assert.fail("Succeeded in changing parameter from non-sensitive to sensitive");
+ fail("Succeeded in changing parameter from non-sensitive to sensitive");
Review Comment:
Should use `assertThrows`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java:
##########
@@ -224,7 +228,7 @@ public void testUpdateUserWithSameIdentity() {
try {
User user1Updated = new User.Builder().identifier("user-id-1").identity("xyz").build();
userGroupProvider.updateUser(user1Updated);
- Assert.fail("Should have thrown exception");
+ fail("Should have thrown exception");
Review Comment:
Should use `assertThrows`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/parameter/TestStandardParameterContext.java:
##########
@@ -281,7 +282,7 @@ public void testChangingParameterForRunningProcessor() {
parameters.put("abc", new Parameter(abcDescriptor, null));
try {
context.setParameters(parameters);
- Assert.fail("Was able to remove parameter while referencing processor was running");
+ fail("Was able to remove parameter while referencing processor was running");
Review Comment:
Should use `assertThrows`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/util/TestFlowDifferenceFilters.java:
##########
@@ -62,7 +62,7 @@ public void testFilterAddedRemotePortsWithRemoteOutputPort() {
DifferenceType.COMPONENT_ADDED, null, remoteGroupPort, null, null, "");
// predicate should return false because we don't want to include changes for adding a remote input port
- Assert.assertFalse(FlowDifferenceFilters.FILTER_ADDED_REMOVED_REMOTE_PORTS.test(flowDifference));
+ Assertions.assertFalse(FlowDifferenceFilters.FILTER_ADDED_REMOVED_REMOTE_PORTS.test(flowDifference));
Review Comment:
Static imports
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/controller/repository/TestRingBufferEventRepository.java:
##########
@@ -57,21 +57,21 @@ public void testPurge() throws IOException {
repo.updateRepository(generateEvent(), id2);
RepositoryStatusReport report = repo.reportTransferEvents(System.currentTimeMillis());
FlowFileEvent entry = report.getReportEntry(id1);
- Assert.assertNotNull(entry);
+ Assertions.assertNotNull(entry);
Review Comment:
Let's move to static imports here for consistency.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java:
##########
@@ -165,7 +169,7 @@ public void testAddGroupsWithSameName() {
try {
userGroupProvider.addGroup(group2);
- Assert.fail("Should have thrown exception");
+ fail("Should have thrown exception");
Review Comment:
Should use `assertThrows`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizerTest.java:
##########
@@ -552,7 +552,7 @@ public void testWaitForQueueToEmpty() throws InterruptedException {
synchronizer.synchronize(connectionAB, null, group, synchronizationOptions);
completionLatch.countDown();
} catch (final Exception e) {
- Assert.fail(e.toString());
Review Comment:
Should used `assertDoesNotThrow`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java:
##########
@@ -142,7 +146,7 @@ public void testAddUsersWithSameIdentity() {
try {
userGroupProvider.addUser(user2);
- Assert.fail("Should have thrown exception");
+ fail("Should have thrown exception");
Review Comment:
Should use `assertThrows`
--
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