Hi Fabio, hi all,


very good question (and after having waited about twenty minutes for two test executions and comparing them manually, I once again wish we had CI for every inbox commits ...)!


And actually, I broke the DecompilerTests, e.g. #testBlockNumbering, because somewhere during compilation, an UndeclaredVariableWarning is raised. All other tests, however, did not change their result after loading this inbox commit.


This behavior leads me to the question of whether it is justified to derive UndeclaredVariableWarning from Warning, given its #defaultAction implementation making it behave like a Notification.

Alternatively, what could we do else if it is not acceptable to interrupt a test once it raises a warning? As I proposed somewhere else, we could catch UnhandledError instead of all these classes eventually open a debugger, but Marcel told me that this would be an implementation detail of the EHS and should not be exposed. Still, I cannot imagine any other way to write an exception handler that detects whether an exception will "raise a problem" eventually or whether it won't. Hmm ... What do you think? :-)


Best,

Christoph

<http://www.hpi.de/>
________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Fabio Niephaus <lists at fniephaus.com>
Gesendet: Donnerstag, 24. September 2020 10:50:44
An: squeak-dev at lists.squeakfoundation.org
Betreff: Re: [squeak-dev] The Inbox: SUnit-ct.129.mcz

Hi Christoph,

If you run all tests in your image without and with your change, does
the number of test failures change?

Fabio

On Thu, Sep 24, 2020 at 10:42 AM <commits at source.squeak.org> wrote:
>
> Christoph Thiede uploaded a new version of SUnit to project The Inbox:
> http://source.squeak.org/inbox/SUnit-ct.129.mcz
>
> ==================== Summary ====================
>
> Name: SUnit-ct.129
> Author: ct
> Time: 24 September 2020, 10:42:52.868426 am
> UUID: 92e68d23-8472-5d48-96d3-8435bd56ac14
> Ancestors: SUnit-pre.122
>
> Proposal: Catch warnings and halts in test case execution as well as Errors.
>
> Catching (Error, Warning, Halt) is a common pattern to be (relatively) sure that no debugger will occur during an operation. For related usages, see Morph >> #fullBounds, WorldState >> #displayWorldSafely:, and many other places. IMO it is no desired behavior that the whole test execution, i.e. in a TestRunner, is interrupted because any method under test contains a halt or raises a DeprecationWarning, for example. Instead, the test should be listed as red.
>
> For a similar discussion, see https://github.com/hpi-swa/smalltalkCI/issues/470. I believe we already had talked about this on squeak-dev, but if I remember correctly, I cannot find the thread again.
>
> =============== Diff against SUnit-pre.122 ===============
>
> Item was changed:
>   ----- Method: TestCase>>timeout:after: (in category 'private') -----
>   timeout: aBlock after: seconds
>         "Evaluate the argument block. Time out if the evaluation is not
>         complete after the given number of seconds. Handle the situation
>         that a timeout may occur after a failure (during debug)"
>
>         | theProcess delay watchdog |
>
>         "the block will be executed in the current process"
>         theProcess := Processor activeProcess.
>         delay := Delay forSeconds: seconds.
>
>         "make a watchdog process"
>         watchdog := [
>                 delay wait.     "wait for timeout or completion"
>                 theProcess ifNotNil:[ theProcess signalException:
>                         (TestFailure new messageText: 'Test timed out') ]
>         ] newProcess.
>
>         "Watchdog needs to run at high priority to do its job (but not at timing priority)"
>         watchdog priority: Processor timingPriority-1.
>
>         "catch the timeout signal"
>         watchdog resume.                                "start up the watchdog"
> +       ^[aBlock on: TestFailure, (Error, Warning, Halt) do: [:ex|
> -       ^[aBlock on: TestFailure, Error, Halt do:[:ex|
>                 theProcess := nil.
>                 ex pass.
>         ]] ensure:[                                                     "evaluate the receiver"
>                 theProcess := nil.                              "it has completed, so ..."
>                 delay delaySemaphore signal.    "arrange for the watchdog to exit"
>         ]!
>
> Item was added:
> + ----- Method: TestResult class>>exAllErrors (in category 'exceptions') -----
> + exAllErrors
> +       ^ self exError, Warning, Halt
> +                       !
>
> Item was changed:
>   ----- Method: TestResult>>runCase: (in category 'running') -----
>   runCase: aTestCase
>
>         | testCasePassed timeToRun |
>         testCasePassed := true.
>
>         [timeToRun := [aTestCase runCase] timeToRunWithoutGC]
>                 on: self class failure
>                 do: [:signal |
>                                 failures add: aTestCase.
>                                 testCasePassed := false.
>                                 signal return: false]
> +               on: self class exAllErrors
> -               on: self class error
>                 do: [:signal |
>                                 errors add: aTestCase.
>                                 testCasePassed := false.
>                                 signal return: false].
>
>         testCasePassed ifTrue: [passed add: aTestCase].
>         self durations at: aTestCase put: timeToRun.!
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200924/0599dc43/attachment.html>