[darcs-users] darcs patch: Sort the list of tests that are run by c... (and 2 more)

Petr Rockai me at mornfall.net
Tue Dec 23 10:35:30 UTC 2008


Thanks for the review!

Eric Kow <kowey at darcs.net> writes:
> I'll just do my usual whining about trailing whitespace ;-)
Booh. : - ( Trent, you had a magic emacs incantation to show trailing
whitespace right? Pretty please... :)

>> +testByPath x = do let [p, y] = splitDirectories x
>
> Why is this guaranteed to work?
That's the only format we support at the entry to testByPath: foo/bar.sh. The
oneTest frontend makes sure the parameter is in this form (and that it actually
exists). Hm, actually, we might also want to support

    cabal test ./test/foo-bar.sh

which will break this code path. A call to normalise might be in place:

    let [p, y] = splitDirectories $ normalise x

but I'd probably not go any further than this. We don't want to do a lot of
voodoo for a feature that no-one will use. Maybe just better error
handling...

    let dirs = splitDirectories $ normalise x
    unless (length dirs == 2) $ fail "..."
    let [p, y] = dirs

>> +oneTest :: String -> IO ()
>> +oneTest t = do let c = [t, t ++ ".sh"] ++ tryin "tests" t ++ tryin "bugs" t
>> +                       ++ tryin "network" t
>> +               run <- filterM doesFileExist c
>> +               sequence_ [ do testByPath x | x <- take 1 run ]
>
> Would mapM_ testByPath $ take 1 run be a better formulation here?
> here?
It would. Code evolution glitch.

>> +    where tryin w t = [w </> t, w </> (t ++ ".sh")]
>
> Perhaps:
>   where tryin w t = [w </> t, w </> t <.> "sh"]
Hmm. Good catch I guess, didn't know about <.>. :)

>> +allTests :: TestKind -> [String] -> IO ()
>> +allTests k s =
>>      do test `mapM` repotypes
>>         return ()
>
> Hmm... Christian (who I think wrote this original code), I think this
> could be written as mapM_ test repotypes.  I've added hlint to the hints
> in http://wiki.darcs.net/index.html/CodingStyle
Could have been me, actually. Probably another code evolution glitch.

I'll address those glitches in a separate patch.

Yours,
   Petr.

-- 
Peter Rockai | me()mornfall!net | prockai()redhat!com
 http://blog.mornfall.net | http://web.mornfall.net

"In My Egotistical Opinion, most people's C programs should be
 indented six feet downward and covered with dirt."
     -- Blair P. Houghton on the subject of C program indentation


More information about the darcs-users mailing list