[darcs-users] darcs patch: add test for issue1406

Trent W. Buck trentbuck at gmail.com
Sun Apr 5 10:02:13 UTC 2009


Thanks for your path, here's the review.  Everything is just minor
nitpicking to bring it into line with our current test script style.

Adam Vogt <vogt.adam at gmail.com> writes:
> Fri Apr  3 16:27:59 EDT 2009  Adam Vogt <vogt.adam at gmail.com>
>   * add test for issue1406
>
>
> New patches:
>
> [add test for issue1406
> Adam Vogt <vogt.adam at gmail.com>**20090403202759
>  Ignore-this: 993e5e7051c68edb3cc805a3d5c4f657
> ] addfile ./bugs/issue1406.sh

The file name should have include a (very short) indication of what is
being tested.

> hunk ./bugs/issue1406.sh 1

There should be a copyright notice and license declaration here,
preferably also with one or two sentences explaining what the bug is.

> +#!/bin/sh

This should use bash (via an env trampoline), and source ../tests/lib.

> +rm -rf temp1

I like this rm -rf to have an explanatory comment in the margin, as its
not clear to a novice why cleanups are needed.

> +darcs init --repodir temp1
> +
> +cd temp1
> +
> +echo "test exit 1" > _darcs/prefs/prefs

This should use "darcs setpref", I believe.

> +echo "name" > _darcs/prefs/author
> +echo "a" > a
> +darcs record --look-for-adds --no-test --all --patch-name=p1
> +echo "b" >> a

No need for quotes around echo's input (in all these lines), but they do
no harm.

> +echo "y" | darcs amend-record --all --patch=p1

You could use the bashism <<<y.  There are some exciting issues with
pipes in bash, so avoiding them where possible is a Good Thing.

> +# There should be one patch in the repo
> +test 1 -eq `darcs changes --count` || exit 1

The "|| exit 1" is superfluous if (as you should be, via ../tests/lib)
set -e is used.

> +# Another check: there should be nothing new after a is restored
> +echo "a" > a
> +darcs whatsnew -l && exit 1 || exit 0

This last line should use not() from ../tests/lib.



More information about the darcs-users mailing list