I intentionally didn't set -e because I don't know how to use that and have the script ignore the failure that the script from exiting on the failure that must happen on the amend-record.
The other suggestions wouldn't be bad, though explicit copyright and licencing for a (hopefully transient) test case seems pretty excessive: I imagine actually fixing the bug (only affects old-style repos) would probably be less effort :) * On Monday, April 06 2009, Eric Kow wrote: >On Sun, Apr 05, 2009 at 20:02:13 +1000, Trent W. Buck wrote: >> Thanks for your path, here's the review. Everything is just minor >> nitpicking to bring it into line with our current test script style. > >Applied, thanks! > >Adam: these sound like useful nitpicks to me. Could you have another >look? > >> 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. > >-- >Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> >PGP Key ID: 08AC04F9 _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
