From: Omar Polo <op@omarpolo.com>
Subject: Re: got: fix incorrect argc check when diffing blobs
To: Stefan Sperling <stsp@stsp.name>
Cc: Mark Jamsek <mark@jamsek.com>, Game of Trees <gameoftrees@openbsd.org>
Date: Tue, 30 Aug 2022 12:10:53 +0200

On 2022/08/30 11:40:47 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Thu, Aug 25, 2022 at 10:13:26PM +0200, Omar Polo wrote:
> > agreed, and the similar codepath for diffing objects has the `argc > 2' check.
> > 
> > ok for me.
> > 
> > should we add something like this in regress?
> > 
> > diff /home/op/w/got
> > commit - f0680473a7db1e5941bffdc2ab5f80ddec209122
> > path + /home/op/w/got
> > blob - 136f659b08eaf6408f6c79e05f6440fe7c754523
> > file + regress/cmdline/diff.sh
> > --- regress/cmdline/diff.sh
> > +++ regress/cmdline/diff.sh
> > @@ -385,6 +385,43 @@ test_diff_basic() {
> >  		diff -u $testroot/stderr.expected $testroot/stderr
> >  		return 1
> >  	fi
> > +
> > +	# diff two blob ids, but first commit the changes to alpha
> > +	(cd $testroot/wt && got commit -m 'edit' alpha >/dev/null)
> > +	ret=$?
> > +	if [ $ret -ne 0 ]; then
> > +		echo "got commit failed" >&2
> > +		test_done "$testroot" "$ret"
> > +		return 1
> > +	fi
> > +
> > +	(cd $testroot/wt && got log -l1 -p alpha) > $testroot/log
> > +	local bold="$(awk '/^blob \-/{print $3}' $testroot/log)"
> > +	local bnew="$(awk '/^blob \+/{print $3}' $testroot/log)"
> > +	sed -n	-e "s/^--- alpha/--- $bold/" \
> > +		-e "s/^\+\+\+ alpha/+++ $bnew/" \
> > +		-e '/^blob -/,$p' \
> > +		$testroot/log > $testroot/diff.expected
> 
> Instead of the above, could this new test code fetch blob IDs with
> the get_blob_id() helper, before and after creating the commit?

yeah, we can do better.  Here's another attemp less hacky with
get_blob_id

diff /home/op/w/got
commit - e4556f5ad27817d84c98b820f3e952b57c4e926c
path + /home/op/w/got
blob - 136f659b08eaf6408f6c79e05f6440fe7c754523
file + regress/cmdline/diff.sh
--- regress/cmdline/diff.sh
+++ regress/cmdline/diff.sh
@@ -19,6 +19,7 @@
 test_diff_basic() {
 	local testroot=`test_init diff_basic`
 	local head_rev=`git_show_head $testroot/repo`
+	local alpha_blobid=`get_blob_id $testroot/repo "" alpha`
 
 	got checkout $testroot/repo $testroot/wt > /dev/null
 	ret=$?
@@ -385,6 +386,37 @@ test_diff_basic() {
 		diff -u $testroot/stderr.expected $testroot/stderr
 		return 1
 	fi
+
+	# diff two blob ids
+	(cd $testroot/wt && got commit -m 'edit' alpha >/dev/null)
+	local alpha_new_blobid=`get_blob_id $testroot/repo "" alpha`
+	(cd $testroot/wt && got diff $alpha_blobid $alpha_new_blobid) > $testroot/diff
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "diff failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cat <<EOF >$testroot/diff.expected
+blob - $alpha_blobid
+blob + $alpha_new_blobid
+--- $alpha_blobid
++++ $alpha_new_blobid
+@@ -1 +1 @@
+-alpha
++modified alpha
+EOF
+
+	cmp -s $testroot/diff.expected $testroot/diff
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo
+		diff -u $testroot/diff.expected $testroot/diff
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
 	test_done "$testroot" "$ret"
 }