On Mon, Mar 23, 2020 at 10:06:00AM -0600, Tracey Emery wrote:
> Hello,
> 
> The following diff makes got tree work on the current branch and adds a
> couple of tests for this. Previously, got tree only worked on the HEAD
> ref.
> 
> Ok?

Some remarks on the test:

> blob - /dev/null
> file + regress/cmdline/tree.sh
> --- regress/cmdline/tree.sh
> +++ regress/cmdline/tree.sh
> @@ -0,0 +1,103 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2020 Stefan Sperling <stsp@openbsd.org>

Please put your own name there, not mine :) 

> +function test_tree_basic {
> +	local testroot=`test_init tree_basic`
> +
> +	got checkout $testroot/repo $testroot/wt > /dev/null
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	echo "new file" > $testroot/wt/foo
> +
> +	echo 'A  foo' > $testroot/stdout.expected
> +	(cd $testroot/wt && got add foo > $testroot/stdout)
> +
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		echo "got add failed unexpectedly" >&2
> +		test_done "$testroot" 1
> +		return 1
> +	fi

Checking whether 'checkout' and 'add' work is redundant. We already
have other tests for that. So please assume those commands will
work as expected, and send their standard output to /dev/null.
Any errors they print should be visible in case there is a problem.

> +	(cd $testroot/wt && got commit -m "add foo" foo >/dev/null)
> +
> +	echo 'alpha' > $testroot/stdout.expected
> +	echo 'beta' >> $testroot/stdout.expected
> +	echo 'epsilon/' >> $testroot/stdout.expected
> +	echo 'foo' >> $testroot/stdout.expected
> +	echo 'gamma/' >> $testroot/stdout.expected
> +	(cd $testroot/wt && got tree > $testroot/stdout)

I would add an empty line between the expected output and the command
being tested, for better readability.

> +
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +	fi
> +
> +	test_done "$testroot" "$ret"
> +}
> +
> +function test_tree_branch {
> +	local testroot=`test_init tree_branch`

The same apply to this test, too.

> +
> +	got checkout $testroot/repo $testroot/wt > /dev/null
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	(cd $testroot/wt && got br foo > $testroot/stdout)
> +	echo "new file" > $testroot/wt/foo
> +
> +	echo 'A  foo' > $testroot/stdout.expected
> +	(cd $testroot/wt && got add foo > $testroot/stdout)
> +
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		echo "got add failed unexpectedly" >&2
> +		test_done "$testroot" 1
> +		return 1
> +	fi
> +
> +	(cd $testroot/wt && got commit -m "add foo" foo >/dev/null)
> +
> +	echo 'alpha' > $testroot/stdout.expected
> +	echo 'beta' >> $testroot/stdout.expected
> +	echo 'epsilon/' >> $testroot/stdout.expected
> +	echo 'foo' >> $testroot/stdout.expected
> +	echo 'gamma/' >> $testroot/stdout.expected
> +	(cd $testroot/wt && got tree > $testroot/stdout)
> +
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +	fi
> +
> +	test_done "$testroot" "$ret"
> +}
> +
> +run_test test_tree_basic
> +run_test test_tree_branch
> 
>