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 <[email protected]> writes: > Fri Apr 3 16:27:59 EDT 2009 Adam Vogt <[email protected]> > * add test for issue1406 > > > New patches: > > [add test for issue1406 > Adam Vogt <[email protected]>**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. _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
