On Tue, Sep 21, 2021 at 06:47:17PM +0000, Lucas wrote:
> After a long troubleshooting on IRC, we found that histedit regression
> assumes that there will always be a fork after histedit. This is not
> the case if, for example, commits and histedit complete during the same
> second.
> 
> Inlined is a `got diff -w`, as the patch is wrapping a big chunk of code
> in an `if`. Attached is a normal patch.
> 
> -Lucas
> 
> diff 3da8ef855d7d7b3da96a9f1cba55df276e112f11 /home/lucas/code/git/git.gameoftrees.org/got
> blob - 8c612fafa7e7fd1f96dfce1527ee0b2f621e0f8c
> file + regress/cmdline/histedit.sh
> --- regress/cmdline/histedit.sh
> +++ regress/cmdline/histedit.sh
> @@ -150,6 +150,12 @@ test_histedit_no_op() {
>  		return 1
>  	fi
>  
> +	# It's possible for this no-op histedit to create commits with the same
> +	# hashes, for example if all commits and histedit run in the same UNIX
> +	# second.
> +	if [ "$old_commit1" != "$new_commit1" ] ||
> +	    [ "$old_commit2" != "$new_commit2" ]; then
> +
>  	# We should have a backup of old commits
>  	(cd $testroot/repo && got histedit -l > $testroot/stdout)
>  	d_orig2=`date -u -r $old_author_time2 +"%a %b %e %X %Y UTC"`

I don't think we should disable the entire histedit -l check.

What we should do is only expect the part that says "history forked at"
when the commits hashes do not happen to match.
But the rest of the output should still be verified.

So could we not create an appropriate stdout.expected file that matches
the expected result in either case?

> @@ -175,6 +181,7 @@ EOF
>  		test_done "$testroot" "$ret"
>  		return 1
>  	fi
> +	fi
>  
>  	(cd $testroot/repo && got histedit -X master \
>  		> $testroot/stdout 2> $testroot/stderr)
> 

> diff 3da8ef855d7d7b3da96a9f1cba55df276e112f11 /home/lucas/code/git/git.gameoftrees.org/got
> blob - 8c612fafa7e7fd1f96dfce1527ee0b2f621e0f8c
> file + regress/cmdline/histedit.sh
> --- regress/cmdline/histedit.sh
> +++ regress/cmdline/histedit.sh
> @@ -150,12 +150,18 @@ test_histedit_no_op() {
>  		return 1
>  	fi
>  
> -	# We should have a backup of old commits
> -	(cd $testroot/repo && got histedit -l > $testroot/stdout)
> -	d_orig2=`date -u -r $old_author_time2 +"%a %b %e %X %Y UTC"`
> -	d_new2=`date -u -r $new_author_time2 +"%G-%m-%d"`
> -	d_orig=`date -u -r $orig_author_time +"%G-%m-%d"`
> -	cat > $testroot/stdout.expected <<EOF
> +	# It's possible for this no-op histedit to create commits with the same
> +	# hashes, for example if all commits and histedit run in the same UNIX
> +	# second.
> +	if [ "$old_commit1" != "$new_commit1" ] ||
> +	    [ "$old_commit2" != "$new_commit2" ]; then
> +
> +		# We should have a backup of old commits
> +		(cd $testroot/repo && got histedit -l > $testroot/stdout)
> +		d_orig2=`date -u -r $old_author_time2 +"%a %b %e %X %Y UTC"`
> +		d_new2=`date -u -r $new_author_time2 +"%G-%m-%d"`
> +		d_orig=`date -u -r $orig_author_time +"%G-%m-%d"`
> +		cat > $testroot/stdout.expected <<EOF
>  -----------------------------------------------
>  commit $old_commit2 (formerly master)
>  from: $GOT_AUTHOR
> @@ -168,12 +174,13 @@ has become commit $new_commit2 (master)
>  history forked at $orig_commit
>   $d_orig $GOT_AUTHOR_11  adding the test tree
>  EOF
> -	cmp -s $testroot/stdout.expected $testroot/stdout
> -	ret="$?"
> -	if [ "$ret" != "0" ]; then
> -		diff -u $testroot/stdout.expected $testroot/stdout
> -		test_done "$testroot" "$ret"
> -		return 1
> +		cmp -s $testroot/stdout.expected $testroot/stdout
> +		ret="$?"
> +		if [ "$ret" != "0" ]; then
> +			diff -u $testroot/stdout.expected $testroot/stdout
> +			test_done "$testroot" "$ret"
> +			return 1
> +		fi
>  	fi
>  
>  	(cd $testroot/repo && got histedit -X master \