Please change assertEquals(Collection, Collection)

1,040 views
Skip to first unread message

Olle

unread,
Apr 13, 2007, 6:57:41 AM4/13/07
to testng-users
After a lot of debugging and finally reading the TestNG JavaDoc I
think the implementation is wrong and should be replaced.

Reasons:
1 Collections don't have order and thus checking equality by order is
wrong.
2 Java Collection's equals() behaves differently (correct).

The following example shows the problem:

@Test
public void equalsTest() {
Collection c1 = new HashSet(10);
Collection c2 = new HashSet(2);
Integer i = 1;
Integer i2 = 2;
Integer i3 = 3;
c1.add(i);
c1.add(i2);
c1.add(i3);
c2.add(i);
c2.add(i2);
c2.add(i3);

*1 Assert.assertTrue(c1.equals(c2));
*2 Assert.assertEquals(c1,c2);
}

*1 works while *2 doesn't

/Olle

Alexandru Popescu ☀

unread,
Apr 13, 2007, 7:06:39 AM4/13/07
to testng...@googlegroups.com

Thanks for letting us know about your concern. But if you check the javadoc of

Assert.assertEquals(Collection, Collection, String) you will notice
that this behavior is clearly stated:

/**
* Asserts that two collections contain the same elements in the
same order. If they do not,
* an AssertionError, with the given message, is thrown.
* @param actual the actual value
* @param expected the expected value
* @param message the assertion error message
*/

Same for Assert.assertEquals(Object[] actual, Object[] expected,
String message). However in Assert you can find:

assertEqualsNoOrder(Object[] actual, Object[] expected, String message)

which according to the javadocs:

* Asserts that two arrays contain the same elements in no
particular order. If they do not,
* an AssertionError, with the given message, is thrown.

Hope this clarifies things.

bests,

./alex
--
.w( the_mindstorm )p.
TestNG co-founder
EclipseTestNG Creator


>
> >
>

Bill Michell

unread,
Apr 13, 2007, 7:27:13 AM4/13/07
to testng...@googlegroups.com
In other words, a Collection is free to decide whether it wants to maintain order over iteration or not. A java HashSet chooses not to. A LinkedList does. Both are Collections.
 
TestNG allows you to choose whether order is significant for your tests or not.

 



--
bill.m...@googlemail.com

Olle

unread,
Apr 13, 2007, 8:13:17 AM4/13/07
to testng-users

I know what the TestNG JavaDoc says that is why I didn't report it as
a Bug :-)

But in Java by definition a Collection has no order, so checking order
on one is stupid and wrong (you are applying a property, ie order, to
Collection which it does not have). A list has order so
assertEquals(List, List) would be fine for the current implementation.
If you look at my example you see that the Set:s (Collections) are
equal according to Java (Assert.assertTrue(c1.equals(c2)) returns
true) but TestNG:s Assert.assertEquals(c1,c2) returns false. Yes it
follows the TestNG JavaDoc so it is ok but it is also confusing since
it is different from normal Java behavior.

The simple fix of TestNG sources would be to use Java's implementation
of equals which would not check order on the Collection unless it was
an OrderedCollection (for example a List). Read the JavaDoc on any
Collection.equals() for info on how to handle it.

It is not really a big problem for me from now on I will never trust
any TestNG.assertEquals() function except Assert.assertTrue() which I
hope you will never change so it doesn't match the Java spec for
Boolean.equals()

Anyway just wanted to thank for a great test tool
/Olle

Alexandru Popescu ☀

unread,
Apr 13, 2007, 9:01:25 AM4/13/07
to testng...@googlegroups.com
On 4/13/07, Olle <olle.s...@gmail.com> wrote:
>
>

I kind of agree with your theoretical point, but if you check the JDK
you will notice that except sets, other collections are providing a
predetermined order, so most of the time you are using an ordered
collection. And I agree that the behavior of the Assert API may be
confusing but only for somebody who is not reading the documentation.
So,....
Last, but not least: changing this behavior would lead to tones of
failures in users' tests, so I would say that even if the theoretical
argument would win, practically we will not be able to do the change.

> Anyway just wanted to thank for a great test tool

Thanks. And keep the feedback coming.

./alex
--
.w( the_mindstorm )p.
TestNG co-founder
EclipseTestNG Creator

> /Olle
>
>
> >
>

Olle

unread,
Apr 13, 2007, 10:43:01 AM4/13/07
to testng-users

> I kind of agree with your theoretical point, but if you check the JDK
> you will notice that except sets, other collections are providing a
> predetermined order, so most of the time you are using an ordered
> collection. And I agree that the behavior of the Assert API may be
> confusing but only for somebody who is not reading the documentation.
> So,....
> Last, but not least: changing this behavior would lead to tones of
> failures in users' tests, so I would say that even if the theoretical
> argument would win, practically we will not be able to do the change.
>
> > Anyway just wanted to thank for a great test tool
>
> Thanks. And keep the feedback coming.

Hi again,

I don't want to start a flame war or something but things like this is
kind of important to us, we might have to stop using TestNG if it
doesn't live up to some standards (and that would be sad since I
haven't found any equally powerful test tool). So please tell me if
you want this discussion per e-mail or if you don't want it at all.
But back to the topic:

I don't think you would break a ton of tests, since a real
collection.equals test would be less restrictive (ie. no order check)
than the current check. The only tests it would break are those that
actually test for "order failure" on an unordered collection and those
tests should fail since Collection does not give any guarantee on
order. Lists would use Java's list.equals()

A new implementation would look something like this:

static public void assertEquals(Collection actual, Collection
expected, String message) {
if (null == message) message = "";
assertEquals(actual.size(), expected.size(), message + ":
collections don't have the same size");

if (!actual.equals(expected)) {
failNotEquals(actual, expected, "Collections contains different
objects");
}
}

This will call the list equals for lists and set equals for sets. Note
that if one is a set and the other a list they cannot per Java
definition be equal (since a list has order and a set does not) so
that is not a problem either.

As an extra feature you could use your implementation but change it to
Assert.equals(List, List) which would even further reduce the number
of backward faults (so that some of those that have used it wrong but
with List arguments would now be using it correctly).

The point I am trying to make is that it is rather meaningless to have
backward compatibility when it is more or less an error (fails on
correct code and succeeds on byggy code). All assert methods primary
task must be to follow the Java standard or they are not testing what
users expect.

An interesting variant of my example is this:


Collection c1 = new HashSet(10);
Collection c2 = new HashSet(2);
Integer i = 1;
Integer i2 = 2;
Integer i3 = 3;
c1.add(i);
c1.add(i2);
c1.add(i3);
c2.add(i);
c2.add(i2);
c2.add(i3);

Assert.assertTrue(c1.equals(c2));
Assert.assertEquals((Object) c1, (Object)c2);

Which actually passes both tests since it uses object.equals (which
works for both sets and lists) instead of your custom equals (which
only works for lists).

/Olle
PS And it is not just Set:s that have no order Bag:s don't either


Bill Michell

unread,
Apr 13, 2007, 11:09:55 AM4/13/07
to testng...@googlegroups.com
I don't agree with your theoretical point. From the javadoc (which I'm reading at http://java.sun.com/j2se/1.5.0/docs/api/)
 

The root interface in the collection hierarchy. A collection represents a group of objects, known as its elements. Some collections allow duplicate elements and others do not. Some are ordered and others unordered. The JDK does not provide any direct implementations of this interface: it provides implementations of more specific subinterfaces like Set and List. This interface is typically used to pass collections around and manipulate them where maximum generality is desired.

In other words, a Collection may be ordered, or it may be unordered.

Some tests will care about ordering. Others will not.

TestNG allows you to choose whether you care or whether you don't.

Brian Hawkins

unread,
Apr 13, 2007, 11:13:42 AM4/13/07
to testng...@googlegroups.com
On 4/13/07, Bill Michell <bill.m...@googlemail.com> wrote:
In other words, a Collection may be ordered, or it may be unordered.

Some tests will care about ordering. Others will not.

TestNG allows you to choose whether you care or whether you don't.


Yes but if you cared about order and cared about writing decent code you would not use a Collection to maintain your data.  Olle's change actually encourages good coding practices.  I like the change, it actually makes it more like the JUnit assert that just uses the .equals to check for equality.

Brian

Cédric Beust ♔

unread,
Apr 13, 2007, 11:13:37 AM4/13/07
to testng...@googlegroups.com
Hi Ollie,

I'm sympathetic with your argument, however I disagree with the following:

On 4/13/07, Olle < olle.s...@gmail.com> wrote:

The point I am trying to make is that it is rather meaningless to have
backward compatibility when it is more or less an error (fails on
correct code and succeeds on byggy code).

Unfortunately, our world is filled with buggy software that has to remain buggy because it would break so many products if it were fixed.  TestNG is no exception, and while I think you are philosophically correct in your assessment of the assert method, I'm afraid there is little we can do at this point.

Additionally, since the fix is so simple, I'm a bit surprised to hear you phrase your query in terms of "if you don't fix this, we might have to stop using TestNG". 

Why not use your own assert method that would simply use the equals-based implementation you pasted earlier?

We could also add your implementation under a different name in org.testng.Assert, it it will make a difference to you.

Let us know.

--
Cédric

Alexandru Popescu ☀

unread,
Apr 13, 2007, 11:17:02 AM4/13/07
to testng...@googlegroups.com
On 4/13/07, Olle <olle.s...@gmail.com> wrote:
>
>
> > I kind of agree with your theoretical point, but if you check the JDK
> > you will notice that except sets, other collections are providing a
> > predetermined order, so most of the time you are using an ordered
> > collection. And I agree that the behavior of the Assert API may be
> > confusing but only for somebody who is not reading the documentation.
> > So,....
> > Last, but not least: changing this behavior would lead to tones of
> > failures in users' tests, so I would say that even if the theoretical
> > argument would win, practically we will not be able to do the change.
> >
> > > Anyway just wanted to thank for a great test tool
> >
> > Thanks. And keep the feedback coming.
>
> Hi again,
>
> I don't want to start a flame war or something but things like this is
> kind of important to us, we might have to stop using TestNG if it
> doesn't live up to some standards (and that would be sad since I
> haven't found any equally powerful test tool). So please tell me if
> you want this discussion per e-mail or if you don't want it at all.

I am not sure what to say about this paragraph. I am always opened for
discussions. As regards "a standard", I would say that the
functionality was well documented and it was the way we decided to go.
In case you don't like it you are free to change it or use your own.
In case our implementation will stop you from using TestNG, then I
guess I will have to live with this :-).

> But back to the topic:
>
> I don't think you would break a ton of tests, since a real
> collection.equals test would be less restrictive (ie. no order check)
> than the current check. The only tests it would break are those that
> actually test for "order failure" on an unordered collection and those
> tests should fail since Collection does not give any guarantee on
> order. Lists would use Java's list.equals()
>

No, I think you might be missing my point. If my tests were using this
API and I am changing its semantics, I will open the door to
regression problems. If my code was supposed to guarantee order, and I
was asserting this using assertEquals, by the time the implementation
will change my code can be broken because the assertion is not anymore
doing what was doing. A contract was broken and this means breaking
backward compatibility and introducing possible bugs in systems tested
with TestNG.

> A new implementation would look something like this:
>
> static public void assertEquals(Collection actual, Collection
> expected, String message) {
> if (null == message) message = "";
> assertEquals(actual.size(), expected.size(), message + ":
> collections don't have the same size");
>
> if (!actual.equals(expected)) {
> failNotEquals(actual, expected, "Collections contains different
> objects");
> }
> }
>
> This will call the list equals for lists and set equals for sets. Note
> that if one is a set and the other a list they cannot per Java
> definition be equal (since a list has order and a set does not) so
> that is not a problem either.
>
> As an extra feature you could use your implementation but change it to
> Assert.equals(List, List) which would even further reduce the number
> of backward faults (so that some of those that have used it wrong but
> with List arguments would now be using it correctly).
>
> The point I am trying to make is that it is rather meaningless to have
> backward compatibility when it is more or less an error (fails on
> correct code and succeeds on byggy code). All assert methods primary
> task must be to follow the Java standard or they are not testing what
> users expect.
>

Once again: you are comparing your expected behavior with a pretty
well defined contract (the javadoc). If this contract is conceptually
wrong, that's another story.

Now, I am wondering what would stop you from using assertEqualsNoOrder
or any custom assertion you want to introduce, considering that
everything is well documented.


> An interesting variant of my example is this:
> Collection c1 = new HashSet(10);
> Collection c2 = new HashSet(2);
> Integer i = 1;
> Integer i2 = 2;
> Integer i3 = 3;
> c1.add(i);
> c1.add(i2);
> c1.add(i3);
> c2.add(i);
> c2.add(i2);
> c2.add(i3);
>
> Assert.assertTrue(c1.equals(c2));
> Assert.assertEquals((Object) c1, (Object)c2);
>
> Which actually passes both tests since it uses object.equals (which
> works for both sets and lists) instead of your custom equals (which
> only works for lists).
>

> PS And it is not just Set:s that have no order Bag:s don't either
>

Excuse my ignorance, but I don't know of any Bag API/impl in JDK (I
know of something called Bag in Hibernate and some other libraries,
but not in JDK -- and I was talking about JDK).

bests,

Alexandru Popescu ☀

unread,
Apr 13, 2007, 11:19:55 AM4/13/07
to testng...@googlegroups.com

In terms of that kind of check: I don't think it is pretty usefull.
Imagine 2 collection of 100 elements failing because element 74 from
the first doesn't have a correspondent in the 2nd. How are you gonna
see which one is the faulty one?

./alex
--
.w( the_mindstorm )p.
TestNG co-founder
EclipseTestNG Creator

> Brian
>
>
> >
>

jkuh...@gmail.com

unread,
Apr 13, 2007, 2:22:37 PM4/13/07
to testng-users
Interesting thread....

This actually touches on the #1 reason why I love TestNG so much -
it's pragmatic in everything it does throughout the core of the
library.

I was previously forced to provide my own assertEquals() methods for
all junit tests on all projects I've ever worked on to properly handle
collection / array matching in this more intuitive way. Finding it in
TestNG by default is a huge win as I found the opposite algorithm of
simply doing Collection.equals(otherCollection) extremely counter
intuitive.

Yes I guess in some ways it's not the clean room specification
friendly exact definition of how Collection classes handle ordering -
and I'm sure that may pose a moral problem for people who like to
stick to the specs / academic side of things in all things they do -
but I really just want to get shit done and have my tests run with
minimal effort. Things like this help improve my daily quality of life
as a programmer and I'm thankful people are making these fellow
developer friendly kinds of decisions on the project .

my 2 cents...

On Apr 13, 10:43 am, "Olle" <olle.sundb...@gmail.com> wrote:
<snipped>

Brian Hawkins

unread,
Apr 13, 2007, 2:41:34 PM4/13/07
to testng...@googlegroups.com

Interesting thread....

This actually touches on the #1 reason why I love TestNG so much -
it's pragmatic in everything it does throughout the core of the
library.

...
my 2 cents...


I disagree.  Your interface is your contract with the world.  OO programming lets you fine tune that contract through the use of inheritance and interfaces.  This is a huge benefit as it lets you clearly define your parameters but leave the implementation up to the guy on the other side.

The problem with this method is that it compares two Collections.  Collections clearly define that they are not ordered, and yet the underlying code expects order.  Very bad code design.  It would be like declaring a method to take parameters of type Object and yet underneath expect to do String compares on the objects.

The contract in question states it compares unordered collections of data.  So it either needs to do it right or the contract should be changed.

Brian

Alexandru Popescu ☀

unread,
Apr 13, 2007, 3:32:50 PM4/13/07
to testng...@googlegroups.com
On 4/13/07, Brian Hawkins <bria...@gmail.com> wrote:

No, the contract is:

* Asserts that two collections contain the same elements in the
same order. If they do not,
* an Assert

as opposed to another method which:

Asserts that two arrays contain the same elements in no
particular order.

What you are calling a bad contract is at most a bad naming, but we
still need to agree it is not the more expected behavior ;-).

./alex
--
.w( the_mindstorm )p.
TestNG co-founder
EclipseTestNG Creator

> Brian
>
>
> >
>

jkuh...@gmail.com

unread,
Apr 13, 2007, 3:39:57 PM4/13/07
to testng-users
Right,.....That was my whole point. For 90% of us this method works
exactly as we'd like / expect it to. ..

It's unfortunate that there are some possible legitimate concerns
about the interface contracts - but I'm sure if it's possible to make
everyone happy without doing any irreparable harm to existing users /
unit tests the TestNG devs will make their best effort to do so.

On Apr 13, 3:32 pm, "Alexandru Popescu ☀"
<the.mindstorm.mailingl...@gmail.com> wrote:
> On 4/13/07, Brian Hawkins <brianh...@gmail.com> wrote:

Paul Cantrell

unread,
Apr 16, 2007, 4:27:24 AM4/16/07
to testng-users
I'm with Olle on this one: anything that ignores the contract of
equals() should be named something other than assertEquals(). Still,
Alexandru is right: the problem of breaking things for existing users
is huge.

However, consider this: *nobody* can currently be relying on this
method for unordered collections (i.e. sets) -- otherwise, their tests
would sometimes succeed and sometimes fail. The order of a HashSet is
nondeterministic. Your current users thus fall into two groups:

(1) people who rely on the order sensitivity, and are using
assertEquals() on Lists, SortedSets and arrays; and

(2) people who do not want order sensitivity, and therefore cannot use
assertEquals() in its current form.


Suppose you change the implementation to use equals() instead. Does it
break anybody's code?

It won't break anything for group #1, because equals() will still be
order-sensitive for them, just like they want it.

It won't break anything for group #2, because they're not currently
using the method.

I think you're in the clear accepting Olle's patch. All it will do is
remove order sensitivity in cases where the order is nondeterministic
anyway.

Cheers,

Paul

Olle

unread,
Apr 16, 2007, 4:45:09 AM4/16/07
to testng-users
> > The root interface in the *collection hierarchy*. A collection represents
> > a group of objects, known as its *elements*. Some collections allow

> > duplicate elements and others do not. Some are ordered and others unordered.
> > The JDK does not provide any *direct* implementations of this interface:

> > it provides implementations of more specific subinterfaces like Set and
> > List. This interface is typically used to pass collections around and
> > manipulate them where maximum generality is desired.
>
> In other words, a Collection may be ordered, or it may be unordered.

Exactly a Collection says absolutely nothing about order so you should
NEVER trust order when working with Collections. If you on the other
hand is working with Lists which are a special type of Collection that
is also ordered, then you could test order.

And you you read (and understood) my argument I suggested introducing
a new Assert.assertEquals(List, List) that would handle the special
case where Collection is also ordered. (where Alexandru's current
"Collection equals" implementation could be used).

If you don't understand this aspect of OO programming I recommend that
you read more about the subject. My second example shows the problem
(when I typecast to Object the equals test succeeds ie we are
overloading assertEquals with a changed behavior which prevents safe
polymorphism).

/Olle

Olle

unread,
Apr 16, 2007, 4:45:35 AM4/16/07
to testng-users
Ok, I understand that you might not want to "break" the tests for
current users (even though their code is flawed). Personally I would
have changed it if I was developing it, but I am not.

But I hope you will at least make a BIG note in the TestNG
documentation that various testng assertEquals methods do not
necessarily follow the Java definition of equals (ie they do not
necessarily use java.equals). And the only way of being sure of a
"Java equals" is to use assertTrue.

/Olle

Alexandru Popescu ☀

unread,
Apr 16, 2007, 4:46:38 AM4/16/07
to testng...@googlegroups.com
Thanks for the comments Paul. Please see mines inlined.

On 4/16/07, Paul Cantrell <paul.c...@gmail.com> wrote:
>
> I'm with Olle on this one: anything that ignores the contract of
> equals() should be named something other than assertEquals(). Still,
> Alexandru is right: the problem of breaking things for existing users
> is huge.
>
> However, consider this: *nobody* can currently be relying on this
> method for unordered collections (i.e. sets) -- otherwise, their tests
> would sometimes succeed and sometimes fail. The order of a HashSet is
> nondeterministic. Your current users thus fall into two groups:
>
> (1) people who rely on the order sensitivity, and are using
> assertEquals() on Lists, SortedSets and arrays; and
>
> (2) people who do not want order sensitivity, and therefore cannot use
> assertEquals() in its current form.
>

They are currently using or can use assertEqualsNoOrder, so they are
not out of luck.

>
> Suppose you change the implementation to use equals() instead. Does it
> break anybody's code?
>
> It won't break anything for group #1, because equals() will still be
> order-sensitive for them, just like they want it.
>

This will lead to loosing functionality in the assert method they were
currently using: an equals based implementation will just report back:
the two collections are not equal, while the current implementation is
reporting different types of errors: different length, element
mismatch.

> It won't break anything for group #2, because they're not currently
> using the method.
>

But, after this they will have a confusion in place: why have we used
assertEqualsNoOrder of assertEquals is doing the same thing.

> I think you're in the clear accepting Olle's patch. All it will do is
> remove order sensitivity in cases where the order is nondeterministic
> anyway.
>

Indeed, I have agreed with Olle's theoretical point right from the
start, but I am still not comfortable with this change, having in mind
that: 1/ the contract is clear; 2/ the real case for the theoretical
point happens for a small fraction of the collections in the JDK.

bests,

./alex
--
.w( the_mindstorm )p.
TestNG co-founder
EclipseTestNG Creator

> Cheers,
>
> Paul
>
>
> >
>

Alexandru Popescu ☀

unread,
Apr 16, 2007, 4:54:49 AM4/16/07
to testng...@googlegroups.com
On 4/16/07, Olle <olle.s...@gmail.com> wrote:
>
> > > The root interface in the *collection hierarchy*. A collection represents
> > > a group of objects, known as its *elements*. Some collections allow
> > > duplicate elements and others do not. Some are ordered and others unordered.
> > > The JDK does not provide any *direct* implementations of this interface:
> > > it provides implementations of more specific subinterfaces like Set and
> > > List. This interface is typically used to pass collections around and
> > > manipulate them where maximum generality is desired.
> >
> > In other words, a Collection may be ordered, or it may be unordered.
>
> [trimmed /]

>
> If you don't understand this aspect of OO programming I recommend that
> you read more about the subject.
>

I am trying to be responsive and as polite as possible on a mailing
list and I usually don't tolerate people being rude in response. I am
happy for you that you understood this OO aspect, and considering this
I think you already know that you can always replace the
implementation with one that will satisfy your needs.

Paul Cantrell

unread,
Apr 16, 2007, 4:34:42 AM4/16/07
to testng-users
OK, I can think of one obscure counterexample to my idea: currently,
you can assertEquals(list, sortedSet) and get true. However, it is
never true that list.equals(sortedSet), so that might break for some
people.... (SortedSets don't even compare equal if they have different
comparators, IIRC.) I wonder if anybody is doing that? It seems like a
rare case.

I'd vote for making assertEquals() actually call equals(), and
renaming the current assertEquals() to something less misleading.

Cheers,

Paul

Olle

unread,
Apr 16, 2007, 11:08:43 AM4/16/07
to testng-users
>
> They are currently using or can use assertEqualsNoOrder, so they are
> not out of luck.
>

The main thing is not that we have no way of solving a problem but
that it effectively makes all assertEquals methods useless since you
can't trust them I might want to implement a Collection with and
equals that has some tolerance (storing for example Doubles) but since
TestNG equals sometimes has its own implementation I must remember NOT
to use it. An example for arguments sake what happens when you in the
future, write an Assert.assertEquals(File, File) not using the objects
own equals? Not very good for OO code...

>
> This will lead to loosing functionality in the assert method they were
> currently using: an equals based implementation will just report back:
> the two collections are not equal, while the current implementation is
> reporting different types of errors: different length, element
> mismatch.
>
> > It won't break anything for group #2, because they're not currently
> > using the method.
>
> But, after this they will have a confusion in place: why have we used
> assertEqualsNoOrder of assertEquals is doing the same thing.
>

My main problem with the current implementation is not that I cannot
do a specific thing but that the code does something else than
expected and that can break code for a lot of new code.

The reason I started this thread was that I spent 5h trying to find a
bug in my code before realizing that the "bug" was in TestNG. It
didn't use the equals of my objects and the implementation didn't
follow the Java specification for Collection equals.

But as you stated the implementation follows the specification in the
TestNG JavaDoc so it should pose no real problem (ie. it was my fault
since I didn't read the JavaDoc). But since the assertEquals is
heavily overloaded it is not always easy to spot which method will be
called and if just a few of them are different from the Java spec it
can be really confusing, for example the asserts below:

Collection c1 =... , c2 = ...;
Assert.assertEquals(c1, c2);
Assert.assertEquals((Object) c1, (Object) c2);

might return different results (one uses TestNG collection equals and
the other the collection's own equals).

I also does not want to start a flame war over this thing, so I will
not make any further comments on this topic, we disagree it is as
simple as that. And will have to write my own Assert if I want it my
way (which I might:-).

/Olle
PS Just to clarify things: what I said about having to stop using
TestNG is that it is not my choice. All software we use must live up
to certain quality standards and I don't set those standards and stuff
like this decrease the chance that we can use it. I was the one who
introduced TestNG at my workplace because I think it is the best unit
test software around (and I want it to be even better if possible;-).

Alexandru Popescu ☀

unread,
Apr 16, 2007, 11:44:19 AM4/16/07
to testng...@googlegroups.com
> The reason I started this thread was that I spent 5h trying to find a
> bug in my code before realizing that the "bug" was in TestNG. It
> didn't use the equals of my objects and the implementation didn't
> follow the Java specification for Collection equals.

I think I have already spent a couple of hours from my spare time on
this thread trying to help out. Unfortunately, I would call that a bug
in your development practice cause you totally ignored the
documentation. Like it or not the fact that you don't like an
implementation cannot be considered a bug.

> I also does not want to start a flame war over this thing, so I will
> not make any further comments on this topic, we disagree it is as
> simple as that. And will have to write my own Assert if I want it my
> way (which I might:-).

I thought I have already mentioned this a couple of times already. And
this way you can guarantee it is completely following OO paradigms and
also follow your shop standards.

Paul Cantrell

unread,
Apr 16, 2007, 12:27:08 PM4/16/07
to testng-users
Alex -- Specific replies are below, but a couple of things up front:

First, all I'm arguing is that the current names are misleading. It is
useful to have methods that compare only elements, elements and order,
and the full functionality of equals(). They are just confusing with
their current names. *Very* confusing, actually. I would propose
something like this:

assertEquals() // uses equals()
assertEqualOrder()
assertEqualNoOrder()

...or something along those lines. Again, if it's called
assertEquals(), it really should test equals().

Second, although I agree with Olle's point, I think he was a bit rude.
I'm sorry that this conversation became frustrating. Please know that
we are only writing because we care!

On Apr 16, 3:46 am, "Alexandru Popescu ☀"


<the.mindstorm.mailingl...@gmail.com> wrote:
> They are currently using or can use assertEqualsNoOrder, so they are
> not out of luck.

Actually, even assertEqualsNoOrder() isn't quite the expected
assertEquals(). With equals(), a List is not equal to a Set (because
ordered is not equal to unordered), and SortedSets are not equal if
they use different comparators. So equals() potentially tests several
things that assertEqualsNoOrder() does not -- and if I use
assertEquals() in a test, trusting the name "assertEquals" means what
it says, then my test is actually not giving the coverage I think it
is.

(Incidentally, in your implementation of assertEqualsNoOrder(), you
can save some code by just saying Collection actualCollection = new
HashSet(actual) instead of looping manually.)

> an equals based implementation will just report back:
> the two collections are not equal, while the current implementation is
> reporting different types of errors: different length, element
> mismatch.

Clearly that information is useful. The thing to do is check equals()
first, and if the collections are not equal, then give useful
diagnostics only if equals() fails: Are the collections different
sizes? Do they contain the same elements, but in a different order?

I'll send along some sample code for what I have in mind.

> the real case for the theoretical
> point happens for a small fraction of the collections in the JDK.

That's not really a fair argument. People who never use Sets are often
surprised to find out about their behavior, but some of us use them
quite a lot! They are not such a rare case.

Cheers,

Paul


Steve Loughran

unread,
Apr 16, 2007, 1:28:09 PM4/16/07
to testng...@googlegroups.com
On 16/04/07, Paul Cantrell <paul.c...@gmail.com> wrote:
>
> I'd vote for making assertEquals() actually call equals(), and
> renaming the current assertEquals() to something less misleading.

I know this conversation doesn't appear to be making progress, but
I've been finding it interesting watching.

Some thoughts

-change the behaviour of assertEquals() and you break existing code.
The tests are different; bad things can happen.

-one problem with assertEquals() is that you could get really stuffed
if you, say, handed it a vector of arbitrary stuff. normal objects
would be tested ok, but anything that was a collection would be tested
with the order-specific test, which could be wrong.

-I could imagine implementng my own assertEquals2(Object,Object)
method that handed off Collection comparisions to the non-ordered
test, leaving the other other stuff alone.

that latter point is important: it means for a small bit of code, any
project can have the behaviour they want. And then, regardless of the
outcome of this discussion, their code will work as they expect.
Which, when you think about it, is what most people need.

-steve

Rob Oxspring

unread,
Apr 16, 2007, 2:10:28 PM4/16/07
to testng...@googlegroups.com
Personally I think the current assertEquals(Collection,Collection) implementation is guaranteed to break expectations for developers that know and understand the Collections framework.  I also think that the current implementation could cause confusion to developers who don't understand the Collections framework - i.e. when they are shocked to learn that collection.equals(other) doesn't act like the assertEquals(Collection,Collection) method that they've come to understand. 

I'd have thought that the sensible solition is to simply deprecate the assertEquals(Collection,Collection) and provide both implementations with more explicit names such as Paul suggested but using the current implementation for the deprecated method.  This way users are encouraged away from the obviously confusing method present at the moment and are guided towards clearly named replacement methods.

Rob
Reply all
Reply to author
Forward
0 new messages