AxelSync commented on a change in pull request #3633: NIFI-6523 MergeRecords 
properties should accept expression language variables
URL: https://github.com/apache/nifi/pull/3633#discussion_r310469753
 
 

 ##########
 File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestMergeRecord.java
 ##########
 @@ -486,104 +749,4 @@ public void testDefragmentExpiredBinFailsOnTimeout() 
throws InterruptedException
         runner.assertTransferCount(MergeRecord.REL_ORIGINAL, 0);
         runner.assertTransferCount(MergeRecord.REL_FAILURE, 1);
     }
-
-    @Test
-    public void testMergeWithMinRecordsFromVariableRegistry() {
-        runner.setVariable("min_records", "3");
-        runner.setVariable("max_records", "3");
-        runner.setValidateExpressionUsage(true);
-
-        // Test MIN_RECORDS
-        runner.setProperty(MergeRecord.MIN_RECORDS, "${min_records}");
-        runner.setProperty(MergeRecord.MAX_RECORDS, "3");
-
-        runner.enqueue("Name, Age\nJohn, 35");
-        runner.enqueue("Name, Age\nJane, 34");
-        runner.enqueue("Name, Age\nAlex, 28");
-
-        runner.run(1);
-        runner.assertTransferCount(MergeRecord.REL_MERGED, 1);
-        runner.assertTransferCount(MergeRecord.REL_ORIGINAL, 3);
-
-        final MockFlowFile mff = 
runner.getFlowFilesForRelationship(MergeRecord.REL_MERGED).get(0);
-        mff.assertAttributeEquals("record.count", "3");
-        mff.assertContentEquals("header\nJohn,35\nJane,34\nAlex,28\n");
-        runner.clearTransferState();
-
-        // Test MAX_RECORDS
-        runner.setProperty(MergeRecord.MIN_RECORDS, "1");
-        runner.setProperty(MergeRecord.MAX_RECORDS, "${max_records}");
-
-        runner.enqueue("Name, Age\nJohn, 35");
-        runner.enqueue("Name, Age\nJane, 34");
-        runner.enqueue("Name, Age\nAlex, 28");
-        runner.enqueue("Name, Age\nDonna, 48");
-        runner.enqueue("Name, Age\nJoey, 45");
-
-        runner.run(2);
-        runner.assertTransferCount(MergeRecord.REL_MERGED, 2);
-        runner.assertTransferCount(MergeRecord.REL_ORIGINAL, 5);
-
-        final MockFlowFile mff1 = 
runner.getFlowFilesForRelationship(MergeRecord.REL_MERGED).get(0);
-        mff1.assertAttributeEquals("record.count", "3");
-        mff1.assertContentEquals("header\nJohn,35\nJane,34\nAlex,28\n");
-
-        final MockFlowFile mff2 = 
runner.getFlowFilesForRelationship(MergeRecord.REL_MERGED).get(1);
-        mff2.assertAttributeEquals("record.count", "2");
-        mff2.assertContentEquals("header\nDonna,48\nJoey,45\n");
-        runner.clearTransferState();
-
-        runner.removeProperty("min_records");
-        runner.removeProperty("max_records");
-    }
-
-    @Test
-    public void testNegativeMinAndMaxRecordsValidators(){
 
 Review comment:
   I found out that there already was a `testValidation()` method which tests 
the other properties together, and it was weird, having to add now validation 
cases, to have them all but the MIN_RECORDS and MAX_RECORDS in the 
`testValidation()` method.
   Moreover, I think it would be nice to reduce the number of test methods now 
that we almost every time reach Travis timeout. 
   I can open a different PR of course, but I think this can still be 
considered in the scope of this one

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


With regards,
Apache Git Services

Reply via email to