telekom / testerra

Testerra is an integrated framework for automating tests for (web) applications.
https://docs.testerra.io/
Apache License 2.0
27 stars 15 forks source link

Feature/control api delay #415

Closed TobiasAdler closed 6 months ago

TobiasAdler commented 7 months ago

Description

This PR adds a new method withDelayAfterAction to CONTROL api to provide a possibility to define a delay after actions on specific elements are performed.

Example of usage:

CONTROL.withDelayAfterAction(1, () -> {
    button.click(); // Delay of 1 second after the click
});

Type of change

Checklist:

mreiche commented 7 months ago

I prefer a overloaded method withTimeout like:

CONTROL.withTimeout(int seconds, int delayAfterAction, () -> {
    element.expect().text("Hello World");
}
martingrossmann commented 7 months ago

I prefer a overloaded method withTimeout like:

CONTROL.withTimeout(int seconds, int delayAfterAction, () -> {
    element.expect().text("Hello World");
}

But using this method you must setup a new timeout. From my point there is no direct relation between a uielement timeout and a delay.

mreiche commented 7 months ago

What about something like:

CONTROL.with(new Override().timeout(3).delayAfterAction(300).retryCount(5), () -> {
    element.expect().text("Hello World");
}
martingrossmann commented 7 months ago

To be consequent I prefer to add an additional method for CONTROL. But we should change milliseconds to seconds, because all other methods using seconds.

mreiche commented 6 months ago

I don't like the Idea, of having these "internal timeouts" as regular high level API feature. The properties UiElement.ELEMENT_WAIT_INTERVAL_MS or UiElement.DELAY_AFTER_ACTION_MILLIS are internal timeouts for not spamming the WebDriver API in case of failures and give the Browser some rendering time. Thats why it is defined in milliseconds per default and they should fit usually for most use-cases.

For particular use-cases (I don't know which they are), you can always use property overriding like:

PROPERTY_MANAGER.setTestLocalProperty(UiElement.DELAY_AFTER_ACTION_MILLIS, 200);
// your test code here
martingrossmann commented 6 months ago

The idea was to slow down the execution for a defined number of steps, not for the whole testcase. Maybe the origin usecase was to specific for a project.

I agree that a user API should not update properties which are intended for internal use only.

mreiche commented 6 months ago

Slowing down the execution is a bad practice when the reason is to outplay flaky DOM elements. It is better to check for the state change in the DOM after every action.

martingrossmann commented 6 months ago

Slowing down the execution is a bad practice when the reason is to outplay flaky DOM elements. It is better to check for the state change in the DOM after every action.

Yes, thats true. I close this PR without merge.