On Wed, Jul 10, 2019 at 10:44:22AM -0700, Junio C Hamano wrote: > Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes: > > > +test_expect_success 'push --atomic also prevents branch creation, reports collateral' ' > > + # Setup upstream repo - empty for now > > + d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git && > > + git init --bare "$d" && > > + test_config -C "$d" http.receivepack true && > > + up="$HTTPD_URL"/smart/atomic-branches.git && > > + > > + # Tell up about two branches for now > > -ECANTPARSE "Tell up" part. s/up/"$&"/ - thanks. [snip] > Up to point, I have no possible improvements to offer ;-) > Very well done. This is nice to hear, thanks! :) > > + # the failed refs should be indicated > > + grep "master -> master" output | grep rejected && > > I'd rather see the effect, i.e. what the command did that can be > observed externally, than the report, i.e. what the command claims > to have done, if it is equally straight-forward to verify either. Hmm. I'd like to argue that part of the requirement is to show the user what happened; for example, in an earlier iteration I was not successfully reporting the "collateral damage" refs to the user, even though they were not being pushed. To that end, I'd rather check both. > > That can be done by making sure that the output from "git -C "$d" > rev-parse refs/heads/master" match output from "git rev-parse > atomic2", no? That ensures 'master' in the receiving end stayed the > same. Sure, I agree. > > > + # the collateral failure refs should be indicated > > + grep "atomic -> atomic" output | grep "atomic push failed" && > > + grep "collateral -> collateral" output | grep "atomic push failed" > > Likewise for the other two. > > FWIW, these three can further lose a process each, i.e. > > grep "^ ! .*rejected.* master -> master" output > > even if we for some reason do not want to check the effect and take > the claim by the command being tested at the face value (which I do > not think is a good idea). Will swap, wasn't sure on preference between regex or process count. Thanks. - Emily
