-
-
Notifications
You must be signed in to change notification settings - Fork 764
fix: Make unit tests compatible with Selenium 4.33.0+ #2293
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: Make unit tests compatible with Selenium 4.33.0+ #2293
Conversation
c1e687c
to
1a5da2e
Compare
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.
I was coming here to do the same, thank you for doing this, @valfirst!
public Timeouts setScriptTimeout(Duration duration) { | ||
return this; | ||
} | ||
|
||
@Override |
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.
we should remove override annotations to be compatible to older versions. It also does not make sense to keep Deprecates ones as the original interface already does that anyway
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.
we should remove override annotations to be compatible to older versions.
Only stable non-deprecated methods are marked with override annotations. It is useful to track changes from Selenium client. Also the methods marked with with override annotations were introduced in Selenium around 4 years ago (SeleniumHQ/selenium@afa9187), so backward compatibility is maintained.
It also does not make sense to keep Deprecates ones as the original interface already does that anyway
This is for housekeeping purposes only: to ensure that we do not forget to remove them at some point in the future. I can use other approach, like, // TODO
comment
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.
I mean a compilation error will be thrown if one wants to add Override annotation to a method that does not exist in superclass or an interface, which automatically makes this fix incompatible with older selenium versions
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.
sorry, it seems I do not follow your thought, could you please elaborate a bit?
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.
sure, for example let say we have libX v1, with the following interface:
public interface Parent {
@Deprecated
public void method();
}
Then libX v2 removes it
public interface Parent {
}
Then the following client declaration would fail to compile if it uses libX v2, but would pass if libX v1 is used
public class Child implements Parent {
@Override
public void method();
}
The below declaration would fail if libX v1 is used, but would pass for v2:
public class Child implements Parent {
}
And the third one would pass for both libX v1 and libX v2:
public class Child implements Parent {
public void method();
}
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.
Got it, thank you! At the current moment the methods marked with @Override
are not deprecated and not scheduled to be removed from Selenium client. If it's needed, I can remove the annotation from them as well
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.
yes please remove the Override annotation from all methods
1a5da2e
to
0ed9e14
Compare
Change list
The PR addresses the latest breaking changes in Selenium client: SeleniumHQ/selenium@1cad0ca.
Types of changes
What types of changes are you proposing/introducing to Java client?
Details