On Thu, Oct 21, 2021 at 10:26 AM Jeff Hostetler via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> Create 2x2 test matrix with the untracked-cache and fsmonitor--daemon
> features and a series of edits and verify that status output is
> identical.
>
> Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> ---
> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
> @@ -508,4 +510,98 @@ test_expect_success 'cleanup worktrees' '
> +test_lazy_prereq UNTRACKED_CACHE '
> + { git update-index --test-untracked-cache; ret=$?; } &&
> + test $ret -ne 1
> +'
I may be missing something obvious, but can't this be expressed more simply as:
test_lazy_prereq UNTRACKED_CACHE '
git update-index --test-untracked-cache
test $? -ne 1
'
How significant is it to test specifically against 1? If not, then
even simpler would be:
test_lazy_prereq UNTRACKED_CACHE '
git update-index --test-untracked-cache
'
which is also more robust since it won't be fooled by die() or crashes.
> +test_expect_success 'Matrix: setup for untracked-cache,fsmonitor matrix' '
> + test_might_fail git config --unset core.useBuiltinFSMonitor &&
More idiomatic:
test_unconfig core.useBuiltinFSMonitor &&
> + git update-index --no-fsmonitor &&
> + test_might_fail git fsmonitor--daemon stop
> +'
> +
> +matrix_clean_up_repo () {
> + git reset --hard HEAD
> + git clean -fd
> +}
Since calls to this function are part of the &&-chain in tests, it
probably would be a good idea to maintain the &&-chain within the body
of the function, as well.
> +matrix_try () {
> + test_expect_success "Matrix[uc:$uc][fsm:$fsm] $fn" '
> + matrix_clean_up_repo &&
> + $fn &&
> + if test $uc = false -a $fsm = false
We avoid -a and -o with `test` and instead chain them with &&:
if test $uc = false && test $fsm = false
Documentation/CodingGuidelines mentions this. Also see [1] & [2].
[1]: https://lore.kernel.org/git/xmqqa6qkb5fi.fsf@gitster.g/
[2]: https://lore.kernel.org/git/CAPig+cQFFsLeE921WpzTxVnBMnNRiKs4N=hUQ2UQi1VznNEQwg@xxxxxxxxxxxxxx/
> + then
> + git status --porcelain=v1 >.git/expect.$fn
> + else
> + git status --porcelain=v1 >.git/actual.$fn
> + test_cmp .git/expect.$fn .git/actual.$fn
> + fi
> + '
Broken &&-chain in the `else` arm.
> + return $?
> +}
No callers care about the return value of this function, so the
`return $?` can be dropped.
> +uc_values="false"
> +test_have_prereq UNTRACKED_CACHE && uc_values="false true"
> +for uc_val in $uc_values
> +do
> + if test $uc_val = false
> + then
> + test_expect_success "Matrix[uc:$uc_val] disable untracked cache" '
> + git config core.untrackedcache false &&
> + git update-index --no-untracked-cache
> + '
> + else
> + test_expect_success "Matrix[uc:$uc_val] enable untracked cache" '
> + git config core.untrackedcache true &&
> + git update-index --untracked-cache
> + '
> + fi
> +
> + fsm_values="false true"
> + for fsm_val in $fsm_values
> + do
> + if test $fsm_val = false
> + then
> + test_expect_success "Matrix[uc:$uc_val][fsm:$fsm_val] disable fsmonitor" '
> + test_might_fail git config --unset core.useBuiltinFSMonitor &&
Ditto: test_unconfig()
> + git update-index --no-fsmonitor &&
> + test_might_fail git fsmonitor--daemon stop 2>/dev/null
> + '
stderr is redirected within tests anyhow, so we normally don't
suppress it manually like this (especially since it may come in handy
when debugging a failing test).
> + else
> + test_expect_success "Matrix[uc:$uc_val][fsm:$fsm_val] enable fsmonitor" '
> + git config core.useBuiltinFSMonitor true &&
> + git fsmonitor--daemon start &&
> + git update-index --fsmonitor
> + '
> + fi
> +
> + matrix_try $uc_val $fsm_val edit_files
> + matrix_try $uc_val $fsm_val delete_files
> + matrix_try $uc_val $fsm_val create_files
> + matrix_try $uc_val $fsm_val rename_files
> + matrix_try $uc_val $fsm_val file_to_directory
> + matrix_try $uc_val $fsm_val directory_to_file
> +
> + if test $fsm_val = true
> + then
> + test_expect_success "Matrix[uc:$uc_val][fsm:$fsm_val] disable fsmonitor at end" '
> + test_might_fail git config --unset core.useBuiltinFSMonitor &&
Ditto: test_unconfig()
> + git update-index --no-fsmonitor &&
> + test_might_fail git fsmonitor--daemon stop 2>/dev/null
Ditto: stderr
> + '
> + fi
> + done
> +done
