Replace exception catching validation on assertNotEquals

[cdalexndr]

TestNG Version

7.0.0

Expected behavior

assertNotEquals should not catch exception as a valid behaviour.

Actual behavior

assertNotEquals uses AssertionError thrown from assertEquals as validation.

Using exception breakpoint on AssertionError results on false positives because of this behaviour.

[krmahadevan]

@cdalexndr - Please add a sample that can be used to recreate the problem.

[cdalexndr]

It's not a programming bug, but an IDE usability issue.
Adding an AssertionError exception breakpoint causes the IDE to stop at breakpoints where the assertion is actually valid, because it expects to throw an AssertionError as a valid work path.

Assert.java:

public static void assertNotEquals(Object actual1, Object actual2, String message) {
    boolean fail;
    try {
      Assert.assertEquals(actual1, actual2);
      fail = true;
    } catch (AssertionError e) {
      fail = false; <-- breakpoint here, but the assertion is valid
    }

    if (fail) {
      fail(format(actual1, actual2, message, false));
    }
  }

search catch (AssertionError in file

[krmahadevan]

@cdalexndr - Its not clear what you are hinting at as a problem. What do you mean by IDE Usability issue ?

[cdalexndr]

When running my tests in debug mode, I add an AssertionError exception breakpoint, so that the IDE stops when an AssertionError is thrown. This helps me stop at errors to investigate them.
Because testng throwns AssertionError even if validation is ok, it gives me false positives.

[krmahadevan]

@cdalexndr - Ok.. But I am still confused. What are you expecting as a fix here? I have tried to annotate the method in question with some comments so that its a bit more clear.

public static void assertNotEquals(Object actual1, Object actual2, String message) {
	boolean fail;
	try {
	  Assert.assertEquals(actual1, actual2);
	  fail = true;
	} catch (AssertionError e) {
	  //We will reach here only if assertEquals() throws an AssertionError. 
	  //Which means, that our testing of "assertNotEquals()" was a success.
	  // Lets mark the boolean flag as "false" and move forward.
	  fail = false;
	}

	if (fail) {
	  fail(format(actual1, actual2, message, false));
	}
}

[cdalexndr]

Instead of calling assertEquals from assertNotEquals, it should call some assertNotEqualsImpl that throws AssertionError only when the inputs are not equal. Just like normal assert equals method but with not-equals logic.

Example:

  public static void assertNotEquals(Object actual, Object expected) {
    assertNotEquals(actual, expected, null);
  }

  public static void assertNotEquals(Object actual, Object expected, String message) {
    if (expected != null && expected.getClass().isArray()) {
      assertArrayNotEquals(actual, expected, message);
      return;
    }
    assertNotEqualsImpl(actual, expected, message);
  }

 //TODO: implement assertNotEqualsImpl and assertArrayNotEquals

This way AssertionError is thrown only when validation fails.

[krmahadevan]

@cdalexndr - Ok.. but I still don't understand why this is a problem because the AssertionError is being gobbled internally. So why would it be a problem ?

All said and done, would you be willing to raise a PR to fix this, if you still feel that this is not done right ?

We will be more than glad to help review it and get it merged.

[cdalexndr]

Ok, I'll try to make a PR