[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