citrus icon indicating copy to clipboard operation
citrus copied to clipboard

repeatOnError should execute action at least once even if condition is true

Open jklingen opened this issue 7 years ago • 1 comments

Citrus Version 2.7.6

Expected behavior repeatOnError().until(true).action(errorThrowingAction) should execute errorThrowingAction exactly once, causing the test to fail

Actual behavior errorThrowingAction is not executed (because the repeat-until condition is evaluated first), causing the test to succeed

Background A simple boolean error in the repeat-until condition can make a test false-positive. As the method is called repeatOnError, I would expect the condition only to be evaluated for repetitions, not the initial execution.

jklingen avatar Sep 11 '18 08:09 jklingen

Hi!

We've discussed this issue and agree, that this is a rare corner case of the repeatOnError().until() function. One could argue back and forth whether it makes more sense to check the condition first or to execute the action first. Nevertheless, the behavior in this rare scenario is producing false positive tests and that should be fixed. We came up with the idea to keep the logic as it is but throw an exception if the condition is initially true. That prevents the repeatOnError().until(true) expression to result in false positives but also doesn't change the current logic that may be widely used by citrus users. The reason why we don't want to change the logic is, that it may have some undesired side effects in the users test setup. Just image someone used the repeatOnError().until() logic to execute an action depending on the result of a previous action. Just like repeatOnError().until(previousAction()).action(myActionWithDependency()). In this case it may be intended to skip the myActionWithDependency implementation. If we would change the logic of the evaluation, this test would break. This is just one of many possible cases where this slight change may break something.

I hope I could make our decision clear.

TL;DR False positives are bad. The logic will be untouched. We'll throw a exception if the until() condition is initially true to prevent false positives.

BR, Sven

svettwer avatar Sep 12 '18 05:09 svettwer