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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
