groovy
  1. groovy
  2. GROOVY-4520

Groovy power assert is confusing if no types are displayed

    Details

    • Type: Improvement Improvement
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.7.5
    • Fix Version/s: 2.x
    • Component/s: groovy-runtime
    • Labels:
      None
    • Number of attachments :
      0

      Description

      In the following code

      def j = "5"
      assert 5 == j
      
      Assertion failed: 
      
      assert 5 == j
               |  |
               |  5
               false
      

      it is confusing to understand why assert fails if no types are displayed. Variables compared may be different but their toString() representations used by assert error message may be identical.

      Another example I had was:

      assert pomSize == project.properties.pomSize
             |       |  |       |          |
             2607    |  |       |          2607
                     |  |       [pomSize:2607, gmavenVersion:1.3]
                     |  MavenProject: com.goldin:groovymag.listing-1:12.2010 @ E:\Projects\GroovyMag\December-2010\Listing-1\pom.xml
                     false
      

      Here I had a number 2607 on the left, but a String "2607" on the right.

      I suggest to add types of variables compared in assert.

        Activity

        Hide
        blackdrag blackdrag added a comment -

        Peter, I think you mentioned it somewhere already... why again did you not mark Strings especially again?

        Show
        blackdrag blackdrag added a comment - Peter, I think you mentioned it somewhere already... why again did you not mark Strings especially again?
        Hide
        Peter Niederwieser added a comment -

        Every time I tried to make the output more unambiguous, I ended up with a more complex output that still at times proved ambiguous in practice. For example, if you decide to render strings in a special way, then what about strings contained in a collection? What about strings contained in some custom type? And what about strings containing quotes? In the end I decided to start out simple and see how it goes. If one keeps in mind that power asserts just print a (Groovy) string representation of the values, it's not too difficult to make sense of outputs like the one above (and refine the assertion).

        Printing the values' types is an interesting idea, but would add a lot of noise for unambiguous cases (which are far more frequent). We could try to be clever and only add type information for equality comparisons where the string representations of both sides are identical, but that's beyond the scope of the current power assert implementation.

        The most frequent ambiguity that I've seen in practice is string vs. number (like above). So maybe we should finally try to introduce literal syntax here and there, possibly also for collections (since Groovy already supports it). More opinions are welcome.

        Show
        Peter Niederwieser added a comment - Every time I tried to make the output more unambiguous, I ended up with a more complex output that still at times proved ambiguous in practice. For example, if you decide to render strings in a special way, then what about strings contained in a collection? What about strings contained in some custom type? And what about strings containing quotes? In the end I decided to start out simple and see how it goes. If one keeps in mind that power asserts just print a (Groovy) string representation of the values, it's not too difficult to make sense of outputs like the one above (and refine the assertion). Printing the values' types is an interesting idea, but would add a lot of noise for unambiguous cases (which are far more frequent). We could try to be clever and only add type information for equality comparisons where the string representations of both sides are identical, but that's beyond the scope of the current power assert implementation. The most frequent ambiguity that I've seen in practice is string vs. number (like above). So maybe we should finally try to introduce literal syntax here and there, possibly also for collections (since Groovy already supports it). More opinions are welcome.
        Hide
        Peter Niederwieser added a comment -

        Any opinions on how to proceed with this issue? Would you like to see literal syntax used in power asserts (e.g. "2067" instead of 2067)?

        Show
        Peter Niederwieser added a comment - Any opinions on how to proceed with this issue? Would you like to see literal syntax used in power asserts (e.g. "2067" instead of 2067)?
        Hide
        Evgeny Goldin added a comment -

        I vote for "2067".

        def j = "5"
        assert 5 == j
        
        Assertion failed: 
        
        assert 5 == j
                 |  |
                 |  "5"
                 false
        
        Show
        Evgeny Goldin added a comment - I vote for "2067". def j = "5" assert 5 == j Assertion failed: assert 5 == j | | | "5" false
        Hide
        Hamlet D'Arcy added a comment -

        I would leave it the way it is if the two toString() values are different, and add type information if the two toString()s are the same. The String example is just one case. This happens a lot more when equals() is not implemented.

        Show
        Hamlet D'Arcy added a comment - I would leave it the way it is if the two toString() values are different, and add type information if the two toString()s are the same. The String example is just one case. This happens a lot more when equals() is not implemented.
        Hide
        Peter Niederwieser added a comment - - edited

        > I would leave it the way it is if the two toString() values are different, and add type information if the two toString()s are the same.

        Since we don't have any structural information about the assertion at runtime (e.g. we can't tell it's a == comparison), all we could do is to add type information whenever two of the values have the same toString() representation. This would again mean lots of noise.

        Show
        Peter Niederwieser added a comment - - edited > I would leave it the way it is if the two toString() values are different, and add type information if the two toString()s are the same. Since we don't have any structural information about the assertion at runtime (e.g. we can't tell it's a == comparison), all we could do is to add type information whenever two of the values have the same toString() representation. This would again mean lots of noise.
        Hide
        blackdrag blackdrag added a comment -

        changed fix version

        Show
        blackdrag blackdrag added a comment - changed fix version

          People

          • Assignee:
            Unassigned
            Reporter:
            Evgeny Goldin
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: