ensure blocks in TestCase>>tearDown are not run if an error occurs inside ensured block when test itself was halted ...
Locked
ensure blocks in TestCase>>tearDown are not run if an error occurs inside ensured block when test itself was halted ...
 
|
While attempting to characterize the "Empty Debugger" problem that I've
recently reported[1], I found that ensure blocks in TestCase>>teardown methods are not run if an Error is signaled while executing the code protected by the ensure block ... when the test itself brings up a debugger --- now that is a mouthful :) ... but a situation that commonly occurs ... I've attached a fileIn with a very simple reproduction of this problem. After filing in the code, execute the following: BugTestCase debug: #test. Abandon the first halt -- this is the halt in the test. Next a debugger is brought up on the error from inside the ensure block in the tearDown method: tearDown [ self error: 'teardown' ] ensure: [ "Imagine that something important NEEDED to be done here" self halt ]. super tearDown. Abandon the error and the `self halt` in the ensure block is not run ... If you take the `self halt` out of the test method itself, the `self halt` in the ensure block _is_ run ... This does not directly lead to the Empty Debugger, but when the ensure blocks called during teardown are not run, a resource is not cleaned up properly and that leads to SharedQueue hanging ... when I interrupt the SharedQueue, I believe that this leads to the "Empty Debugger" a separate, but more nasty problem ... Dale [1] http://forum.world.st/Re-Empty-debugger-Pharo-5-0-td4909911.html |
BugTestCase.st (826 bytes) |
|
Wow, that’s pretty bad. The process termination logic should be taking care of the ensure blocks. Unfortunately I can’t run any Pharo images at the moment but if there’s something wrong with process termination then it’s likely that me or Ben made some mistake. Could someone please rerun Dale’s test scenario with the original process termination code (e.g. from Pharo 3) and report the results?
Cheers, Max > On 8 Aug 2016, at 03:25, Dale Henrichs <[hidden email]> wrote: > > While attempting to characterize the "Empty Debugger" problem that I've recently reported[1], I found that ensure blocks in TestCase>>teardown methods are not run if an Error is signaled while executing the code protected by the ensure block ... when the test itself brings up a debugger --- now that is a mouthful :) ... but a situation that commonly occurs ... > > I've attached a fileIn with a very simple reproduction of this problem. After filing in the code, execute the following: > > BugTestCase debug: #test. > > Abandon the first halt -- this is the halt in the test. Next a debugger is brought up on the error from inside the ensure block in the tearDown method: > > tearDown > [ self error: 'teardown' ] > ensure: [ > "Imagine that something important NEEDED to be done here" > self halt ]. > super tearDown. > > Abandon the error and the `self halt` in the ensure block is not run ... > > If you take the `self halt` out of the test method itself, the `self halt` in the ensure block _is_ run ... > > This does not directly lead to the Empty Debugger, but when the ensure blocks called during teardown are not run, a resource is not cleaned up properly and that leads to SharedQueue hanging ... when I interrupt the SharedQueue, I believe that this leads to the "Empty Debugger" a separate, but more nasty problem ... > > Dale > > [1] http://forum.world.st/Re-Empty-debugger-Pharo-5-0-td4909911.html > > <BugTestCase.st> |
Re: ensure blocks in TestCase>>tearDown are not run if an error occurs inside ensured block when test itself was halted ...
|
|
2016-08-08 8:52 GMT+02:00 Max Leske <[hidden email]>: Wow, that’s pretty bad. The process termination logic should be taking care of the ensure blocks. Unfortunately I can’t run any Pharo images at the moment but if there’s something wrong with process termination then it’s likely that me or Ben made some mistake. Could someone please rerun Dale’s test scenario with the original process termination code (e.g. from Pharo 3) and report the results? Yes, test runs fine in pharo 30864 (halts in the ensure block)
|
|
|
Thanks Nicolai.
I’ve opened an issue on phogbugz: https://pharo.fogbugz.com/f/cases/18885/Ensure-blocks-in-test-tear-down-not-always-executed.
|
Re: ensure blocks in TestCase>>tearDown are not run if an error occurs inside ensured block when test itself was halted ...
|
|
Max, Thanks for looking into this. Do you think that this bug will be fixed in Pharo5.0? When I'm debugging the tests with this fatal pattern, my only recourse is to start and stop images between test runs ... The side effect of not running ensure blocks in tests is that a SharedQueue gets stuck waiting on an empty queue and when I interrupt that process (and get another debugger that is closed) I end up with the Empty Debugger problem where the debuggers have decide to stop working ... I saw that there was logic in Process>>terminate involved in dealing with processes running in critical blocks and that logic might be faulty as well ... Dale On 8/8/16 12:11 AM, Max Leske wrote:
Thanks Nicolai. |
|
|
|
Re: ensure blocks in TestCase>>tearDown are not run if an error occurs inside ensured block when test itself was halted ...
|
|
2016-08-08 15:26 GMT+02:00 Max Leske <[hidden email]>:
Hey Max, I looked at Process>>#terminate and I think the problem is that it sets isTerminating := true, but later for the "Unwind error during termination", it spaws a new debugger for this process. And if we now again press "Abondand" we don't try to terminate this process (and don't call the inner ensure-block) because isTerminating is already true.
|
|
|
Thanks Nicolai, I can’t look at the code at the moment, but as far as I recall, sending #terminate twice should signal a warning. So if what you’re saying is true, I’d expect to see a debugger opened with that warning. I also think that even if there was an unwind error, the process that is already terminating the process within termination should be the only process to continue termination. In other terms: only a single process should ever execute termination for a given process. But that’s an educated guess, as I don’t know how the exception handling is implemented for unwind errors. If it’s indeed how you describe, then I think the proper way to terminate the process in case of an unwind error, would be to pop the contexts up to the next unwind handler and execute that. #terminate should not be sent multiple times. Cheers, Max
|
Re: ensure blocks in TestCase>>tearDown are not run if an error occurs inside ensured block when test itself was halted ...
|
|
2016-08-08 17:35 GMT+02:00 Max Leske <[hidden email]>:
There are actually two ensure-blocks involved. If we "Abandon" the first debugger window, the ensure block of TestCase>>#runCase is called, which in turn throws an error from BugTestCase>>#tearDown, so we now have an exception during process terminations unwind operation. That is why the second debugger windonw shows "Unwind error during termination" instead of "Error:teardown". If we new "ABandon" this second debugger window, we try to terminate a process that is within its terminate-and-unwind operation.
|
|
|
Ok. Thanks for the analysis.
|
|
|
In reply to this post by Max Leske
Thank you all for taking care and feeling like Pharo is your baby :). Stef Le 8/8/16 à 09:11, Max Leske a écrit :
Thanks Nicolai. |
|
|
In reply to this post by Dale Henrichs-3
Hi dale
Le 8/8/16 à 15:00, Dale Henrichs a
écrit :
Definitively. We do regular backport of fixes when they are important and this one looks important.
|
«
Return to Pharo Smalltalk Developers
|
1 view|%1 views
| Free forum by Nabble | Edit this page |