assertEquals doesn't work for sets

[mchintoanu]

TestNG Version

7.1.0

Expected behavior

assertEquals yields true if called with two sets that contain the same elements, even if they are defined as Collection.

Actual behavior

Depending on the type of set and the type of objects they contain assertEquals might work well or fail because it relies on iterators to compare the two sets and in the case of sets the iteration order is not guaranteed. This behavior is is most evident on LinkedHashSets but also appears on HashSets depending on the type of objects they contain. I suspect it works well with TreeSets, which have a predictable iteration order.

Is the issue reproductible on runner?

• Shell
• Maven
• Gradle
• Ant
• Eclipse
• IntelliJ
• NetBeans

Test case sample

Test case excerpt (full test in attachment):

    @Test
    public void assertEqualsWorksOnLinkedHashSets() {
        final Collection<Integer> oneSet = new LinkedHashSet<>();
        oneSet.add(1);
        oneSet.add(2);
        final Collection<Integer> anotherSet = new LinkedHashSet<>();
        anotherSet.add(2);
        anotherSet.add(1);
        assertTrue(oneSet.equals(anotherSet)); //passes
        assertEquals(oneSet, anotherSet); //fails
    }

AssertEqualsOnSetsTest.zip

[mchintoanu]

As a workaround you can define the expected as Set instead of Collection and wrap the actual in a new set, or wrap them both:

        assertEquals(new HashSet<>(oneSet), new HashSet<>(anotherSet));

[mchintoanu]

I implemented a fix but couldn't create a pull request, so here it is: add the following code in method org.testng.Assert#assertEquals(Collection, Collection, String) (full file attached).

    if (actual instanceof Set && expected instanceof Set) {
      assertEquals((Set) actual, (Set) expected, message);
      return;
    }

Assert.zip

[krmahadevan]

@mchintoanu - what is the issue you are facing in raising a PR?

[mchintoanu]

lack of permissions

[krmahadevan]

@mchintoanu - There should not be any permission constraints. All you need to do is, fork this repository (which will create a Xerox copy of this repo in your account), clone the forked repo to your local machine, apply the changes, push the changes to your repo and then raise a pull request.

See here for some good explanation.

[amitbhoraniya]

@mchintoanu - You are using an instance of LinkedHashSet, which means you are expecting that order should be maintained.

assertEquals method with Collection argument expects that order also should be same. If you don't wont to maintain the order then use HashSet instead of
LinkedHashSet . Same method will work for you.

final Collection<Integer> oneSet = new HashSet<>();
oneSet.add(1);
oneSet.add(2);
final Collection<Integer> anotherSet = new HashSet<>();
anotherSet.add(2);
anotherSet.add(1);
Assert.assertTrue(oneSet.equals(anotherSet)); //passes
Assert.assertEquals(oneSet, anotherSet); //passes

[mchintoanu]

@amitbhoraniya
Assert.assertEquals(oneSet, anotherSet) works only for sets containing objects which have a natural ordering defined (Integer, String, etc); it doesn't work for other sets.
Regarding your comment about LinkedHashSet: equality for LinkedHashSet is defined the same as for other types of Set: based on their contents, NOT their iteration order.

[amitbhoraniya]

Yes I agree with you, but I don't think for your issue we should change the implementation of assertEquals with Collection argument. It should be kept on iteration order only, cause for Set comparison there is already method.

[mchintoanu]

I believe the contract of assertEquals is broken as long as it doesn't return the same result as equals. And I have just thought of a simpler, cleaner solution than the one I proposed initially:

    if (actual.equals(expected)) {
      return;
    }
    //rest of assertEquals(Collection, Collection, String) implementation

I'll check out the project and try to create a pull request if I have some spare time in the following days.