Github user stain commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-language/pull/38#discussion_r196753530
--- Diff:
taverna-scufl2-cwl/src/test/java/org/apache/taverna/scufl2/cwl/TestParser.java
---
@@ -0,0 +1,96 @@
+package org.apache.taverna.scufl2.cwl;
+
+
+import java.util.Set;
+import java.util.HashSet;
+
+import org.junit.Before;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+
+import org.yaml.snakeyaml.Yaml;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+
+import org.apache.taverna.scufl2.api.core.Workflow;
+import org.apache.taverna.scufl2.api.core.Processor;
+import org.apache.taverna.scufl2.api.core.DataLink;
+
+import org.apache.taverna.scufl2.api.common.NamedSet;
+
+import org.apache.taverna.scufl2.api.port.InputWorkflowPort;
+import org.apache.taverna.scufl2.api.port.OutputWorkflowPort;
+import org.apache.taverna.scufl2.api.port.InputProcessorPort;
+
+
+public class TestParser {
+ private static final String HELLO_WORLD_CWL = "/hello_world.cwl";
+
+ private static JsonNode cwlFile;
+ private WorkflowParser parser;
+ private Workflow workflow;
+
+ @Before
+ public void initialize() {
+
+ Yaml reader = new Yaml();
+ ObjectMapper mapper = new ObjectMapper();
+ cwlFile =
mapper.valueToTree(reader.load(TestParser.class.getResourceAsStream(HELLO_WORLD_CWL)));
+ System.out.println(cwlFile);
+ this.parser = new WorkflowParser(cwlFile);
+
+ this.workflow = parser.buildWorkflow();
+ }
+
+ @Test
+ public void testParseInputs() throws Exception {
+
+ NamedSet<InputWorkflowPort> workflowInputs =
workflow.getInputPorts();
+ NamedSet<InputWorkflowPort> expectedInputs = new NamedSet<>();
+ expectedInputs.add(new InputWorkflowPort(workflow, "name"));
--- End diff --
This would use the same `workflow` instance from the parser, As the
[`InputWorkflowPort`
constructor](https://github.com/apache/incubator-taverna-language/blob/master/taverna-scufl2-api/src/main/java/org/apache/taverna/scufl2/api/port/InputWorkflowPort.java#L48)
with a parent will effectively be calling
[inputPort.setParent(workflow)](https://github.com/apache/incubator-taverna-language/blob/master/taverna-scufl2-api/src/main/java/org/apache/taverna/scufl2/api/port/InputWorkflowPort.java#L73)
which would insert the expected port into `parent.getInputPorts()`.
Now in this test the expected parent is the same as the parsed parent, so
the expected port is
[added](https://github.com/apache/incubator-taverna-language/blob/master/taverna-scufl2-api/src/main/java/org/apache/taverna/scufl2/api/common/NamedSet.java#L73)
to the already populated `NamedSet`, replacing the parsed `InputWorkflowPort`
(as if the parsing worked, it has the same name). Thus these collections will
generally match because it's the very same JVM instance - we have lost the
parsed port which may otherwise be wrong.
Rather you need to make the equivalent tree of expected objects, and make
both orphaned. This is because all WorkflowBeans have an
[`.equals`](https://github.com/apache/incubator-taverna-language/blob/master/taverna-scufl2-api/src/main/java/org/apache/taverna/scufl2/api/common/AbstractNamed.java#L103)
that simply checks `.getName()`, and optionally `.getParent()` and
`.getType()`.
So we can short-circuit this ancestor comparisons by setting parent to null
at some point in parsed and expected - that messing about is OK for unit tests
as `@Before` is started fresh for each `@Test`.
Something like
```java
workflow.setParent(null) // Orphaned
expectedWorkflow = new Workflow(workflow.getName()); // same name
expectedInputs = expectedWorkflow.getInputPorts();
expectedInputs.add(new InputWorkflowPort(expectedWorkflow, "name"));
```
I understand if you have concerns about this weird parenting equality check
and those constructors, we can raise that on dev@taverna as a separate
discussion thread to see if we can change it to something more intuitive.
---
