Hi Chris,
I think this breaks some of the debugging experience for SUnit significantly. Can we please revise this again? :-)
Here are some constraints that I would expect to work:
1) When I run a test that errors or fails from a system browser (message menu), a debugger is invoked on the error of the test. 2) When I debug a test from a system browser (message menu), a debugger is invoked that's execution is very close to test method, so I need only a few clicks to actually debug the test. (But you still should be able to reach the specific #setUp method from your test class.) 3) While I am debugging a test (i.e.: watching and stepping it in a debugger) and also thereafter, it must not time out. This is because I often need more than 5 seconds to investigate what it does, and it is fatal when a timeout failure error pops out and disallows me to continue stepping through the test execution.
Your change complicates 2) and breaks 3): Regarding 2), navigating to the test method previously required 4 steps (Over, Into, Over, Into), now it requires 7 steps (Over, Over, Through, Proceed, Into, Over, Into). Until I have performed these steps, the test often already has timed out - see 3).
Yet, I think I understand the motivation to your change:
4) When I run a test that times out from a system browser (message menu), I should be informed about the timeout.
Before your patch, this never (at least not for years?) has worked, because SUnit used to first run the test invisibly, catch the timeout failure, and then run the test again without catching exceptions but also without applying a timeout, thus the test would pass in the second run (or block the image forever if the test has an infinite loop). So I like that you have fixed that, this has been confusing me and some other people who I have seen using SUnit in the past.
But I still think we need to fix the above requirements 2) and 3). Especially 3) is critical IMHO. What would be some other strategies to make timeouts more discoverable? Display a pop-up notification when the automatic debugging of a test after it failed (see CodeHolder>>#testRunSuite: and TestRunner>>#debugSuite:) completes WITHOUT having raised a debugger (a simple exception handler should do this)? Automatically unattach the timeout watchdog in the moment the running test is being debugged (I don't know a simple way to do this, and if interrupting a running test would affect its execution semantics, this might cause a lot of confusion*). What else? :-)
*Note that there is a variant of 3) that has never worked so far when you run a test and interrupt it before it succeeds/fails, which already before your patch caused the mentioned timeout failures. This one has probably never been addressed to avoid exactly such kind of confusion. It's still not perfect ...
Please let's get this fixed again. Looking forward to your ideas! :-)
Best, Christoph
--- Sent from Squeak Inbox Talk
On 2024-04-30T05:43:21+00:00, commits@source.squeak.org wrote:
Chris Muller uploaded a new version of SUnit to project The Trunk: http://source.squeak.org/trunk/SUnit-cmm.146.mcz
==================== Summary ====================
Name: SUnit-cmm.146 Author: cmm Time: 30 April 2024, 12:43:19.759559 am UUID: 51424d55-94d0-4d2e-b8dc-f470b05cb44d Ancestors: SUnit-mt.145
Let SUnit provide consistent results when running vs. debugging, rather than quietly hiding SUnit timeout configuration bugs when debugging.
=============== Diff against SUnit-mt.145 ===============
Item was changed: ----- Method: TestCase>>debug (in category 'running') ----- debug "Run the receiver and open a debugger on the first failure or error."
- ^ self assureResourcesDuring: [self runCase]!
- ^ self assureResourcesDuring: [self runCaseWithoutTimeout]!
Item was changed: ----- Method: TestCase>>debugAsFailure (in category 'running') ----- debugAsFailure "Spawn a debugger that is ready to debug the receiver."
(Process forBlock: [self debug] runUntil: [:context | context isClosureContext "navigate the process directly to the point where it is about to send #setUp"
and: [context selector = #runCase]])
and: [context selector = #runCaseWithoutTimeout]]) debug.!