markap14 commented on code in PR #5896:
URL: https://github.com/apache/nifi/pull/5896#discussion_r842081291
##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/test/java/org/apache/nifi/xml/TestXMLReader.java:
##########
@@ -31,80 +33,99 @@
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import static junit.framework.TestCase.assertEquals;
public class TestXMLReader {
- private XMLReader reader;
-
private final String ATTRIBUTE_PREFIX = "attribute_prefix";
private final String CONTENT_NAME = "content_field";
private final String EVALUATE_IS_ARRAY = "xml.stream.is.array";
- public TestRunner setup(String filePath) throws InitializationException,
IOException {
-
+ private TestRunner setup(Map<PropertyDescriptor, String>
xmlReaderProperties) throws InitializationException {
TestRunner runner =
TestRunners.newTestRunner(TestXMLReaderProcessor.class);
- reader = new XMLReader();
+ XMLReader reader = new XMLReader();
+
runner.addControllerService("xml_reader", reader);
runner.setProperty(TestXMLReaderProcessor.XML_READER, "xml_reader");
- final String outputSchemaText = new
String(Files.readAllBytes(Paths.get(filePath)));
- runner.setProperty(reader, SchemaAccessUtils.SCHEMA_ACCESS_STRATEGY,
SchemaAccessUtils.SCHEMA_TEXT_PROPERTY);
- runner.setProperty(reader, SchemaAccessUtils.SCHEMA_TEXT,
outputSchemaText);
+ for (Map.Entry<PropertyDescriptor, String> entry :
xmlReaderProperties.entrySet()) {
+ runner.setProperty(reader, entry.getKey(), entry.getValue());
+ }
+ runner.enableControllerService(reader);
return runner;
}
@Test
- public void testRecordFormat() throws IOException, InitializationException
{
- TestRunner runner = setup("src/test/resources/xml/testschema");
-
- runner.setProperty(reader, XMLReader.RECORD_FORMAT,
XMLReader.RECORD_EVALUATE);
-
- runner.enableControllerService(reader);
-
+ public void testRecordFormatDeterminedBasedOnAttribute() throws
IOException, InitializationException {
+ // GIVEN
Review Comment:
I don't think there are necessarily any concerns within the community around
the 'structure' here - performing a setup, then invoking some method, and then
making assertions about the result is a pretty common, straight-forward
approach. The concern has been more about the arbitrary code comments.
Code comments should generally do one of 3 things:
- Explain what the code is doing
- Explain why it's doing it
- Provide notes to help with maintenance (// TODO: Maybe we should consider
this or that or the other...)
The comment "GIVEN" does none of these. Nor does the comment "WHEN" or the
comment "THEN".
The pattern itself is common and doesn't really need code comments to break
out the separate sections. Adding them can even lead to confusion if code is
later updated.
However, if the comments are left in, they should explicitly explain what
the code is doing:
"Create Record Reader", "Run Processor against File ABC", "Validate output",
etc.
--
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
