tinkerpop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmallette <...@git.apache.org>
Subject [GitHub] tinkerpop pull request #934: Apply StringEscapeUtils in GroovyTranslator#con...
Date Fri, 21 Sep 2018 19:42:57 GMT
Github user spmallette commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/934#discussion_r219607573
  
    --- Diff: gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
---
    @@ -141,4 +144,52 @@ public void shouldHandleEmptyMaps() {
         public void shouldHaveValidToString() {
             assertEquals("translator[h:gremlin-groovy]", GroovyTranslator.of("h").toString());
         }
    +
    +    @Test
    +    @LoadGraphWith(LoadGraphWith.GraphData.MODERN)
    +    public void shouldHandleTraversalAddV() {
    --- End diff --
    
    the names of our tests are meant to convey information about what is being tested (when
possible). to that end, you're not really testing "addV()" here but you're testing "string
escaping", so something more like `shouldEscapeStrings()` would probably be a better name.
I would probably combine this test with `shouldHandleVertex()` as they are both validating
the "escaping" but if you have a better name for that test as it test something more specific
than that then feel free to just rename the other test and keep them separate.


---

Mime
View raw message