groovy

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: 1.8.x, 2.x
  • Component/s: None
  • 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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
blackdrag blackdrag added a comment -

changed fix version

Show
blackdrag blackdrag added a comment - changed fix version

People

  • Assignee:
    Unassigned
    Reporter:
    Evgeny Goldin
Vote (1)
Watch (1)

Dates

  • Created:
    Updated: