Hi,

the following patches replace our current shell harness with a new one, based
on test-framework.

Pros:

- each test is run in its own private temporary directory
- tests can be run in parallel (although not through cabal test, but the runner
  understands -j N to specify number of threads) ... this gives a substantial
  speedup (5:03 with old harness to 1:27 with the new one with -j 4)
- one-stop place to run all the tests we have (QC, HUnit, Shell)
- we outsource part of the testing infrastructure to external lib (test-framework)
- better test selection options: you can ask for "issue123" and get tests that
  contain that string (you could only use full test names with the old runner)
  .. use -t with the runner or just the string to match with cabal test; there
  may be even some globbing support or something
- colors!

Cons:

- we can no longer run any tests without -ftest (and those extra dependencies)
- test-framework does not print a summary of the failed tests at the end like
  our previous runner used to (this could be possibly hacked in, or better, we
  could request the feature in test-framework)
- the output is slightly less nice, maybe another feature request for
  test-framework (align the results in a column or something)
- (...) I may be forgetting something

NB: The test runner comes with a dependency on a not-yet-uploaded hackage
package, namely "shellish". Grab from http://repos.mornfall.net/shellish
... that code originated in darcs-benchmark and when I upload it to hackage,
I'll change darcs-benchmark to use the library too. Upload ETA couple of days,
I need to document it properly first.

Yours,
   Petr.

4 patches for repository http://darcs.net:

Fri Aug 27 22:44:48 CEST 2010  Petr Rockai <me@mornfall.net>
  * Implement a test-framework-based shell harness.
  
  This merges the two test drivers we previously had, "unit" and
  Distribution.ShellHarness (used directly by Setup), into a single "darcs-test"
  binary. This patch only goes half the way, though: the new shell harness is not
  used by "cabal test" yet. That will come as a separate patch.

Sat Aug 28 00:03:53 CEST 2010  Petr Rockai <me@mornfall.net>
  * Couple of improvements in the new shell test runner.

Sat Aug 28 00:04:14 CEST 2010  Petr Rockai <me@mornfall.net>
  * Add a --network option to darcs-test to run the network shell tests.

Sat Aug 28 00:04:54 CEST 2010  Petr Rockai <me@mornfall.net>
  * Flip "cabal test" over to use darcs-test to run the shell tests.

Attachments

• implement-a-test_framework_based-shell-harness_.dpatch
• unnamed

Yay!

Simon: would you be interested in guest-reviewing this patch bundle
given your shelltestrunner experience?

On Fri, Aug 27, 2010 at 22:21:21 +0000, Petr Ročkai wrote:
> the following patches replace our current shell harness with a new one, based
> on test-framework.
> 
> Pros:
> 
> - each test is run in its own private temporary directory

This is a pretty big QA win (http://bugs.darcs.net/issue1283),
makes it easier to trust our tests

Two nice bonuses:

- tests that accidentally cd .. out of their test dirs don't affect the
  Darcs darcs directory
- no more clean-up after somebody else boilerplate! (we should update
  EXAMPLE.sh)

Two question:

1. when I run tests, I sometimes like to cd into the temporary directory
for forensics.  Is there a way to make this convenient?

2. what's the relationship between this work and shelltestrunner?

> - tests can be run in parallel (although not through cabal test, but the runner
>   understands -j N to specify number of threads) ... this gives a substantial
>   speedup (5:03 with old harness to 1:27 with the new one with -j 4)


> - one-stop place to run all the tests we have (QC, HUnit, Shell)

Nice

> - we outsource part of the testing infrastructure to external lib (test-framework)

That makes me happy!

> - we can no longer run any tests without -ftest (and those extra dependencies)

Can -ftest be on by default once shellish is up?

> - test-framework does not print a summary of the failed tests at the end like
>   our previous runner used to (this could be possibly hacked in, or better, we
>   could request the feature in test-framework)

Oh :-(

> - the output is slightly less nice, maybe another feature request for
>   test-framework (align the results in a column or something)

I don't mind 

> - (...) I may be forgetting something
> 
> NB: The test runner comes with a dependency on a not-yet-uploaded hackage
> package, namely "shellish". Grab from http://repos.mornfall.net/shellish
> ... that code originated in darcs-benchmark and when I upload it to hackage,
> I'll change darcs-benchmark to use the library too. Upload ETA couple of days,
> I need to document it properly first.

darcs failed:  Couldn't fetch
`0000000046-5066653559d4d6134b022d66a634a17fdcf8db35d28b447e581fec284afa4689'

> Fri Aug 27 22:44:48 CEST 2010  Petr Rockai <me@mornfall.net>
>   * Implement a test-framework-based shell harness.
>   
>   This merges the two test drivers we previously had, "unit" and
>   Distribution.ShellHarness (used directly by Setup), into a single "darcs-test"
>   binary. This patch only goes half the way, though: the new shell harness is not
>   used by "cabal test" yet. That will come as a separate patch.
> 
> Sat Aug 28 00:03:53 CEST 2010  Petr Rockai <me@mornfall.net>
>   * Couple of improvements in the new shell test runner.
> 
> Sat Aug 28 00:04:14 CEST 2010  Petr Rockai <me@mornfall.net>
>   * Add a --network option to darcs-test to run the network shell tests.
> 
> Sat Aug 28 00:04:54 CEST 2010  Petr Rockai <me@mornfall.net>
>   * Flip "cabal test" over to use darcs-test to run the shell tests.

I'll leave behind the new rough notes from when I first started trying
to review this.

Implement a test-framework-based shell harness.
-----------------------------------------------
> move ./src/unit.hs ./src/test.hs

Convergence

> move ./tests/example_binary.png ./tests/repos/example_binary.png
> move ./tests/haskell_policy.sh ./tests/failing-haskell_policy.sh
> move ./tests/repos ./tests/data
> adddir ./tests/bin
> move ./tests/hspwd.hs ./tests/bin/hspwd.hs
> move ./tests/trackdown-bisect-helper.hs ./tests/bin/trackdown-bisect-helper.hs

Seems to be general tidying

> hunk ./src/test.hs 2
>  module Main ( main ) where

> +data Format = Hashed | Darcs2 | OldFashioned deriving Show
> +data Running = Running deriving Show
> +data Result = Success | Skipped | Failed String

Just leaving this for reference
The String in Failed seems to be the log output from the shell tests

> +instance Testlike Running Result ShellTest where
> +  testTypeName _ = "Shell"
> +  runTest topts test = runImprovingIO $ liftIO (shellish $ runtest test)

I'm looking forward to seeing test-framework-shell in the future.

> +shellTest :: FilePath -> Format -> String -> Test
> +shellTest dp fmt file = Test (file ++ " (" ++ show fmt ++ ")") $ ShellTest fmt file dp
> +
> +findShell :: FilePath -> Bool -> ShIO [Test]
> +findShell dp failing =
> +  do files <- sort <$> grep relevant <$> grep (".sh" `isSuffixOf`) <$> ls "tests"
> +     return [ shellTest dp format file
> +            | format <- [ Darcs2, Hashed, OldFashioned ]
> +            , file <- files ]
> +  where relevant = (if failing then id else not) . ("failing-" `isPrefixOf`)

> +data Config = Config { failing :: Bool
> +                     , shell :: Bool
> +                     , unit :: Bool
> +                     , darcs :: String
> +                     , tests :: [String]
> +                     , threads :: Int }
> +            deriving (Data, Typeable, Eq)
> +
> +instance Attributes Config where
> +  attributes _ = group "Options"
> +    [ failing %> Help "Run the failing (shell) tests."
> +    , shell %> Help "Run the shell tests." %+ Default True
> +    , unit %> Help "Run the unit tests." %+ Default True
> +    , tests %> Help "Pattern to limit the tests to run." %+ short 't'
> +    , threads %> Default (1 :: Int) %+ short 'j' ]

This is using cmdlib

> +data DarcsTest = DarcsTest deriving Typeable
> +instance Command DarcsTest (Record Config) where
> +  run _ conf _ = do
> +    let args = [ "-j", show $ threads conf ] ++ concat [ ["-t", x ] | x <- tests conf ]
> +    ftests <- shellish $ if failing conf then findShell (darcs conf) True else return []
> +    stests <- shellish $ if shell conf then findShell (darcs conf) False else return []
> +    utests <- if unit conf then Unit.unit else return []
> +    ntests <- {- if network conf then findNetwork else -} return []
> +    defaultMainWithArgs (ftests ++ stests ++ utests ++ ntests) args
> +
> +main :: IO ()
> +main = getArgs >>= execute DarcsTest


> +set -vex
>  
>  # Print some stuff out for debugging if something goes wrong:
>  echo $HOME
> hunk ./tests/harness.sh 13
>  
>  # Check things that should be true when all the testscripts run
>  
> -test -f "$HOME"/harness.sh || { echo "HOME=\"$HOME\" is not the test suite directory"; exit 1; }
> +test -f "$HOME/lib"
> +password="AKARABNADABARAK-KARABADANKBARAKA"
> +grep $password "$HOME/test" || grep $password "$HOME/harness.sh"

Not sure what this is about

> hunk ./tests/harness.sh 17
> -homedirname=`dirname "$HOME"`
> -command -v darcs | grep "$homedirname"
> +command -v darcs | grep "$DARCS"
 
> -. ../tests/lib 
> +. lib 

Lots of these little changes modernising from the old days when failing
tests lived in bugs/; necessary for the new structure.

> hunk ./tests/gzcrcs.sh 33
> -gunzip -c repos/maybench-crc.tgz | tar xf -
> +gunzip -c $TESTDATA/maybench-crc.tgz | tar xf -

And this, less hard-coding of paths and more relying on the environment
This also makes it a little bit trickier to run tests by hand, but
that's not too big a deal (these tests are kind of rare anyway)

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, try +44 (0)1273 64 2905 or
xmpp:kowey@jabber.fr (Jabber or Google Talk only)

Hi,

Eric Kow <kowey@darcs.net> writes:
> 1. when I run tests, I sometimes like to cd into the temporary directory
> for forensics.  Is there a way to make this convenient?

My intention for this was to hook on the failing test and tar up the
working directory into a $sha1.tar.gz and point that out in the output,
probably keeping it somewhere convenient. That would both give you what
you want as well as make it possible to eventually have buildbot publish
those tar files somehow (I think it is possible to have buildsteps copy
files from the slave to the buildmaster and make them available for
download or such these days).

> 2. what's the relationship between this work and shelltestrunner?

If I understand correctly, shelltestrunner serves a fairly different
purpose, namely to run a single shell command and check its outputs
against regexps or such.

The test-framework and shell components in there are largely unrelated
to those that we have.

>> - we can no longer run any tests without -ftest (and those extra dependencies)
>
> Can -ftest be on by default once shellish is up?

I certainly think so. Unfortunately, cabal will automatically turn the
flag off if it can't satisfy the dependencies without giving a warning,
IIRC. (This only doesn't happen with cURL (where it would be actually
useful) since that's not a Haskell package.)

>> - the output is slightly less nice, maybe another feature request for
>>   test-framework (align the results in a column or something)
>
> I don't mind 

I slightly do, but I can patch my test-framework and send patches to Max
when I get around to. : - )

>> NB: The test runner comes with a dependency on a not-yet-uploaded hackage
>> package, namely "shellish". Grab from http://repos.mornfall.net/shellish
>> ... that code originated in darcs-benchmark and when I upload it to hackage,
>> I'll change darcs-benchmark to use the library too. Upload ETA couple of days,
>> I need to document it properly first.
>
> darcs failed:  Couldn't fetch
> `0000000046-5066653559d4d6134b022d66a634a17fdcf8db35d28b447e581fec284afa4689'

Booh. Darcs seems to very occasionally ignore my --umask set in
~/.darcs/defaults. Maybe bug-worthy. Anyway, fixed now. (Could this, by
any remote chance, be related to those perpetual problems that patch-tag
has with permissions?)

>> move ./tests/example_binary.png ./tests/repos/example_binary.png
>> move ./tests/haskell_policy.sh ./tests/failing-haskell_policy.sh
>> move ./tests/repos ./tests/data
>> adddir ./tests/bin
>> move ./tests/hspwd.hs ./tests/bin/hspwd.hs
>> move ./tests/trackdown-bisect-helper.hs ./tests/bin/trackdown-bisect-helper.hs
>
> Seems to be general tidying

Aye.

>> +instance Testlike Running Result ShellTest where
>> +  testTypeName _ = "Shell"
>> +  runTest topts test = runImprovingIO $ liftIO (shellish $ runtest test)
>
> I'm looking forward to seeing test-framework-shell in the future.

It *might* be just above the threshold of usefulness. I am not
sure... the actual bit of the code that's not darcs-specific is fairly
small.

>> hunk ./tests/harness.sh 13
>>  
>>  # Check things that should be true when all the testscripts run
>>  
>> -test -f "$HOME"/harness.sh || { echo "HOME=\"$HOME\" is not the test suite directory"; exit 1; }
>> +test -f "$HOME/lib"
>> +password="AKARABNADABARAK-KARABADANKBARAKA"
>> +grep $password "$HOME/test" || grep $password "$HOME/harness.sh"
>
> Not sure what this is about

This is the harness.sh test, the purpose of which is to check that the
environment for the tests is set correctly. For that, we check that a
file named $HOME/test or $HOME/harness.sh (the latter is needed to avoid
breaking the existing ShellHarness) contains a rather unlikely string,
which re-assures us, that $HOME is indeed pointing to our test's working
directory.

>> hunk ./tests/gzcrcs.sh 33
>> -gunzip -c repos/maybench-crc.tgz | tar xf -
>> +gunzip -c $TESTDATA/maybench-crc.tgz | tar xf -
>
> And this, less hard-coding of paths and more relying on the environment
> This also makes it a little bit trickier to run tests by hand, but
> that's not too big a deal (these tests are kind of rare anyway)

If that's a concern, it would be easy to add a check to lib that sets
TESTDATA to something reasonable if it is not set.

Yours,
   Petr.

Latest iteration of the patches. Shellish is on Hackage now, so nothing
substantial should stand in the way of this now.

Yours,
   Petr.

6 patches for repository http://darcs.net:

Fri Aug 27 22:44:48 CEST 2010  Petr Rockai <me@mornfall.net>
  * Implement a test-framework-based shell harness.
  
  This merges the two test drivers we previously had, "unit" and
  Distribution.ShellHarness (used directly by Setup), into a single "darcs-test"
  binary. This patch only goes half the way, though: the new shell harness is not
  used by "cabal test" yet. That will come as a separate patch.

Sat Aug 28 00:03:53 CEST 2010  Petr Rockai <me@mornfall.net>
  * Couple of improvements in the new shell test runner.

Sat Aug 28 00:04:14 CEST 2010  Petr Rockai <me@mornfall.net>
  * Add a --network option to darcs-test to run the network shell tests.

Sat Aug 28 00:04:54 CEST 2010  Petr Rockai <me@mornfall.net>
  * Flip "cabal test" over to use darcs-test to run the shell tests.

Sun Aug 29 15:47:47 CEST 2010  Petr Rockai <me@mornfall.net>
  * Bump shellish dependency to >= 0.1.1, to avoid the infinite loop in rm_rf problem.

Sun Aug 29 15:56:23 CEST 2010  Petr Rockai <me@mornfall.net>
  * Drop the "test" cabal flag and build darcs-test unconditionally.

Attachments

• implement-a-test_framework_based-shell-harness_.dpatch
• unnamed

This patch bundle (with 4 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-e81dc5b4c0bf4b808785bcb58dc6e5f7af41a3ed

This patch bundle (with 6 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-36cc331f21c39a69eb035e230368a588f04509ad

This patch bundle (with 6 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-36cc331f21c39a69eb035e230368a588f04509ad

This patch bundle (with 4 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-e81dc5b4c0bf4b808785bcb58dc6e5f7af41a3ed