Hi Christoph --

> Do you agree with that idea in general? :-)

+1 Do it. Make all cases of "Warning" or "Halt" end up as a red test result.

Best,
Marcel
Am 10.01.2022 18:22:11 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:
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 [https://github.com/hpi-swa-lab/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/20220111/cdb80bca/attachment.html>