"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> When testing the sparse-checkout feature, we need to compare the
> contents of the working-directory against some expected output.
> Using here-docs was useful in the beginning, but became repetetive
> as the test script grew.
>
> Create a check_files helper to make the tests simpler and easier
> to extend. It also reduces instances of bad here-doc whitespace.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> t/t1091-sparse-checkout-builtin.sh | 215 ++++++++++-------------------
> 1 file changed, 71 insertions(+), 144 deletions(-)
>
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index ff7f8f7a1f..20caefe155 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -12,6 +12,13 @@ list_files() {
> (cd "$1" && printf '%s\n' *)
> }
>
> +check_files() {
> + DIR=$1
> + printf "%s\n" $2 >expect &&
> + list_files $DIR >actual &&
It is unclear if the script is being deliberate or sloppy.
It turns out that not quoting $2 is deliberate (i.e. it wants to
pass more than one words in $2, have them split at $IFS and show
each of them on a separate line), at the same time not quoting $DIR
is simply sloppy.
And it is totally unnecessary to confuse readers like this.
Unless you plan to extend this helper further, I think this would be
much less burdensome to the readers:
check_files () {
list_files "$1" >actual &&
shift &&
printf "%s\n" "$@" >expect &&
test_cmp expect actual
}
This ...
> test_cmp expect repo/.git/info/sparse-checkout &&
> - list_files repo >dir &&
> - cat >expect <<-EOF &&
> - a
> - folder1
> - folder2
> - EOF
> - test_cmp expect dir
> + check_files repo "a folder1 folder2"
... is a kind of change that the log message advertises, which is a
very nice rewrite.
And ...
> test_expect_success 'clone --sparse' '
> git clone --sparse repo clone &&
> git -C clone sparse-checkout list >actual &&
> - cat >expect <<-EOF &&
> - /*
> - !/*/
> + cat >expect <<-\EOF &&
> + /*
> + !/*/
> EOF
... this is a style-fix that is another nice rewrite but in a
different category. I wonder if they should be done in separate
commits.
Other than that, makes sense.
Thanks.
