changed notification of item state change listeners to asynchronous c… #1704
Conversation
…alls and made it robust in case exceptions are thrown by the listener Signed-off-by: Kai Kreuzer <kai@openhab.org>
I received this error twice.
I assume this could be related to this PR |
I cannot reproduce this failure, the tests are always successful if I execute them locally. Could you check whether #1712 makes a difference? |
Thank you, seems to be solved. |
And after another one, also another error
|
Ah, its the same, just another line. |
After reverting the two commits (this change and the test fix), I was able to build ESH again. But with this PR, I am not able to build with enabled test execution. |
Hm, pretty difficult for me to analyze as I do not get any local errors :-/ |
Hi Kai, But before add a PR, one question. If we are using waitFor[Asssert] in a test, we normally want to that a condition met sometime and fail if not. I don't think we have tests that really tests that a condition is met in a given time period. The normal use case should be that a test succeeds, if it takes some time, all is fine. If a test would like to wait for not longer than e.g. 5 seconds, the timeout has to be set explicitly regardless of the default value. So, WDYT about changing the default timeout from 1s to 1min? |
Ok to change the timeout, but 1 min is a lot - I prefer a "fail fast". I would tend to say that whatever action takes more than 5-10 seconds should be considered to be a problem (and designing tests that REALLY expect to run longer than 10 seconds before an assertion is met is probably a bad style of writing tests as it will also slow down the successful builds). |
…alls and made it robust in case exceptions are thrown by the listener
Signed-off-by: Kai Kreuzer kai@openhab.org