-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
fix: update ExpectedCondition to extend java util Function #10695
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## trunk #10695 +/- ##
=======================================
Coverage 46.56% 46.56%
=======================================
Files 86 86
Lines 5863 5863
Branches 278 278
=======================================
Hits 2730 2730
Misses 2855 2855
Partials 278 278 ☔ View full report in Codecov by Sentry. |
Hmm...all the failing tests are ruby ones...surely those are not caused by this java change 🤷🏻♂️ |
The changes look good to me for now. But I think we should try to move away from Guava and implement things Java way in the long run. It will be a big change and my concern is how it will impact the users. But we can consider it for 4.3 or 4.4. |
Agreed! For now, this is just a backwards compatible change that should resolve things that were not intended to break when upgrading from selenium 3.x to 4.x (i.e. if a project used the wrong version of guava, then |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @ammmze!
Description
Some builds of guava have
Function
that does NOT extend the java native function. This causesExpectedCondition
instances to be incompatible the various waituntil
methods that were recently updated to acceptjava.util.function.Function
. See more here.Motivation and Context
Fixes #10606
Types of changes
Checklist
My change requires a change to the documentation.N/AI have updated the documentation accordingly.N/AI have added tests to cover my changes.Not testable unless we are running with a different version of guava (specifically the "android" builds because those are java 7 compatible and therefore don't have thejava.util.function.Function
interface iirc)