Clarify assertSame message for equivalent objects

[anmol0705]

Fixes testng-team/testng-asserts#10.

This clarifies the assertSame failure message when two objects are equivalent but not the same reference.

Validation:

• .\gradlew.bat :testng-asserts:test: PASS
• git diff --check: PASS
• git show --check: PASS

Summary by CodeRabbit

• Bug Fixes
  • Improved assertSame error messages to clearly distinguish between objects that are equivalent but not identical, making assertion failures easier to diagnose.

[coderabbitai]

📝 Walkthrough

Walkthrough

Assert.failNotSame gains a branch that checks whether actual and expected are equal via areEqual before formatting the failure message; if they are equal but not the same reference, it throws an AssertionError with an "equivalent but not same" message. A new test and a changelog entry accompany the change.

Changes

assertSame failure message improvement

Layer / File(s)	Summary
failNotSame equivalent-objects branch, test, and changelog testng-asserts/src/main/java/org/testng/Assert.java, testng-asserts/src/test/java/org/testng/AssertTest.java, CHANGES.txt	failNotSame adds an areEqual check to emit a specialized "equivalent but not same" AssertionError when objects are equal but not reference-identical. A new test asserts this message fires for new String("foo") vs "foo", and the changelog records the fix under GITHUB-561.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested reviewers

• juherr
• krmahadevan

Poem

🐇 Two "foo"s walked in, equal in name,
But one was a clone — not quite the same!
The rabbit checked closely, sniffed left and right,
"Equivalent? Yes! Identical? No — that's the fight!"
Now errors speak clearly, no more confusion's game. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: clarifying assertSame message for equivalent objects, which directly addresses the PR's primary objective.
Linked Issues check	✅ Passed	The PR implements the primary objective from issue #561 by detecting equivalent but non-identical objects and providing a clearer error message, though the secondary concern about assertNotSame message redundancy is not addressed.
Out of Scope Changes check	✅ Passed	All changes are directly related to addressing issue #561: updating CHANGES.txt, modifying Assert.failNotSame() logic, and adding a test case for the new behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@testng-asserts/src/main/java/org/testng/Assert.java`:
- Around line 1586-1594: The assertSame method is calling areEqual(actual,
expected) in the failure formatting block, which executes user-defined equals()
logic. Since assertSame should only perform identity checks (using ==), not
equality checks, replace the areEqual(actual, expected) call with a direct
identity comparison using the == operator. This ensures that user-defined
equals() methods are never invoked in assertSame, preventing unexpected
exceptions from changing assertion behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 65ea2ee7-ff2c-4a70-b148-e63d125b1aa7

📥 Commits

Reviewing files that changed from the base of the PR and between de5d9b7 and 6bcdb25.

📒 Files selected for processing (3)

• CHANGES.txt
• testng-asserts/src/main/java/org/testng/Assert.java
• testng-asserts/src/test/java/org/testng/AssertTest.java

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid executing user equals() logic in assertSame failure formatting.

assertSame is identity-only, but this new path evaluates areEqual(actual, expected), which calls user-defined equals(). If equals() throws, assertSame now throws that exception instead of an AssertionError, changing assertion behavior.

Suggested fix

   private static void failNotSame(Object actual, Object expected, String message) {
     String formatted = "";
     if (message != null) {
       formatted = message + " ";
     }
-    if (areEqual(actual, expected)) {
+    boolean equivalentButNotSame = false;
+    try {
+      equivalentButNotSame = areEqual(actual, expected);
+    } catch (RuntimeException ignored) {
+      // Keep assertSame semantics stable: failure reporting must not depend on user equals().
+    }
+    if (equivalentButNotSame) {
       fail(
           formatted
               + ASSERT_EQUAL_LEFT
               + expected
               + "] but found equivalent but not same ["
               + actual
               + "]");
     }
     fail(formatted + ASSERT_EQUAL_LEFT + expected + ASSERT_MIDDLE + actual + ASSERT_RIGHT);
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testng-asserts/src/main/java/org/testng/Assert.java` around lines 1586 -
1594, The assertSame method is calling areEqual(actual, expected) in the failure
formatting block, which executes user-defined equals() logic. Since assertSame
should only perform identity checks (using ==), not equality checks, replace the
areEqual(actual, expected) call with a direct identity comparison using the ==
operator. This ensures that user-defined equals() methods are never invoked in
assertSame, preventing unexpected exceptions from changing assertion behavior.

[juherr]

Thanks for the contribution but check #3278 which replaces internal logic by assertj

[anmol0705]

Thanks for pointing me to #3278. I checked it and it looks like this PR is superseded by the AssertJ-backed assertion refactor, since assertSame/assertNotSame failure messages will come from AssertJ going forward.

I’ll close this PR to avoid duplicate/conflicting work. Thanks for the review.