nandorsoma commented on code in PR #5905:
URL: https://github.com/apache/nifi/pull/5905#discussion_r844033749
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/cloudwatch/PutCloudWatchMetric.java:
##########
@@ -70,6 +70,33 @@
public static final Set<Relationship> relationships =
Collections.unmodifiableSet(
new HashSet<>(Arrays.asList(REL_SUCCESS, REL_FAILURE)));
+ private static final Set<String> units = Collections.unmodifiableSet(
+ new HashSet<>(Arrays.asList(
+ "Seconds", "Microseconds", "Milliseconds", "Bytes",
+ "Kilobytes", "Megabytes", "Gigabytes", "Terabytes",
+ "Bits", "Kilobits", "Megabits", "Gigabits", "Terabits",
+ "Percent", "Count", "Bytes/Second", "Kilobytes/Second",
+ "Megabytes/Second", "Gigabytes/Second", "Terabytes/Second",
+ "Bits/Second", "Kilobits/Second", "Megabits/Second",
+ "Gigabits/Second", "Terabits/Second", "Count/Second",
+ "None", "")));
Review Comment:
Are we sure that empty string is a valid unit?
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/cloudwatch/TestPutCloudWatchMetric.java:
##########
@@ -16,20 +16,21 @@
*/
package org.apache.nifi.processors.aws.cloudwatch;
-import com.amazonaws.services.cloudwatch.model.Dimension;
-import com.amazonaws.services.cloudwatch.model.InvalidParameterValueException;
-import com.amazonaws.services.cloudwatch.model.MetricDatum;
-import org.apache.nifi.components.PropertyDescriptor;
-import org.apache.nifi.util.TestRunner;
-import org.apache.nifi.util.TestRunners;
-import org.junit.jupiter.api.Test;
-
import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.HashMap;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+
+import com.amazonaws.services.cloudwatch.model.Dimension;
+import com.amazonaws.services.cloudwatch.model.MetricDatum;
+import org.junit.Test;
Review Comment:
Why did you change the existing tests to use Junit4?
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/cloudwatch/PutCloudWatchMetric.java:
##########
@@ -70,6 +70,33 @@
public static final Set<Relationship> relationships =
Collections.unmodifiableSet(
new HashSet<>(Arrays.asList(REL_SUCCESS, REL_FAILURE)));
+ private static final Set<String> units = Collections.unmodifiableSet(
Review Comment:
Minor, but if you want, you can use ImmutableSet.of(...) from guava
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/cloudwatch/ITPutCloudWatchMetric.java:
##########
@@ -16,25 +16,32 @@
*/
package org.apache.nifi.processors.aws.cloudwatch;
+import java.io.File;
+import java.io.IOException;
+
import org.apache.nifi.processors.aws.AbstractAWSCredentialsProviderProcessor;
import
org.apache.nifi.processors.aws.credentials.provider.service.AWSCredentialsProviderControllerService;
import org.apache.nifi.processors.aws.sns.PutSNS;
import org.apache.nifi.util.TestRunner;
import org.apache.nifi.util.TestRunners;
import org.junit.jupiter.api.Test;
-import java.io.IOException;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assume.assumeThat;
/**
- * Provides integration level testing with actual AWS CloudWatch resources for
{@link PutCloudWatchMetric} and requires additional configuration and resources
to work.
+ * Provides integration level testing with actual AWS CloudWatch resources for
+ * {@link PutCloudWatchMetric} and requires additional configuration and
resources to work.
*/
public class ITPutCloudWatchMetric {
private final String CREDENTIALS_FILE = System.getProperty("user.home") +
"/aws-credentials.properties";
@Test
- public void testPublish() throws IOException {
+ public void ifCredentialsThenTestPublish() throws IOException {
final TestRunner runner = TestRunners.newTestRunner(new
PutCloudWatchMetric());
+ File credsFile = new File(CREDENTIALS_FILE);
Review Comment:
Thanks for adding this assertion. Makes the test more robust.
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/cloudwatch/TestPutCloudWatchMetric.java:
##########
@@ -264,27 +265,34 @@ public void testMetricExpressionInvalidRoutesToFailure()
throws Exception {
runner.enqueue(new byte[] {}, attributes);
runner.run();
- assertEquals(0, mockPutCloudWatchMetric.putMetricDataCallCount);
+ Assert.assertEquals(0, mockPutCloudWatchMetric.putMetricDataCallCount);
runner.assertAllFlowFilesTransferred(PutCloudWatchMetric.REL_FAILURE,
1);
}
- @Test
- public void testInvalidUnitRoutesToFailure() throws Exception {
+ @ParameterizedTest
+ @CsvSource({"nan","percent","count"})
+ public void testInvalidUnit(String unit) throws Exception {
MockPutCloudWatchMetric mockPutCloudWatchMetric = new
MockPutCloudWatchMetric();
- mockPutCloudWatchMetric.throwException = new
InvalidParameterValueException("Unit error message");
final TestRunner runner =
TestRunners.newTestRunner(mockPutCloudWatchMetric);
runner.setProperty(PutCloudWatchMetric.NAMESPACE, "TestNamespace");
runner.setProperty(PutCloudWatchMetric.METRIC_NAME, "TestMetric");
- runner.setProperty(PutCloudWatchMetric.UNIT, "BogusUnit");
- runner.setProperty(PutCloudWatchMetric.VALUE, "1");
- runner.assertValid();
+ runner.setProperty(PutCloudWatchMetric.UNIT, unit);
+ runner.setProperty(PutCloudWatchMetric.VALUE, "1.0");
+ runner.assertNotValid();
+ }
- runner.enqueue(new byte[] {});
- runner.run();
+ @ParameterizedTest
+ @CsvSource({"Count","Bytes","Percent"})
Review Comment:
Minor, but here I'd reference the collection created in the processor. This
way these values would be tied to the valid ones and we could have the benefit
that this test could run for all valid units.
--
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
