GitHub 2913

[ahgittin]

Fixes #2913 .

Did you remember to?

• Add test case(s)
• Update CHANGES.txt
• Auto applied styling via ./gradlew autostyleApply

[ahgittin]

Test case checkMapNotEqualsWithNull added failing in 7a4394b passing once fixed applied in 4216a3d .

[juherr]

Could be replaced but assertFalse

[krmahadevan]

can we refactor this to

try {
     assertNotEquals((Object) m1, (Object) m2);
    Assert.fail("Maps reported as not equal when equal: " + m1 + " / " + m2);
} catch (AssertionError ignored) {}

This will do away with the extra variable succeeded

[ahgittin]

I like the var name instead of the comment -- I've called it expected.

However putting the Assert.fail call inside the try will cause the AssertionError that it throws to be ignored, won't it?, and that defeats the point of the test. I don't know of a simple way to avoid the ugly succeeded, apart from introducing lambdas or splitting up the test and using expectedExceptions or checking the body of the failure message -- all of which I think are worse. (But I don't feel at strongly and this is your codebase so LMK if you prefer something different!)

[ahgittin]

(gradle insists on putting a newline between the {} but it saves a bit of bloat)

[juherr]

You can try {} //

[krmahadevan]

@ahgittin - Sigh! my bad. Yes you are correct.

[krmahadevan]

@ahgittin It would be good if you could please help add your ID/name at the end of this line, so that we can call out your contribution in the release notes of this version (when it goes out). Its our small way of thanking you for helping TestNG become better.

[ahgittin]

All applied - TY