[darcs-users] darcs patch: modify issue1300 test to fail for the ri... (and 2 more)

Eric Kow kowey at darcs.net
Thu Sep 17 07:56:26 UTC 2009


Hi Kamil,

I've applied this patch, thanks!

By the way, reviewers/mutt users: for hand-attached patches, you can
hit '^E' to change the attachment type from application/octet-stream
to text/plain so that you can easily reply to them.

modify issue1300 test to fail for the right reason
--------------------------------------------------
> Kamil Dworakowski <kamil at dworakowski.name>**20090915090307
>  Ignore-this: daae62898293f2871023f52d25a8eb4f
> ] hunk ./tests/failing-issue1300_record_delete-file.sh 48
>  darcs setpref test 'exit 1'
>  echo 'test fail' > f
> -darcs record -am g --test --logfile log --delete-logfile
> +not darcs record -am g --test --logfile log --delete-logfile
>  test -e log # should *not* be deleted

Oops, thanks!  (Here the problem was that darcs record was failing due
to the deliberate test failure.  A failure was what I wanted, but I
didn't check to make sure it was the *right* failure)

resolve issue1300: logfile deleted on unsucessful record
--------------------------------------------------------
>  Don't honour --delete-logfile when the record fails for any reason, a test
>  failure for instance.

>  I have changed the definition of get_log not to delete the logfile, but to
>  return it for deferred deletion. I capitalized on the fact that get_log
>  was already returning (Just temp_logfile) for deferred deletion.
>  
>  get_log is an exported name, used in AmendRecord, Tag and Rollback. Some
>  of them ignore the logfile to delete, though none of them accept
>  --delete-logfile flag, and thus they don't need to change, nor are they
>  affected in any way.
>  
>  Unintended side effect of the change: with --delete-logfile flag present, a
>  massage '"Logfile left in " ++ filepath' gets printed in case of the test
>  failure on record. This may actually be desirable so I did not bother to
>  change it.

This is a fine log, but it should be made more concise :-)
--edit-description is a good place for some stuff.

I don't have any fixed rules about this by the way (yet); it's just a
sort of "feeling" for what will be most useful when you're trying to
study the darcs changes output in the future.

> ] move ./tests/failing-issue1300_record_delete-file.sh ./tests/issue1300_record_delete-file.sh
>                   (name, thelog, _) <- read_long_comment f firstname
> -                 when (RmLogFile `elem` opts) $ removeFileMayNotExist f
> -                 return (name, thelog, Nothing)
> +                 let toRemove = if RmLogFile `elem` opts
> +                        then Just $ toFilePath f
> +                        else Nothing
> +                 return (name, thelog, toRemove)

Looks good to me.  As Kamil says, the removal instruction is later
picked up by do_actual_record.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090917/69664e6f/attachment.pgp>


More information about the darcs-users mailing list