Feature/13 element finder #28
Conversation
updated com.fasterxml.jackson.core to 2.10.2
added tests for conditionalWait
added friendly messages to the tests
src/main/java/aquality/selenium/core/elements/interfaces/IElementFinder.java
Outdated
Show resolved
Hide resolved
| /** | ||
| * Defines desired state for element with ability to handle exceptions. | ||
| */ | ||
| public class DesiredState { |
There was a problem hiding this comment.
can we make this class package-private? and do not we need to extract an interface of it?
There was a problem hiding this comment.
no, we cannot make it private, we will need it in child libraries.
no, I don't think that we need to extract an interface of it. This class is a simple model, and we expect it to remain the same. We don't register it in DI, so the interface is needless
| List<WebElement> resultElements = new ArrayList<>(); | ||
| try { | ||
| conditionalWait.waitFor(driver -> { | ||
| List<WebElement> currentFoundElements = driver == null ? new ArrayList<>() : driver.findElements(locator); |
There was a problem hiding this comment.
where is the driver initialized? I am asking because trying to imagine what case driver might be null
There was a problem hiding this comment.
I don't think that the driver could possibly be null (it gained from the application class, from the DI). There is only one reason for this null check: IDE code-checking plugins suggest to add it
|
|
||
| @Override | ||
| public List<WebElement> findElements(By locator, DesiredState desiredState, Long timeoutInSeconds) { | ||
| AtomicBoolean wasAnyElementFound = new AtomicBoolean(false); |
There was a problem hiding this comment.
why do we need this variable? why can't we operate only with 'resultElements.isEmpty()'?
or it is just because lambda usage?
There was a problem hiding this comment.
result elements is a different array, in fact it has elements after filtering, while this variable defines whether any element was there before filtering. please recheck your suggestion
| } | ||
|
|
||
| @Override | ||
| public List<WebElement> findElements(By locator, DesiredState desiredState, Long timeoutInSeconds) { |
There was a problem hiding this comment.
if I am right the difference between this method and another one from the ElementFinder class that here we use 'searchContextSupplier.get()' instead 'driver' - can we create one parametrized and re-use it for both cases by passing necessary supplier?
There was a problem hiding this comment.
actually no, you are wrong.
Nevertheless, I guess that I can extract the method from the lambda expression
There was a problem hiding this comment.
refactored, please check again
|
Kudos, SonarCloud Quality Gate passed!
|
3 participants
PR Details
Implement IElementFinder, add DesiredState and ElementState helpful classes
add RelativeElementFinder, add tests
Related Issue Link:
closes #13
How Has This Been Tested
Checklist