Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it clear for java classes recursive comparison #3209

Open
wants to merge 7 commits into
base: 3.x
Choose a base branch
from

Conversation

FlorianCousin
Copy link

@FlorianCousin FlorianCousin commented Oct 5, 2023

Check List:

Types from java package are not comparable with recursive comparison.

To prevent developers from wasting time looking for the problem, this PR adds an explicit log.
To do so, an element is added in additionalInformation in differences when needed.

This PR does not change the behavior.

Logs before :

Top level actual and expected objects differ:
- actual value  : IntSummaryStatistics{count=0, sum=0, min=2147483647, average=0.000000, max=-2147483648} (IntSummaryStatistics@163e4e87)
- expected value: IntSummaryStatistics{count=0, sum=0, min=2147483647, average=0.000000, max=-2147483648} (IntSummaryStatistics@56de5251)

Logs after :

Top level actual and expected objects differ:
- actual value  : IntSummaryStatistics{count=0, sum=0, min=2147483647, average=0.000000, max=-2147483648} (IntSummaryStatistics@423e4cbb)
- expected value: IntSummaryStatistics{count=0, sum=0, min=2147483647, average=0.000000, max=-2147483648} (IntSummaryStatistics@6e16b8b5)
Comparison objects are of Java types and were then compared with equals method

@FlorianCousin
Copy link
Author

FlorianCousin commented Oct 5, 2023

#1815 (comment) explains why java package types are not compared field by field.

@joel-costigliola
Copy link
Member

Thanks @FlorianCousin, I will review your PR shortly !

@scordio scordio added the theme: recursive comparison An issue related to the recursive comparison label Oct 5, 2023
@FlorianCousin FlorianCousin force-pushed the fco/Make-it-clear-for-java-classes-comparison branch from 1498bbb to 9f39e98 Compare October 6, 2023 09:48
@FlorianCousin
Copy link
Author

The error I found in OS ubuntu-latest failed job is as follows.

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.6.0:javadoc (default-cli) on project assertj-guava: An error has occurred in Javadoc report generation: 
[ERROR] Exit code: 1
[ERROR] error: Error fetching URL: https://javadoc.io/doc/com.google.guava/guava/32.1.2-jre/ (java.io.IOException: Server returned HTTP response code: 521 for URL: https://javadoc.io/doc/com.google.guava/guava/32.1.2-jre/package-list)
[ERROR] error: Error fetching URL: https://javadoc.io/doc/org.assertj/assertj-core/3.25.0-SNAPSHOT/ (java.io.IOException: Server returned HTTP response code: 521 for URL: https://javadoc.io/doc/org.assertj/assertj-core/3.25.0-SNAPSHOT/package-list)

The URL https://javadoc.io/doc/com.google.guava/guava/32.1.2-jre/ is indeed not accessible.
I guess the problem is not caused by my PR but then I don't know how to fix it.

Do you have any idea ?
Could you please launch the workflows again now I rebased on main ?

@scordio
Copy link
Member

scordio commented Oct 6, 2023

javadoc.io should be back now so there shouldn't be failures anymore

@FlorianCousin FlorianCousin force-pushed the fco/Make-it-clear-for-java-classes-comparison branch from 1cb4490 to 07ff297 Compare December 23, 2023 08:39
@FlorianCousin
Copy link
Author

@joel-costigliola @scordio
I updated my branch by rebasing onto assertj/main and there is no conflict anymore.
Would you please launch the workflows ?

@FlorianCousin
Copy link
Author

2 tests have been fixed.
I think workflows can be launched again.
Sorry I had not checked the tests on my own machine before.

@FlorianCousin
Copy link
Author

An error occured during 'Upload reports" :

Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

I guess it is independant of my code changes because it looks linked to the workflows itself.

@joel-costigliola
Copy link
Member

it is @FlorianCousin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: recursive comparison An issue related to the recursive comparison
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants