[ 
https://issues.apache.org/jira/browse/TINKERPOP-848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17427120#comment-17427120
 ] 

ASF GitHub Bot commented on TINKERPOP-848:
------------------------------------------

spmallette commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r726105358



##########
File path: 
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReaderTest.java
##########
@@ -0,0 +1,62 @@
+package org.apache.tinkerpop.gremlin.structure.io.graphml;
+
+import junit.framework.TestCase;
+import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import static org.apache.tinkerpop.gremlin.structure.T.label;
+import static org.apache.tinkerpop.gremlin.structure.T.id;
+
+
+import java.io.IOException;
+import java.io.InputStream;
+
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.atLeastOnce;
+
+public class GraphMLReaderTest extends TestCase {

Review comment:
       Good use of mocking here. Two things:
   
   1. I imagine the rat plugin is failing for this build because you don't have 
the Apache license header in this new file.
   2. Please prefer JUnit annotations over the use of `TestCase` - You may 
refer to any of the other unit tests in the code for examples of that style.




-- 
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: commits-unsubscr...@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Support default attribute values in GraphMLReader
> -------------------------------------------------
>
>                 Key: TINKERPOP-848
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-848
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: io
>    Affects Versions: 3.0.2-incubating
>            Reporter: Pavel Klinov
>            Priority: Trivial
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> Looking at the code of GraphMLReader I see that it doesn't support default 
> values of attributes, which are allowed by the GraphML spec. This is a bit 
> annoying especially if the input defines default values for attributes which 
> are used for mandatory data, e.g. edge labels. 
> One small example is the sample graph at [1]. "d_e" is the label attribute 
> with a default value. There're <edge .. /> elements w/o body later in the 
> document and reading those will throw a "java.lang.IllegalArgumentException: 
> Label can not be null" exception (if the vendor considers edge labels 
> mandatory).
> I'd personaly squash both keyIdMap and keyTypesMap into a single String -> 
> AttrInfo map, where AttrInfo would contain information about the data 
> attribute name, type, and the default value.
> [1] http://www.eecs.wsu.edu/~yyao/DirectedStudyI/Datasets/AS/sample.graphml



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to