Hi all,

I would like to raise this topic again.

tl;dr: I proposed to treat all warnings raised in a test case as an error of the test, similar as we already do for Halts. Currently, a test that raises a warning will break the entire test runner. Do you agree with that idea in general? :-)

If yes, I can also update the DecompilerTests so that they do not raise a UndeclaredVariableWarning any longer.

Best,
Christoph

---
Sent from Squeak Inbox Talk

On 2020-09-24T10:22:19+00:00, christoph.thiede at student.hpi.uni-potsdam.de wrote:

> 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>
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220110/e76162d2/attachment-0001.html>