ComparisonFailure reports all failures as "expected: null<null> but was: null<null>" #976
Comments
|
Works for me. Can you still reproduce it? |
|
Works for me as well. Here is the trace I get - |
|
We have been seeing this error on our CI box (ubuntu), but not on our dev workstations (Mac). Switching from 4.12-beta-1 back to 4.11 fixed the problem on CI. With 4.12-beta-1: With 4.11: In this case, the test is using assertj's |
|
@dsaff What's your setup? |
|
Oddly, Mac. Likewise, setting back to 4.11 fixed the problem. |
|
I just realized that the test quoted above was not failing on our workstations (only on CI), so I'll have to report back later whether or not the |
|
I'm part-way there. If you un-tar this, and run ./gradlew :tests:test (That's on OS X, command may be somewhat different on different platforms), then the output shows the problem. Can someone confirm they're seeing the same thing? Could very very well be a gradle/junit compatibility kerfuffle. Tar file: https://drive.google.com/a/google.com/file/d/0B343vp5qh978THRKVnRDeHlIdjQ/view?usp=sharing |
|
@dsaff What version of Gradle are you using? |
|
Oh, I guess Gradle is in the tar file as well. Never mind then. I've sent you an authorization request for the tar file via Google Drive. |
|
@dsaff I still cannot access the tar file on your Google Drive account. |
|
@dsaff Thanks for the file. I can reproduce the problem. I tried remote debugging via @avh4 Are you also using Gradle? If so, we should try to get help from someone that knows how Gradle's test runner works internally since I'm not a Gradle user. |
|
I have slightly edited the example test case:
|
|
I wonder if this has something to do with is changing the field names in Do you see the same problem if you compare two integers? Or is it unique to
|
|
Yes, I was also using gradle when I saw this. |
That's exactly right. Fixing this issue in ComparisonFailure is dead simple, other serializable classes where fields names were changed need to be updated in a similar manner. diff --git a/src/main/java/org/junit/ComparisonFailure.java b/src/main/java/org/junit/ComparisonFailure.java
index 85aed73..f7d2c3f 100644
--- a/src/main/java/org/junit/ComparisonFailure.java
+++ b/src/main/java/org/junit/ComparisonFailure.java
@@ -16,7 +16,7 @@ public class ComparisonFailure extends AssertionError {
* @see ComparisonCompactor
*/
private static final int MAX_CONTEXT_LENGTH = 20;
- private static final long serialVersionUID = 1L;
+ private static final long serialVersionUID = 2L;
private String expected;
private String actual;
|
|
@anba Does Gradle serialize the exceptions thrown by JUnit tests? |
|
Apparently it does. I'm neither a Gradle developer nor user, so I don't actually know the internals of Gradle. Updating the serial UID did resolve this problem, though. Later: I've just added a |
|
@anba Thanks for digging into this! The key to understanding this issue indeed seems to be Gradle's Message class. Apparently, Gradle uses it for communication between its test runner and the system under test. In the above stacktrace the Gradle uses two separate classpathes for the test runner and the system under test. The first is called "implementation classpath", the latter "application classpath". When you enable the debug output you'll get a trace of the two. I spare you the detailed output, essentially With the current beta the When changing the Summing up, I think we need to go through all our classes that are |
|
wow! amazing sleuthing, @anba and @marcphilipp ! |
|
I think we should undo the field renames for fields that are The alternative is to see if we could find a way that we can add |
|
Here's a link to a page that describes how we would keep the fields with the new names but not break serialization: http://www.coderanch.com/t/582933/java/java/Serialization But, again, it would be challenging to write tests to verify that we don't break backwards compatibility. We could write a tool that compiles against the 4.11 jars and serializes these objects, and then write tests in 4.11 that serializes the objects and compares the output with the expected output. Note we have changes serialVersionId in the past for some exceptions ( |
|
I've submitted pull request #994 which restores the old field names for all exceptions and almost all other class that are |
|
Now that #994 has been merged, I'll publish another beta in the next few days. |
|
Works using 4.12-beta-2 now:
|
|
@dsaff Can you give it a try in your real project as well? |
|
Confirmed it's fixed in our real project. Thanks! |
|
FYI, you missed ArrayComparisonFailure.fCause, which disappeared in 4.12. This caused much hair-pulling for us as it caused a gradle test task to fail with no apparent cause. We had upgraded to gradle 2.4 which bundles junit 4.12 whereas our application classpath pointed to 4.11. |
…tCause()) to avoid npe and unused field warnings, respectively Issue junit-team#1178, junit-team#976
Add back field fCause, initialize and use in the constructor (via initCause()) to avoid npe and unused field warnings, respectively Issue junit-team#1178, junit-team#976
Add back field fCause, initialize and use in the constructor (via initCause()) to avoid npe and unused field warnings, respectively Issue junit-team#1178, junit-team#976
Still debugging locally, but since my local gradle build picked up 4.12-beta-1, all string comparison failures are reporting
org.junit.ComparisonFailure: expected: null but was: null
at org.junit.Assert.assertEquals(Assert.java:115)
at org.junit.Assert.assertEquals(Assert.java:144)
This is on comparison of two obviously non-null strings, for example assertEquals("a", "b").
This appears to be affecting our whole local team, but still may be something odd about our setup. Continuing to investigate.
The text was updated successfully, but these errors were encountered: