serenity-bdd / serenity-core

Serenity BDD is a test automation library designed to make writing automated acceptance tests easier, and more fun.
http://serenity-bdd.info
Other
722 stars 518 forks source link

Method withTimeoutOf is not overriding timeout #2395

Closed aybartsch closed 3 years ago

aybartsch commented 3 years ago

Hi there, as far as I understood the use of withTimeoutOf() should override the value "webdriver.timeouts.implicitlywait" but I'm always facing the default timeout. I was applying the below code snippet:

@FindBy(css = "td[ng-class='column.class']")
private List<WebElementFacade> tableData;
...
withTimeoutOf(Duration.ofSeconds(3)).waitFor(tableData).get(0).isVisible();
withTimeoutOf(Duration.ofSeconds(3)).waitFor(By.cssSelector("td[ng-class='column.class']"));
withTimeoutOf(Duration.ofSeconds(3)).waitForPresenceOf(By.cssSelector("td[ng-class='column.class']"));

The webelement I'm searching for is not visible in some cases and thus, I'd like to catch the NoSuchElementException after 3 seconds but I always have to wait for 60 seconds which is the default value set. I don't understand why this exception is not thrown after 3 seconds. Could you please tell me what went wrong here?

Thanks, André

aybartsch commented 3 years ago

In PageObject class I found:

    public RenderedPageObjectView withTimeoutOf(int timeout, TemporalUnit units) {
        return withTimeoutOf(Duration.of(timeout, units));
    }

    public RenderedPageObjectView withTimeoutOf(Duration timeout) {
        return new RenderedPageObjectView(driver, this, timeout, false);
    }

The value for timeoutCanBeOverriden (last signature value of RenderedPageObjectView) is always set to false. Seems to be wrong.

aybartsch commented 3 years ago

Hi there, could you please reply to issue? I'm still facing this issue and I guess it's an error caused by serenity. Thanks and best regards, André

globalworming commented 3 years ago

Why do you have a default timeout of 60 seconds though? I'd recommend setting timeouts to only a few seconds in general and introduce fluent waits like e.g. actor.attemptsTo(WaitUntil.the(target, isVisible()).forNoMoreThan(10).seconds()) where you explicitly need them

aybartsch commented 3 years ago

@globalworming Thanks for your reply. Unfortunately, we are not using the screenplay pattern so far. How would the common fluent wait look like?

In general, I have some doubts about serenity implementation of "withTimeoutOf()" as I have written on Feb. 23. Am I right?

wakaleo commented 3 years ago

The timeoutCanBeOverriden field is an internal field that is used in other circumstances - it is not relevant for this use case. Generally speaking @globalworming is right - your implicit wait timeouts should be kept fairly low. But I can't reproduce the behaviour you describe - in the latest version at least, I can confirm that the withTimeoutOf(...) method will override default timeout values.

aybartsch commented 3 years ago

@wakaleo Thanks for your answer. We are using serenity version 2.4.51 but the below usage of withTimeoutOf() is never overwriting the webdriver.timeouts.implicitlywait set in serenity properties file:

withTimeoutOf(Duration.ofSeconds(3)).waitFor(divScreenLocked).isPresent()

Could the usage of waitFor() combined with withTimeoutOf() be wrong?

wakaleo commented 3 years ago

Maybe try with version 2.5.8. Here are some examples of exercises from some of the Serenity training modules:

    public void waitForMessageToDissapear() {
        withTimeoutOf(Duration.ofSeconds(10)).waitForAbsenceOf(".alert");
    }
    public void start() {
        $("#start button").click();
        withTimeoutOf(Duration.ofSeconds(10)).waitFor(visibilityOfElementLocated("#finish"));
    }
aybartsch commented 3 years ago

I think I have mixed up the implicit with the fluent wait and thus, I had always to wait until implicit wait (60 secs) was over. Afterwards, the withTimeoutOf() was applied overriding the fluent wait (30 secs).

I have reduced the implicit wait now and it looks much better than before. Thank you!