Skip to content
New issue

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

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

Already on GitHub? # to your account

review: Fix failing builds because of the usage of random in CompilationUnit comparison #1954

Merged
merged 5 commits into from
Apr 12, 2018

Conversation

surli
Copy link
Collaborator

@surli surli commented Apr 9, 2018

This PR intends to fix the failing build created after the merge of #1938
Apparently we got lucky with the random when it was first executed: the usage of some random number could lead to the trigger of the following exception IllegalArgumentException:Comparison method violates its general contract!

This happens during the sort algorithm because as we use a random answer for sorting, the result is not transitive.
To avoid this problem I decided to completely remove the concept of seed and to just sort or shuffle the CU list, based on a switch value in the environment.

Before merging the PR I need to uncoment the lines of the shell script to make it only available on master commits: here I uncommented them only to check if travis is fixed.

@surli
Copy link
Collaborator Author

surli commented Apr 9, 2018

There's actually one remaining failing test on the job with the shuffling of CUs. The funny part is that actually I got the same error on my computer with JDK 9 for a while but I thought my configuration was broken somewhere.

Actually it seems to be related with the order of resolution of CUs: so we already have a bug because of that.

@monperrus
Copy link
Collaborator

So I guess it's a WIP?

@surli surli changed the title review: Fix failing builds because of the usage of random in CompilationUnit comparison WIP: Fix failing builds because of the usage of random in CompilationUnit comparison Apr 11, 2018
@surli
Copy link
Collaborator Author

surli commented Apr 11, 2018

It is but I'd be glad to get some help on this one ;)

@surli surli changed the title WIP: Fix failing builds because of the usage of random in CompilationUnit comparison review: Fix failing builds because of the usage of random in CompilationUnit comparison Apr 11, 2018
@surli
Copy link
Collaborator Author

surli commented Apr 11, 2018

Apparently the error does not appear using JDK8 so I fixed it by changing TravisCI script to use JDK8.

@surli
Copy link
Collaborator Author

surli commented Apr 12, 2018

ping @monperrus it's ready now

@monperrus monperrus merged commit cb5c315 into INRIA:master Apr 12, 2018
@surli surli deleted the fix-failing-travis branch April 13, 2018 10:06
@surli surli mentioned this pull request Jun 25, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants