Introduce missing assertEquals(long, Long, String) overload
#3199
Conversation
WalkthroughThe pull request introduces a new method Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Some context. Found while working on PicnicSupermarket/error-prone-support#1483.
| @@ -51,6 +51,7 @@ public void assertEqualsBoxedUnboxedLong() { | |||
| assertEquals(a, b); | |||
| assertEquals(a, b, ""); | |||
| assertEquals(b, a); | |||
| assertEquals(b, a, ""); | |||
There was a problem hiding this comment.
Without the added method, this statement causes a compilation failure:
Execution failed for task ':testng-asserts:compileTestJava'.
> Compilation failed; see the compiler output below.
/tmp/testng/testng-asserts/src/test/java/test/asserttests/AssertTest.java:54: error: reference to assertEquals is ambiguous
assertEquals(b, a, "");
^
both method assertEquals(double,double,String) in Assert and method assertEquals(Object,Object,String) in Assert match
1 error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
testng-asserts/src/test/java/test/asserttests/AssertTest.java (1)
54-55: Enhance test clarity with dedicated test method or inline comment.Two additional assertions have been introduced (
assertEquals(b, a, "")andassertEquals(Long.valueOf(b), a, "")). Although these assert the same logical equality (sincebis the primitive equivalent ofLong.valueOf(b)), it may be beneficial to clarify this subtle difference. This could be done by:
- Creating a dedicated test method referencing "primitive vs. wrapped long" equivalence, or
- Adding an inline comment describing the reason for double coverage with both the primitive and the wrapper type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
testng-asserts/src/main/java/org/testng/Assert.java(1 hunks)testng-asserts/src/test/java/test/asserttests/AssertTest.java(1 hunks)
🔇 Additional comments (1)
testng-asserts/src/main/java/org/testng/Assert.java (1)
971-982: New overload aligns with existing design—but verify usage.
The newly added method assertEquals(long actual, Long expected, String message) is consistent with the existing overloads for other numeric types. Be sure to confirm that all relevant call sites are either added or updated (e.g. user code calling assertEquals with a long, Long signature).
✅ Verification successful
Let me gather more information about the usage patterns.
Implementation and usage patterns are consistent
The verification shows that:
- The new
assertEquals(long actual, Long expected, String message)overload is properly implemented with a corresponding no-message variant - The implementation correctly boxes the primitive
longtoLongbefore delegation - Existing test cases in
AssertionsTestSample.groovydemonstrate proper usage with both message and no-message variants, though they use explicitly boxed values
The implementation aligns with TestNG's assertion method patterns and has test coverage. No issues requiring updates were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify usage references of the newly added assertEquals(long, Long, String).
rg -A 5 $'assertEquals\\(\\s*[^,]+,\\s*[^,]+,\\s*\"' | grep 'long.*Long'
Length of output: 384
Script:
#!/bin/bash
# Search for assertEquals with long and Long parameters in test files
rg -t java -t groovy "assertEquals.*\(.*long.*Long.*\)" -B 1 -A 1
# Search for potential call sites that might need updating
ast-grep --pattern 'assertEquals($actual, $expected, $_)'
Length of output: 1238
This reinstates a method that was introduced in
80e02ff but accidentally replaced in
89dc584.
Summary by CodeRabbit
New Features
Tests