Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to run in strict mode. Fail if any exception is thrown while polling. #993

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jsalinaspolo
Copy link

@jsalinaspolo jsalinaspolo commented Apr 22, 2019

Using PollingConditions might hide some errors while invoking the closure.

Adding a log.error might help to discover that an even that the test passed, an exception occurred.


This change is Reviewable

@leonard84
Copy link
Member

Hi, spock has no logging library at the moment, this is not an accident. Furthermore, you already get the error as the cause of the SpockException so I don't see a great gain here.

@jsalinaspolo
Copy link
Author

Hi,
The reason was to avoid having to wrap the closure within a try/catch each time or to create our wrapper on PollingConditions.

If you do not see a gain, feel free to reject it. Might be a good reason to do not add logging.

@jsalinaspolo
Copy link
Author

Having a second thought. You might have noticed that the SpockException is only thrown when the time elapsed times out.
If intermediate iterations fails with any exception then it will be hidden when an iteration success. In our case, we would like to know if have been any exception different than the assertion.

Instead of logging, would you prefer to have a config parameter to be unexceptional and fail the eventually if exceptions different than the assertion are thrown?

@leonard84
Copy link
Member

leonard84 commented Apr 23, 2019

Hm, having something akin to

/**
* Configures which types of Exceptions should be retried.
*
* Subclasses are included if their parent class is listed.
*
* @return array of Exception classes to retry.
*/
Class<? extends Throwable>[] exceptions() default {Exception.class, AssertionError.class};
as property for PollingConditions would be ok.

Please be aware, that we won't merge anything until 2.0 branch is green.

@leonard84 leonard84 added this to the 2.0 milestone Apr 23, 2019
@jsalinaspolo jsalinaspolo force-pushed the add-log-polling-conditions branch 2 times, most recently from bd3a168 to abdf79b Compare April 24, 2019 21:39
@jsalinaspolo
Copy link
Author

@leonard84 Let me know if you think that passing a parameter to run it on strict mode looks a good approach?

@jsalinaspolo jsalinaspolo force-pushed the add-log-polling-conditions branch from abdf79b to 6d5e6cf Compare April 24, 2019 21:43
@jsalinaspolo jsalinaspolo changed the title Log an error when during polling conditions there is an exception invoking the closure. Allow to run in strict mode. Fail if any exception is thrown while polling. Apr 24, 2019
@jsalinaspolo jsalinaspolo changed the title Allow to run in strict mode. Fail if any exception is thrown while polling. Allow to run in strict mode. Fail if any exception is thrown while polling. Apr 24, 2019
@jsalinaspolo jsalinaspolo force-pushed the add-log-polling-conditions branch from 6d5e6cf to 33a6b0f Compare January 5, 2020 15:50
@jsalinaspolo jsalinaspolo changed the base branch from spock-2.0 to master January 5, 2020 15:52
@leonard84 leonard84 removed this from the 2.0 milestone Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants