From: Jeff Hostetler Date: Wed, 27 Oct 2021 20:06:18 GMT Subject: Re: [PATCH v4 29/29] t7527: test status with untracked-cache and fsmonitor--daemon Message-ID: In-Reply-To: On 10/22/21 1:23 AM, Eric Sunshine wrote: > On Thu, Oct 21, 2021 at 10:26 AM Jeff Hostetler via GitGitGadget > 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 >> --- >> 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. > IIRC I stole that from t7063. It didn't occur to me to try to simplify it. >> +test_expect_success 'Matrix: setup for untracked-cache,fsmonitor matrix' ' >> + test_might_fail git config --unset core.useBuiltinFSMonitor && > > More idiomatic: > > test_unconfig core.useBuiltinFSMonitor && > Good to know, thanks! I'll take a look at this and the rest of your comments below. thanks Jeff >> + 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@mail.gmail.com/ > >> + 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