Re: [PATCH v2 08/11] commit-graph: add --split option to builtin

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Wed, May 22, 2019 at 12:53:27PM -0700, Derrick Stolee via GitGitGadget wrote:
> diff --git a/t/t5323-split-commit-graph.sh b/t/t5323-split-commit-graph.sh
> new file mode 100755
> index 0000000000..96704b9f5b
> --- /dev/null
> +++ b/t/t5323-split-commit-graph.sh
> @@ -0,0 +1,122 @@
> +#!/bin/sh
> +
> +test_description='split commit graph'
> +. ./test-lib.sh
> +
> +GIT_TEST_COMMIT_GRAPH=0
> +
> +test_expect_success 'setup repo' '
> +	git init &&
> +	git config core.commitGraph true &&
> +	infodir=".git/objects/info" &&
> +	graphdir="$infodir/commit-graphs" &&
> +	test_oid_init
> +'
> +
> +graph_read_expect() {
> +	NUM_BASE=0
> +	if test ! -z $2
> +	then
> +		NUM_BASE=$2
> +	fi
> +	cat >expect <<- EOF
> +	header: 43475048 1 1 3 $NUM_BASE
> +	num_commits: $1
> +	chunks: oid_fanout oid_lookup commit_metadata
> +	EOF
> +	git commit-graph read >output &&
> +	test_cmp expect output
> +}
> +
> +test_expect_success 'create commits and write commit-graph' '
> +	for i in $(test_seq 3)
> +	do
> +		test_commit $i &&
> +		git branch commits/$i
> +	done &&

Please add a "|| return 1" at the end of the for loop's body, i.e.

  for ....
  do
        this &&
        that || return 1
  done

because for loops continue iteration even when the commands in their
body failed, potentially hinding errors.

This applies to the other three for loops below as well.

> +	git commit-graph write --reachable &&
> +	test_path_is_file $infodir/commit-graph &&
> +	graph_read_expect 3
> +'
> +
> +graph_git_two_modes() {
> +	git -c core.commitGraph=true $1 >output
> +	git -c core.commitGraph=false $1 >expect
> +	test_cmp expect output
> +}
> +
> +graph_git_behavior() {
> +	MSG=$1
> +	BRANCH=$2
> +	COMPARE=$3
> +	test_expect_success "check normal git operations: $MSG" '
> +		graph_git_two_modes "log --oneline $BRANCH" &&
> +		graph_git_two_modes "log --topo-order $BRANCH" &&
> +		graph_git_two_modes "log --graph $COMPARE..$BRANCH" &&
> +		graph_git_two_modes "branch -vv" &&
> +		graph_git_two_modes "merge-base -a $BRANCH $COMPARE"
> +	'
> +}
> +
> +graph_git_behavior 'graph exists' commits/3 commits/1
> +
> +verify_chain_files_exist() {
> +	for hash in $(cat $1/commit-graph-chain)
> +	do
> +		test_path_is_file $1/graph-$hash.graph
> +	done
> +}
> +
> +test_expect_success 'add more commits, and write a new base graph' '
> +	git reset --hard commits/1 &&
> +	for i in $(test_seq 4 5)
> +	do
> +		test_commit $i &&
> +		git branch commits/$i
> +	done &&
> +	git reset --hard commits/2 &&
> +	for i in $(test_seq 6 10)
> +	do
> +		test_commit $i &&
> +		git branch commits/$i
> +	done &&
> +	git reset --hard commits/2 &&
> +	git merge commits/4 &&
> +	git branch merge/1 &&
> +	git reset --hard commits/4 &&
> +	git merge commits/6 &&
> +	git branch merge/2 &&
> +	git commit-graph write --reachable &&
> +	graph_read_expect 12
> +'
> +
> +test_expect_success 'add three more commits, write a tip graph' '
> +	git reset --hard commits/3 &&
> +	git merge merge/1 &&
> +	git merge commits/5 &&
> +	git merge merge/2 &&
> +	git branch merge/3 &&
> +	git commit-graph write --reachable --split &&
> +	test_path_is_missing $infodir/commit-graph &&
> +	test_path_is_file $graphdir/commit-graph-chain &&
> +	ls $graphdir/graph-*.graph >graph-files &&
> +	test_line_count = 2 graph-files &&
> +	verify_chain_files_exist $graphdir
> +'
> +
> +graph_git_behavior 'split commit-graph: merge 3 vs 2' merge/3 merge/2
> +
> +test_expect_success 'add one commit, write a tip graph' '
> +	test_commit 11 &&
> +	git branch commits/11 &&
> +	git commit-graph write --reachable --split &&
> +	test_path_is_missing $infodir/commit-graph &&
> +	test_path_is_file $graphdir/commit-graph-chain &&
> +	ls $graphdir/graph-*.graph >graph-files &&
> +	test_line_count = 3 graph-files &&
> +	verify_chain_files_exist $graphdir
> +'
> +
> +graph_git_behavior 'three-layer commit-graph: commit 11 vs 6' commits/11 commits/6
> +
> +test_done
> -- 
> gitgitgadget
>