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. + ! |
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. > + ! > > > |
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 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
|
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 |
On Thu, May 13, 2010 at 12:07 PM, Brent Pinkney <[hidden email]> wrote: Hi, 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.
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
|
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. |
> >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 |
Free forum by Nabble | Edit this page |