willowtreeapps / conductor-mobile

Conductor Mobile is a port of the Conductor Web Framework for iOS and Android
Apache License 2.0
6 stars 5 forks source link

[BUG] isPresentWait should use visibilityOfAllElementsLocatedBy #129

Closed aaron-goff closed 5 years ago

aaron-goff commented 5 years ago

The problem

In the 0.19.0 release, isPresentWait was refactored to begin using the presenceOfAllElementsLocatedBy method. Unfortunately, this changes the behavior of isPresentWait.

In certain cases, elements can exist and be invisible (or not on the screen) and would return true with the new method.

To avoid this issue, we should use visibilityOfAllElementsLocatedBy instead.

We could also just refactor the method again to do something else...

Environment

Details

Also to keep in mind, this is a hypothetical problem that I haven't seen yet, but am anticipating when/if people start upgrading.

Link to Appium/Conductor-Mobile logs

Create a GIST which is a paste of your full Stacktrace, Appium logs, and config.yaml contents. Link them here. Do NOT paste your full logs here, as it will make this issue very long and hard to read! If you are reporting a bug, always include Relevant logs, please!

Code To Reproduce Issue [ Good To Have ]

public boolean isPresentWait(By by, long timeOutInSeconds) {
        waitForCondition(presenceOfAllElementsLocatedBy(by), timeOutInSeconds, 500);
        return isPresent(by);
    }
public boolean isPresentWait(By by, long timeOutInSeconds) {
        waitForCondition(visibilityOfAllElementsLocatedBy(by), timeOutInSeconds, 500);
        return isPresent(by);
    }