Bytes assertions enhancements - better readability

[mariuszs]

My proposition for assertions arrays:

assertThat("zólc".getBytes()).contains("żółć".getBytes("ISO-8859-2"));

java.lang.AssertionError:
Expecting:
<[7A:C3:B3:6C:63]>
to contain:
<[BF:F3:B3:E6]>
but could not find:
<[BF:F3:E6]>

This is more readable than:

java.lang.AssertionError:
Expecting:
<[0x7A, 0xC3, 0xB3, 0x6C, 0x63]>
to contain:
<[0xBF, 0xF3, 0xB3, 0xE6]>
but could not find:
<[0xBF, 0xF3, 0xE6]>

Simple values comparision without changes:

assertThat((byte) 0xff).isEqualTo((byte) '3');

org.junit.ComparisonFailure:
Expected :0x33
Actual   :0xFF

What do you think?

[JakeWharton]

Awesome stuff. I'm in favor.

[joel-costigliola]

I was a little skeptic because we change a standard representation but since more than one user likes it and we know that we are asserting bytes then I'm ok to integrate it.

I'll review the PR by the end of the year.

Thanks for the idea and the contribution.

[mariuszs]

Im little skeptic too :) We can do this better.

My proposition is to use something like Representation - StandardRepresentation for printing standard values like 256, HexadecimalRepresentation for values like 0xFF or FF and BinaryRepresentations for values like 0b11111111. Example api usage:

Standard representation:-

assertThat(new byte[]{1, 2, 3}).contains("sss".getBytes());

Result:

java.lang.AssertionError:
Expecting:
 <[122, -61, -77, 108, 99]>
to contain:
 <[-65, -13, -77, -26]>
but could not find:
 <[-65:-13:-26]>

Hexadecimal representation:

 assertThat("zólc".getBytes()).asHex().contains("żółć".getBytes("ISO-8859-2"));

Result:

 java.lang.AssertionError:
 Expecting:
  <[7A:C3:B3:6C:63]>
 to contain:
  <[BF:F3:B3:E6]>
 but could not find:
  <[BF:F3:E6]>

Binary representation:

  assertThat("zólc".getBytes()).asBinary().contains("żółć".getBytes("ISO-8859-2"));

Result:

  java.lang.AssertionError:
  Expecting:
   <[0b1111010, 0b11111111111111111111111111000011, 0b11111111111111111111111110110011, 0b1101100, 0b1100011]>
  to contain:
   <[0b11111111111111111111111110111111, 0b11111111111111111111111111110011, 0b11111111111111111111111110110011, 0b11111111111111111111111111100110]>
  but could not find:
   <[0b11111111111111111111111110111111:0b11111111111111111111111111110011:0b11111111111111111111111111100110]>

This apply to all form of assertions, if it makes sense.

[mariuszs]

This also is more clear, we can add special information like Expecting in hexadecimal representation:.

assertThat(new byte[]{0x10,0x20}).asHex().contains(new byte[]{0x30});

java.lang.AssertionError: 
Expecting in hexadecimal representation:
 <[10:20]>
to contain:
 <[30]>
but could not find:
 <[30]>

Other examples

assertThat((short) 2).asBinary().isEqualTo(3);

 org.junit.ComparisonFailure: 
 Expected :0b100000000000000000000000000000011
 Actual   :0b10000000000000010

[joel-costigliola]

I think we should concentrate on bytes enhancements first and create another issue with the more global "presentation" stuff. I need to take more time to think about the latter, I like the idea but I'm worried it may be too much, I don't see a need for that but it does not mean there is not, it would be good to have some real life examples.

Things are getting a little messy on git, I have fetched your assertj fork repository and see two similar commits for "compact message for better readability": mariuszs@8abb54d on topic/presentation branch and mariuszs@a782c71 on topic/compact_hex_view. I see also two PR, one coming from topic/presentation and another from topic/compact_hex_view, the first including the latter.

The topic/compact_hex_view PR is good, I have two comments on it:

• why use : to separate list elements instead of , ? I would prefer to keep , as it is widely used
• it would be nice to start the error description with "Expecting in compact hexadecimal representation: " as you suggested adding only "compact".

[mariuszs]

: is used because I prefer hexadecimal colon notation. It is more compact and readable for bytes. I think this was very popular representation for hexadecimal values, for example ipv6 is using hex colon notation.

topic/presentation is based on topic/compact_hex_view and was updated with your master. PR from topic/compact_hex_view differs because needs merging.

I think "presentation stuff" is helpful and fix some issues. For example natural for byte is format like 0xFC, but as java developer you can code like this byte i = -2 and use this as signed number. After this assertion message is misleading for developer. But with presentation you can select proper presentation. This is very similar to assertThat(x).as("winner").hasSize(2);. So if your byte is used for signed number use assertThat(i).isEqualTo(-1);, but if you are using byte as 0xFE, then use assertThat(i).asHex().isEqualTo(-1);.This is helpful with TDD, because the error message is nice and adequate to code. We think what we need to implement, so we knew which "presentations stuff" to use.

I can prepare more useful scenerios.

Btw, presentation stuff PR is not complete yet. it needs more love, refactor, name change and javadocs :) but in general works :)

Other interesting example is comparing UNICODE characters - many unicode chars have duplicate characters assigned, so this is impossible to find differences from standard message:

assertThat("µµµ").contains("μμμ");

Standard message:

java.lang.AssertionError: 
Expecting:
 <"µµµ">
to contain:
 <"μμμ"> 

Hexadecimal message with assertThat("µµµ").asHex().contains("μμμ");:

java.lang.AssertionError:
Expecting:
 <"['00B5', '00B5', '00B5']">
to contain:
 <"['03BC', '03BC', '03BC']">

it would be nice to start the error description with "Expecting in compact hexadecimal representation: " as you suggested adding only "compact".

I think this is useful only in "presentation stuff"

[mariuszs]

Messy on git fixed :)

[joel-costigliola]

Ok I'm convinced this is useful :)

Tell me when you think the PR can be reviewed.

[mariuszs]

I think this is ready for review. General change in this PR is to replace static invocation to ToString.toStringOf with invocation to generic org.assertj.core.presentation.Presentation instance. Im not sure if Presentation is best name for this, maybe Representation or Renderer? This class provides toString for single object, toString for object in array/collections and method for separator.

In AbstractDateAssert is fixme added to verify and correct.

In Tuple also is Fixme, please look at this. My solution is to separate this code from Presentation and add Tuple as supported type to Presentation toString.

There is many JavaDocs missing or incomplete, I need help with this.

I dont have time also to make bigger refatoring, please look if refactoring is needed now.

Also Presentation implementations (HexadecimalPresentation and BinaryPresentation) can be improved and have support for more cases, but I think this can be made later. StandardPresentation presentation is complete.

[joel-costigliola]

Yes, I think we can find a better naming for Presentation concept, Representation is not bad, Renderer is too GUI oriented to me, anyway I will also take some time to think to this.

For the javadoc, just focus on the API part, the one that is used by users, the remaining is less important.

Thanks for the work !

[mariuszs]

I think its ready, of course there is place for improvments... Please make review.

Colon notation in hex is removed, we can add this later.

[joel-costigliola]

I haven't done a full review yet this will take some time but I have got already some comments:

• I like Representation better than Presentation :)
• Let's rename asHex() to asHexadecimal() to be consistent with asBinary() which is not shortened.
• Javadoc API
  Change Enable binary object representation instead of standard java representation. to Enable binary object representation instead of standard java representation in error messages. otherwise it's not crystal clear that it is related to error message.

my most important comment last !

asBinary() and asHexadecimal() should only be available when relevant, that is in: numbers, char and String or array/Iterable of those (tell me if I have forgotten some types).
It is very important not to pollute the API with unrelevant proposals like in the the example below where there is no point in using binary representation for dates:

assertThat(theTwoTowers.getReleaseDate()).asBinary().isInThePast();

[mariuszs]

In general, you most important comment is right but hard to implement properly. If we enable this api for collections and arrays, then this involve all object types. So I will prefer to make this for general use and eventually add more implementations types (ex. for BigInteger, other object via ObjectOutputStream etc) or simply print not implemented representations or something like that.

[mariuszs]

Eventually we can add more detailed information in JavaDoc, somehting like: Enable if possible binary object representation etc.

[joel-costigliola]

One solution would be to change AbstractAssert.asBinary() visibility to protected and in subclasses like AbstractComparableAssert that should make it available, to overwrite it to public, users will only be able to see asBinary() where its visibility has been relaxed.

[mariuszs]

Your comments included, any progress with review?

[joel-costigliola]

No real progress :( but definitely on my short time todo list !

[joel-costigliola]

real progress :)
I have reduced the visibility of asBinary and asHexadicemal in AbstractAssert and made it public only on relevant Assert classes.
I also added code example in the javadoc but I think we could find better ones (more real life stuff).

Anyway, thanks for this big contribution and being patient !

[joel-costigliola]

Damned ! I think I missed your latest commits (not sure why), I have to merge them on top of what I did and it has some overlapping ... :(

[joel-costigliola]

Just a thought, I'm wondering if we should rename asHexadecimal to inHexadecimal ...

We are planning to add XML assertions like this :

// assertThat(xmlString).asXml() converting the String to an XML Document and returns an XmlAssert instance, 
assertThat(xmlString).asXml().containsXpath("//planet[@name='Earth']");

and writing asSomething makes think that a conversion to Something is going to be performed, but with asHexadecimal it's not exactly what happen.

With an example:

assertThat("µµµ").inHexadecimal().contains("μμμ");

Any objections ?

[mariuszs]

No, 👍 for inRepresentation :)

[mariuszs]

We should integrate JsonPath assertion and add inJson representation :)

[joel-costigliola]

Interesting ! We have already some plan to add JSON assertions, I'm adding a reference to JsonPath in #75

[joel-costigliola]

@mariuszs I don't have the compact hexa notation when executing

assertThat("zólc".getBytes()).asHexadecimal().contains("żółć".getBytes("ISO-8859-2"));

This is strange because I have integrated your work, as I had trouble with git and your branches, I have probably missed a commit.
Could you indicate me the commit to integrate ? or if it's not too much to ask to do another PR.

I'm a little bit lost right here ...

[mariuszs]

Ok, I'm looking into it

2014-01-30 Joel Costigliola notifications@github.com:

@mariuszs https://github.com/mariuszs I don't have the compact hexa
notation when executing

assertThat("zólc".getBytes()).asHexadecimal().contains("żółć".getBytes("ISO-8859-2"));

This is strange because I have integrated your work, as I had trouble with
git and your branches, I have probably missed a commit.
Could you indicate me the commit to integrate ? or if it's not too much to
ask to do another PR.

I'm a little bit lost right here ...

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/148#issuecomment-33729695
.

Mariusz Smykuła

[joel-costigliola]

thanks for looking into it !

[mariuszs]

Hmm, this works for me just fine :)

java.lang.AssertionError:
Expecting:
<[0x7A, 0xC3, 0xB3, 0x6C, 0x63]>
to contain:
<[0xBF, 0xF3, 0xB3, 0xE6]>
but could not find:
<[0xBF, 0xF3, 0xE6]>

[mariuszs]

Ok, I dont see any problems (after first look) , all is working. We have even test for this assertions

@Test
public void should_assert_bytes_as_hexadecimal() {
  thrown.expectMessage("expected:<[0x0[1]]> but was:<[0x0[2, 0x03]]>");
  assertThat(new byte[]{2, 3}).asHexadecimal().isEqualTo(new byte[]{1});
}

Please rebuild assertj from master again :)