| Message ID | 507020bbef08a02afc53815754cc999c390eb7c7.1634826309.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Builtin FSMonitor Part 2 | expand |
On Thu, Oct 21, 2021 at 10:26 AM Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com> 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@microsoft.com> > --- > 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@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
On 10/22/21 1:23 AM, Eric Sunshine wrote: > On Thu, Oct 21, 2021 at 10:26 AM Jeff Hostetler via GitGitGadget > <gitgitgadget@gmail.com> 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@microsoft.com> >> --- >> 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
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh index eea9ca1a309..e62ec9aa3ca 100755 --- a/t/t7527-builtin-fsmonitor.sh +++ b/t/t7527-builtin-fsmonitor.sh @@ -162,6 +162,8 @@ test_expect_success 'setup' ' .gitignore expect* actual* + flush* + trace* EOF git -c core.useBuiltinFSMonitor= add . && @@ -508,4 +510,98 @@ test_expect_success 'cleanup worktrees' ' stop_daemon_delete_repo wt-base ' +# The next few tests perform arbitrary/contrived file operations and +# confirm that status is correct. That is, that the data (or lack of +# data) from fsmonitor doesn't cause incorrect results. And doesn't +# cause incorrect results when the untracked-cache is enabled. + +test_lazy_prereq UNTRACKED_CACHE ' + { git update-index --test-untracked-cache; ret=$?; } && + test $ret -ne 1 +' + +test_expect_success 'Matrix: setup for untracked-cache,fsmonitor matrix' ' + test_might_fail git config --unset 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 +} + +matrix_try () { + uc=$1 + fsm=$2 + fn=$3 + + test_expect_success "Matrix[uc:$uc][fsm:$fsm] $fn" ' + matrix_clean_up_repo && + $fn && + if test $uc = false -a $fsm = false + 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 + ' + + return $? +} + +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 && + git update-index --no-fsmonitor && + test_might_fail git fsmonitor--daemon stop 2>/dev/null + ' + 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 && + git update-index --no-fsmonitor && + test_might_fail git fsmonitor--daemon stop 2>/dev/null + ' + fi + done +done + test_done