But we could integrate the patch (his work marking slow tests) and the code, and then disable the enforcing mechanism. That will allow us to re-able it quickly later on.
> On 16 Feb 2017, at 14:23, Esteban Lorenzano <[email protected]> wrote: > > >> On 16 Feb 2017, at 13:49, Guillermo Polito <[email protected]> wrote: >> >> I vote for not introducing during code-freeze. > > +1: not in code freeze. > >> >> We can discuss it for Pharo 7, I'm not against actually. But I have some >> concerns: >> >> - How does it work when we are debugging? >> I mean, imagine I'm running a test, a debugger is open, and I start working >> on it. How do these timeout currently work in this scenario? I imagine that >> the test runner should detect you entered into a "I'm debugging" mode and >> should not kill your test, but this is not clear to me how this works right >> now. >> >> - Also, how can we better document these things? Because I was bitten a >> couple of times by this timeouts and the "I kill your process but I do not >> tell you", and it was not nice because it was completely silent or obscure. >> >> Guille >> >> On Thu, Feb 16, 2017 at 1:33 PM, Denis Kudriashov <[email protected]> >> wrote: >> Hi. >> >> In Pharo 6 we introduced time limit for tests. Currently it is set to 1 >> minute by default. And concrete test cases or single tests can redefine it >> with suitable value. There is also setting to change default limit globally >> which should help to not adopt external projects immediately if current >> limit is bad for them. >> >> There is issue 19105 to make limit really small. It will set default limit >> for 2 seconds. Slow tests are already marked with bigger value. So it is >> safe for integration >> >> Here I ask for your agreement with this change. Or disagreement if there are >> good reasons to not do this. >> >> I hope following story will convince you that default time limit should be >> small: >> >> I worked on tests for new project and I got hanging tests. These tests were >> waiting for some event which not happens because of bugs. They will be >> failed after 1 minutes due to timeout. >> >> But when you work on code you of course do not want to wait 1 minute. So it >> always forces manual interrupt of running tests. But then you could not run >> full test suite to see all problem tests. And also it is not really easy to >> detect current running test after interrupt. You need analyze stack in >> debugger and if you just close debugger information is lost. >> >> So to fix it I change default timeout in each of my test cases. >> If you will work on similar code you will need same approach to comfortably >> detect and fix "synchronization" bugs. >> >> So this was my story. I am sure default behaviour with small time limit will >> make SUnit much better. >> Common case is that unit tests are supposed to be fast. Users expect it by >> default. And if something is going wrong fast tests should fail fast. >> When user wrote slow test it is expected to know that test is slow. And for >> such rare cases user can mark slow tests explicitly. But SUnit should not >> expect tests to be slow by default. >> >> I think it is really good improvement for Pharo 6. It makes TDD in Pharo >> better. >> >> Best regards, >> Denis >> >
