Proposed patch to issue 1394.  This puts a time indication on tests  
that take longer than 5 seconds.  To reduce visual bulk, it doesn't  
mark up short-running (well-behaved) tests.

Sat Nov 14 14:55:23 PST 2009  Kim Wallmark <kim_darcsnet@arlim.org>
   * Annotate long-running tests to make optimization easier.

   See http://bugs.darcs.net/issue1394

Attachments

• darcs39508

Thanks for the patch!

On Sun, Nov 15, 2009 at 06:26:57 +0000, Kimberly Wallmark wrote:
> Proposed patch to issue 1394.  This puts a time indication on tests  
> that take longer than 5 seconds.  To reduce visual bulk, it doesn't  
> mark up short-running (well-behaved) tests.
> 
> Sat Nov 14 14:55:23 PST 2009  Kim Wallmark <kim_darcsnet@arlim.org>
>    * Annotate long-running tests to make optimization easier.
> 
>    See http://bugs.darcs.net/issue1394

I thought it looked good and pushed it.  Unfortunately, I later had to
roll it back.  The problem is...

Annotate long-running tests to make optimization easier.
--------------------------------------------------------
> +import Data.Time.Clock

is that not everybody has this package, which breaks
'cabal install darcs'

Some solutions proposed by Petr
- make shell harness a separate binary
- restrict shell harness to only use dependencies that
  Cabal already has (is there a way to do this timing stuff?)

Personally, I think the time package in darcs.cabal would be good
enough, but it makes Petr grumpy because it's not a systematic
solution.

Thanks,

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

Thanks for your feedback!

I think Petr is right to be grumpy about putting time in darcs.cabal.  It causes
a chicken-and-egg problem:

Time is used in Distribution/ShellHarness.hs.  Distribution/ShellHarness.hs is
imported by Setup.lhs.  Setup.lhs is what handles "cabal [anything]".  "cabal
configure" is what tells the user that they need to go install time.

I think it's actually an insoluble loop.

I'm not confident about other options, either.  time (and the deprecated library
oldtime) are the only things that provide wall-clock time.  System.CPUTime is in
base, but measuring elapsed time in CPU time wouldn't give us accurate
information for long-running tests that are, for example, disk IO-bound.  Making
Distribution.ShellHarness a separate binary is going to make the connection
between Setup and ShellHarness ugly for no good effect.

I'm going to withdraw this patch and think about the problem more.  It may not
be possible to use non-base libraries in anything referenced by Setup.lhs.

Hi,

Kim Wallmark <bugs@darcs.net> writes:
> I'm not confident about other options, either.  time (and the deprecated library
> oldtime) are the only things that provide wall-clock time.  System.CPUTime is in
> base, but measuring elapsed time in CPU time wouldn't give us accurate
> information for long-running tests that are, for example, disk IO-bound.
even though you don't have wallclock time, you could be a little
creative with forkIO and sleep, which may be good enough and should be
available in base (although it may well be that the setup does not use
the threaded runtime and we maybe use blocking syscalls, this needs to
be checked first).

> Making Distribution.ShellHarness a separate binary is going to make
> the connection between Setup and ShellHarness ugly for no good effect.
I agree it's not such a great option, but it's still acceptable if it
helps us get the features we want. We already do this (and have to do
this) in the case of unit tests anyway, so it is not such a big loss. In
fact, if we ever ported the shell harness over to test-framework, we
would have to do this as well.

Yours,
   Petr.