The Trunk: SUnit-ar.76.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: SUnit-ar.76.mcz

commits-2
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.
+ !


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: SUnit-ar.76.mcz

Chris Muller-3
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.

 - Chris




On Mon, May 10, 2010 at 10:55 PM,  <[hidden email]> 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.
> + !
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: SUnit-ar.76.mcz

Eliot Miranda-2
Hi Chris, Andreas,

On Thu, May 13, 2010 at 10:57 AM, Chris Muller <[hidden email]> 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,  <[hidden email]> 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.
> + !
>
>
>




Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: SUnit-ar.76.mcz

Brent Pinkney-2
In reply to this post by Chris Muller-3
Hi,

It is not simpler to just wrap the test in #valueWithin:OnTimeout:   ?

In that case the entire test runner can be edited by the user, it has a gui, to set the timeout ?


Brent

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: SUnit-ar.76.mcz

Eliot Miranda-2


On Thu, May 13, 2010 at 12:07 PM, Brent Pinkney <[hidden email]> wrote:
Hi,

It is not simpler to just wrap the test in #valueWithin:OnTimeout:   ?

You mean in the body of the method itself?  If so, then this doesn't seem simpler or better to me.

a) the code of the test is harder to read because its indented within a valueWithin:OnTimeout:.

b) if I wanted to run the test without a timeout I couldn't because putting the #valueWithin:OnTimeout: couples the running of the test with the timeout.


In that case the entire test runner can be edited by the user, it has a gui, to set the timeout ?

I don't understand this.  Why can't one edit the timeout in the method tag just as easily as if it were in the body of the method?  I guess I'm not understanding where and how you're suggesting do the wrapping.  Can you be more exact; assume I'm clueless (not a bad assumption in my case)?

cheers
Eliot



Brent




Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: SUnit-ar.76.mcz

Igor Stasenko
In reply to this post by Brent Pinkney-2
I having a dual feelings about this update.

>From one side, putting a small default timeout, motivates a tests
author to write tests which run fast
(so, you can expect to get a test results during your lifetime).

>From other side, a small timeouts, of course is inappropriate for
tests, which depending heavily on various side effects,
like network latency, CPU power etc.

So, i think , there one thing is missing:
 - ask suite whether its using timeouts for tests or not, i.e.:

 runCase

   self usesTimeouts ifTrue: [
       [[self setUp. self performTest] ensure: [self tearDown]]
               valueWithin: self timeoutForTest seconds
               onTimeout:[TestFailure signal: 'Test timed out'].
   ]
  ifFalse: [  self setUp. self performTest] ensure: [self tearDown]

--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: SUnit-ar.76.mcz

Chris Muller-3
> >From one side, putting a small default timeout, motivates a tests
> author to write tests which run fast
> (so, you can expect to get a test results during your lifetime).

I can almost imagine a context for a hard-limit timeout:  a hard-limit
might be useful for Squeak trunk development.  For testing a Smalltalk
system, tests are low-level, low-volume, and really really should be
brief, both to encourage trunk development and frequent running of the
tests.  I think everyone here agrees, no test should run any longer
than it needs to.  But a new community member might not know that and
write a really long test and disrupt running of tests for others..?

>   self usesTimeouts ifTrue: [
>       [[self setUp. self performTest] ensure: [self tearDown]]
>               valueWithin: self timeoutForTest seconds
>               onTimeout:[TestFailure signal: 'Test timed out'].
>   ]
>  ifFalse: [  self setUp. self performTest] ensure: [self tearDown]

My main interest is in preserving my TestCase code-base, which is
currently running in 3.9 thru trunk, and Pharo 1.0, without having to
modify a lot of methods.  I saw the pragma code and totally glossed
over the fact that, if the pragma isn't there, it just uses
defaultTimeout which I am content to override with a value of "500
days asSeconds" rather than introduce an additional boolean..  False
alarm, sorry!  :-s

Regards,
  Chris