"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: new gotd regression test: test_repo_write_empty
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 08 Nov 2022 18:36:39 +0100

Download raw body.

Thread
  • Omar Polo:

    new gotd regression test: test_repo_write_empty

  • On 2022/11/07 21:40:11 +0100, Stefan Sperling <stsp@stsp.name> wrote:
    > This tests gotd by sending to empty repositories. Testing this use
    > case this uncovered a couple of bugs in several places. I fixed
    > some bugs directly on the main branch, and some bugs have outstanding
    > fixes on this list I sent earlier today.
    > 
    > With all those changes, this new test is passing.
    > 
    > ok? (will commit on top of all the other changes once they are in)
    
    about to forget about this one.
    
    ok op@
    
    some possible style improvements below
    
    > diff 5a3cf3b6bde9a6f3b33391a2229d7a6870417361 073e150a7dbb6832e56f9db10ab893441e996dfd
    > commit - 5a3cf3b6bde9a6f3b33391a2229d7a6870417361
    > commit + 073e150a7dbb6832e56f9db10ab893441e996dfd
    > blob - 785eb48bd53304382de88cd45d349c595ff2ec96
    > blob + 0ac2352fd9d6c39216dc6fe24a959fdd62f17cbe
    > --- regress/gotd/Makefile
    > +++ regress/gotd/Makefile
    > @@ -1,4 +1,4 @@
    > -REGRESS_TARGETS=test_repo_read test_repo_write
    > +REGRESS_TARGETS=test_repo_read test_repo_write test_repo_write_empty
    >  NOOBJ=Yes
    >  
    >  .PHONY: ensure_root prepare_test_repo check_test_repo start_gotd
    > @@ -25,6 +25,7 @@ GOTD_TEST_ENV=GOTD_TEST_ROOT=$(GOTD_TEST_ROOT) \
    >  	GOTD_TEST_REPO_URL=$(GOTD_TEST_REPO_URL) \
    >  	GOTD_TEST_REPO=$(GOTD_TEST_REPO) \
    >  	GOTD_SOCK=$(GOTD_SOCK) \
    > +	GOTD_DEVUSER=$(GOTD_DEVUSER) \
    >  	HOME=$(GOTD_TEST_USER_HOME) \
    >  	PATH=$(GOTD_TEST_USER_HOME)/bin:$(PATH)
    >  
    > @@ -48,6 +49,10 @@ test_repo_read: prepare_test_repo start_gotd
    >  	@chown ${GOTD_USER} "${GOTD_TEST_REPO}"
    >  	@su -m ${GOTD_USER} -c 'env $(GOTD_TEST_ENV) sh ./prepare_test_repo.sh'
    >  
    > +prepare_test_repo_empty: ensure_root
    > +	@chown ${GOTD_USER} "${GOTD_TEST_REPO}"
    > +	@su -m ${GOTD_USER} -c 'env $(GOTD_TEST_ENV) sh ./prepare_test_repo.sh 1'
    > +
    >  test_repo_read: prepare_test_repo start_gotd
    >  	@-$(GOTD_TRAP); su ${GOTD_TEST_USER} -c \
    >  		'env $(GOTD_TEST_ENV) sh ./repo_read.sh'
    > @@ -59,5 +64,11 @@ test_repo_write: prepare_test_repo start_gotd
    >  		'env $(GOTD_TEST_ENV) sh ./repo_write.sh'
    >  	@$(GOTD_STOP_CMD) 2>/dev/null
    >  	@su -m ${GOTD_USER} -c 'env $(GOTD_TEST_ENV) sh ./check_test_repo.sh'
    > +
    > +test_repo_write_empty: prepare_test_repo_empty start_gotd
    > +	@-$(GOTD_TRAP); su ${GOTD_TEST_USER} -c \
    > +		'env $(GOTD_TEST_ENV) sh ./repo_write_empty.sh'
    > +	@$(GOTD_STOP_CMD) 2>/dev/null
    > +	@su -m ${GOTD_USER} -c 'env $(GOTD_TEST_ENV) sh ./check_test_repo.sh'
    >  	
    >  .include <bsd.regress.mk>
    > blob - 712a02a775533761863f614cf975ef0dae5a8b49
    > blob + 24bceaedbd0b5f96864aeb98fb8f75684810689b
    > --- regress/gotd/prepare_test_repo.sh
    > +++ regress/gotd/prepare_test_repo.sh
    > @@ -16,14 +16,27 @@ if [ -e "${GOTD_TEST_REPO}" ]; then
    >  
    >  . ../cmdline/common.sh
    >  
    > +make_repo()
    > +{
    > +	local repo_path="$1"
    > +	local no_tree="$2"
    > +
    > +	gotadmin init "${repo_path}"
    > +
    > +	if [ -n "$no_tree" ]; then
    > +		return
    > +	fi
    > +
    > +	test_tree=`mktemp -d "${GOTD_TEST_ROOT}/gotd-test-tree-XXXXXXXXX"`
    > +	make_test_tree "$test_tree"
    > +	got import -m "import the test tree" -r "${GOTD_TEST_REPO}" "$test_tree" \
    > +		> /dev/null
    > +	rm -r "$test_tree" # TODO: trap
    
    the "TODO: trap" was there even before but I'm not sure what it means.
    
    > +}
    > +
    > +
    >  if [ -e "${GOTD_TEST_REPO}" ]; then
    >  	rm -rf "${GOTD_TEST_REPO}"
    >  fi
    >  
    > -gotadmin init "${GOTD_TEST_REPO}"
    > -
    > -test_tree=`mktemp -d "${GOTD_TEST_ROOT}/gotd-test-tree-XXXXXXXXX"`
    > -make_test_tree "$test_tree"
    > -got import -m "import the test tree" -r "${GOTD_TEST_REPO}" "$test_tree" \
    > -	> /dev/null
    > -rm -r "$test_tree" # TODO: trap
    > +make_repo "${GOTD_TEST_REPO}" "$1"
    > blob - /dev/null
    > blob + ff025cb335faccdf7917ec4892f40d2cb1f25548 (mode 644)
    > --- /dev/null
    > +++ regress/gotd/repo_write_empty.sh
    > @@ -0,0 +1,153 @@
    > +#!/bin/sh
    > +#
    > +# Copyright (c) 2022 Stefan Sperling <stsp@openbsd.org>
    > +#
    > +# Permission to use, copy, modify, and distribute this software for any
    > +# purpose with or without fee is hereby granted, provided that the above
    > +# copyright notice and this permission notice appear in all copies.
    > +#
    > +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
    > +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
    > +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
    > +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
    > +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
    > +# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    > +# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
    > +
    > +. ../cmdline/common.sh
    > +. ./common.sh
    > +
    > +test_send_empty() {
    > +	local testroot=`test_init send_empty`
    > +	local commit_id=`git_show_head $testroot/repo`
    > +
    > +	(cd ${GOTD_TEST_REPO} && find . > $testroot/repo-list.before)
    > +
    > +	# The gotd-controlled test repository starts out empty.
    > +	got ref -l -r ${GOTD_TEST_REPO} > $testroot/ref-list.before
    > +	echo "HEAD: refs/heads/main" > $testroot/ref-list.expected
    > +	cmp -s $testroot/ref-list.expected $testroot/ref-list.before
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		diff -u $testroot/ref-list.expected $testroot/ref-list.before
    > +		test_done "$testroot" "$ret"
    > +		return 1
    > +	fi
    > +
    > +	got checkout -q $testroot/repo $testroot/wt >/dev/null
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		echo "got checkout failed unexpectedly" >&2
    > +		test_done "$testroot" "1"
    
    no need to quote 1
    
    > +		return 1
    > +	fi
    > +
    > +	# send contents of $testroot/repo to ${GOTD_TEST_REPO}
    > +	cat >> $testroot/wt/.got/got.conf <<EOF
    > +remote "gotd" {
    > +	server ${GOTD_DEVUSER}@127.0.0.1
    > +	repository "test-repo"
    > +	protocol ssh
    > +}
    > +EOF
    > +	(cd $testroot/wt && got send -q -a gotd)
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		echo "got send failed unexpectedly" >&2
    > +		test_done "$testroot" "1"
    
    ditto
    
    > +		return 1
    > +	fi
    > +
    > +	# Server should have created a new reference.
    > +	got ref -l -r ${GOTD_TEST_REPO} > $testroot/ref-list.after
    > +	cat > $testroot/ref-list.expected <<EOF
    > +HEAD: refs/heads/main
    > +refs/heads/master: $commit_id
    > +EOF
    > +	cmp -s $testroot/ref-list.expected $testroot/ref-list.after
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		diff -u $testroot/ref-list.expected $testroot/ref-list.after
    > +		test_done "$testroot" "$ret"
    > +		return 1
    > +	fi
    > +
    > +	# Verify that the result can be cloned again.
    > +	# XXX need -b master at present because gotd does not rewrite HEAD
    > +	got clone -q -b master ${GOTD_TEST_REPO_URL} $testroot/repo-clone2
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		echo "got clone failed unexpectedly" >&2
    > +		test_done "$testroot" "1"
    
    ditto
    
    (actually all these $ret don't need to be quoted.)
    
    > +		return 1
    > +	fi
    > +
    > +	got tree -R -r $testroot/repo-clone2 > $testroot/stdout
    > +	cat > $testroot/stdout.expected <<EOF
    > +alpha
    > +beta
    > +epsilon/
    > +epsilon/zeta
    > +gamma/
    > +gamma/delta
    > +EOF
    > +	cmp -s $testroot/stdout.expected $testroot/stdout
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		diff -u $testroot/stdout.expected $testroot/stdout
    > +		test_done "$testroot" "$ret"
    > +		return 1
    > +	fi
    > +
    > +	# sending to a repository should result in a new pack file
    > +	(cd ${GOTD_TEST_REPO} && find . > $testroot/repo-list.after)
    > +	diff -u $testroot/repo-list.before $testroot/repo-list.after \
    > +		> $testroot/repo-list.diff
    > +	grep '^+[^+]' < $testroot/repo-list.diff > $testroot/repo-list.newlines
    
    no need to redirect stdin
    
    I'm not sure it's needed but I'd feel better if the first '+' was
    escaped;  However...
    
    > +	nplus=`wc -l < $testroot/repo-list.newlines | tr -d ' '`
    
    ...we can avoid this stuff with a simple awk one-liner
    
    	nplus=$(awk '/^\+[^+]/{c++} END{print c}' $testroot/repo-list.diff)
    
    dependending on whether you think keeping repo-list.diff around for
    debugging is useful, could also just pipe diff -u to awk and drop the
    file.
    
    > +	if [ "$nplus" != "4" ]; then
    > +		echo "$nplus new files created:"
    
    I think we usually echo these messages to stderr; not sure if it's
    really important though.
    
    > +		cat $testroot/repo-list.diff
    > +		test_done "$testroot" "1"
    > +		return 1
    > +	fi
    > +	egrep -q '\+\.\/objects\/pack\/pack-[a-f0-9]{40}.pack' $testroot/repo-list.newlines
    
    no need to quote /
    
    you can also avoid quoting the first '.' too.  it's a special
    character but doubt it would ever end up matching anything but '.'.
    If you feel strongly about quoting the dot however, you forgot to
    quote the second one (".pack".)
    
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		echo "new pack file not found in ${GOTD_TEST_REPO}"
    > +		cat $testroot/repo-list.newlines
    > +		test_done "$testroot" "$ret"
    > +		return 1
    > +	fi
    > +	egrep -q '\+\.\/objects\/pack\/pack-[a-f0-9]{40}.idx' $testroot/repo-list.newlines
    
    ditto
    
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		echo "new pack index not found in ${GOTD_TEST_REPO}"
    > +		test_done "$testroot" "$ret"
    > +		return 1
    > +	fi
    > +	egrep -q '\+\.\/refs\/heads' $testroot/repo-list.newlines
    
    ditto
    
    this one could become just fgrep -q '+./refs/heads'
    
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		echo "new refs/heads directory not found"
    > +		test_done "$testroot" "$ret"
    > +		return 1
    > +	fi
    > +	if ! [ -d ${GOTD_TEST_REPO}/refs/heads ]; then
    
    (while valid, I'd argue that [ ! ... ] is more idiomatic than ! [ ... ])
    
    > +		echo "new refs/heads is not a directory"
    > +		test_done "$testroot" "1"
    > +		return 1
    > +	fi
    > +	egrep -q '\+\.\/refs\/heads\/master' $testroot/repo-list.newlines
    
    ditto ditto
    
    Also, some of these could become just
    
    	if ! fgrep -q '+./refs/heads/master; then
    		echo ...
    		test_done "$testroot" 1
    		return 1
    	fi
    
    to save some churn.
    
    > +	ret=$?
    > +	if [ $ret -ne 0 ]; then
    > +		echo "new refs/heads/master not found"
    > +		test_done "$testroot" "$ret"
    > +		return 1
    > +	fi
    > +
    > +	test_done "$testroot" "$ret"
    > +}
    > +
    > +test_parseargs "$@"
    > +run_test test_send_empty
    
    
    
  • Omar Polo:

    new gotd regression test: test_repo_write_empty