Hi Chris, Andreas,

On Thu, May 13, 2010 at 10:57 AM, Chris Muller <asqueaker at gmail.com> wrote:

> Andreas, I have hundreds of tests for Magma and various proprietary
> applications that each take anywhere from 1 millisecond to 1 hour.
> Would you tell me what I am supposed to do to allow my tests taking
> more than 5 seconds not to "timeout"?  Please don't say I have to put
> a pragma into every single method..
>
> And please don't say, "tests should be quick."  Many of my tests are
> written to be _thorough_ in that they ensure adequate volumes of data
> are handled correctly.  Unlike recently, when a significant
> performance regression was introduced 4.1 in the hashed-collections
> simply because the test was "quick" and not thorough.
>
> Tests will take different amounts of time depending on the speed of
> the machine running them.  So now when I run on a older, slower
> machine, I may get test failures just because the hardware wasn't fast
> enough, when actually the software is working normally.  I see you
> have already had to "fiddle" with the timeout value to run through
> successfully on whatever machine you are using.
>
> The reasons stated for introducing this change were:
>
> > Time out tests by default to avoid a test deadlocking or requiring
> unexpected user input.
> ..
> > The change heavily simplifies automated testing since tests that deadlock
> or require user input unexpectedly no longer lock up the testing process but
> rather fail the individual test.
>
> After years of straight-away interactive testing, this seems like a
> drastic change for a very limited use-case; some kind overnight test
> server that spams everyone with test results every morning?  Sorry,
> I'm just not happy about this..
>
> What percentage of tests encounter user-input?  Machines prompting
> users is inherently wrong anyway (like SqueakMap, prompting me to
> update the map, it's plain wrong).
>
> What percentage of tests could possibly encounter a deadlock?  That's
> for when a test involving multiple processes, right?  Small
> percentage.
>
> Therefore, I suggest we make the default timeout much higher, like two
> days, and then the very few tests that encounter user-input or
> potential deadlocks can stick it in their pragma.
>

Seems to me the correct default behaviour is that test methods without a
timeout are run with an infinite timeout and only methods with a timeout tag
will timeout.  Any "large" default timeout is kind of arbitrary.

I have to say I love the use of method tags for the test timeout.  Its an
excellent example of metadata specific to a particular method.

best
Eliot


>
>  - Chris
>
>
>
>
> On Mon, May 10, 2010 at 10:55 PM,  <commits at source.squeak.org> wrote:
> > Andreas Raab uploaded a new version of SUnit to project The Trunk:
> > http://source.squeak.org/trunk/SUnit-ar.76.mcz
> >
> > ==================== Summary ====================
> >
> > Name: SUnit-ar.76
> > Author: ar
> > Time: 10 May 2010, 8:54:50.15 pm
> > UUID: 7b624ddc-b133-894a-a273-7757b0c1b742
> > Ancestors: SUnit-ul.75
> >
> > Time out tests by default to avoid a test deadlocking or requiring
> unexpected user input. The timeout can either be set on a per-class basis
> (for example DecompilerTests>>defaultTimeout) or using the <timeout:
> seconds> tag on a per-test basis (for example LocalTest>>testLocaleChanged).
> >
> > The change heavily simplifies automated testing since tests that deadlock
> or require user input unexpectedly no longer lock up the testing process but
> rather fail the individual test.
> >
> >
> > =============== Diff against SUnit-ul.75 ===============
> >
> > Item was changed:
> >  ----- Method: TestCase>>runCase (in category 'running') -----
> >  runCase
> >
> > +       [[self setUp.
> > +       self performTest] ensure: [self tearDown]]
> > +               valueWithin: self timeoutForTest seconds
> > +               onTimeout:[TestFailure signal: 'Test timed out'].
> > +       !
> > -       [self setUp.
> > -       self performTest] ensure: [self tearDown]
> > -                       !
> >
> > Item was added:
> > + ----- Method: SUnitTest>>testTestTimeoutLoop (in category 'testing')
> -----
> > + testTestTimeoutLoop
> > +       <timeout: 1>
> > +       self should:[[true] whileTrue.] raise: TestFailure.
> > + !
> >
> > Item was added:
> > + ----- Method: SUnitTest>>testTestTimeoutTag (in category 'testing')
> -----
> > + testTestTimeoutTag
> > +       <timeout: 1>
> > +       self should:[(Delay forSeconds: 3) wait] raise: TestFailure.
> > + !
> >
> > Item was added:
> > + ----- Method: TestCase>>timeoutForTest (in category 'accessing') -----
> > + timeoutForTest
> > +       "Answer the timeout to use for this test"
> > +
> > +       | method |
> > +       method := self class lookupSelector: testSelector asSymbol.
> > +       (method pragmaAt: #timeout:) ifNotNil:[:tag| ^tag arguments
> first].
> > +       ^self defaultTimeout!
> >
> > Item was added:
> > + ----- Method: TestCase>>defaultTimeout (in category 'accessing') -----
> > + defaultTimeout
> > +       "Answer the default timeout to use for tests in this test case.
> > +       The timeout is a value in seconds."
> > +
> > +       ^5 "seconds"!
> >
> > Item was added:
> > + ----- Method: SUnitTest>>testTestTimeout (in category 'testing') -----
> > + testTestTimeout
> > +       self should:[(Delay forSeconds: 6) wait] raise: TestFailure.
> > + !
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20100513/8afcd21d/attachment.htm