Re: [PATCH v2 16/19] parallel-checkout: add tests for basic operations

>
> Hi,
>
> Matheus Tavares wrote:
>
> > Add tests to populate the working tree during clone and checkout using
> > the sequential and parallel modes, to confirm that they produce
> > identical results. Also test basic checkout mechanics, such as checking
> > for symlinks in the leading directories and the abidance to --force.
>
> Thanks for implementing parallel checkout!  I'm excited about the
> feature.  And thanks for including these tests.

> [...]
> > --- /dev/null
> > +++ b/t/lib-parallel-checkout.sh
> > @@ -0,0 +1,39 @@
> [...]
> > +# Runs `git -c checkout.workers=$1 -c checkout.thesholdForParallelism=$2 ${@:4}`
> > +# and checks that the number of workers spawned is equal to $3.
> > +git_pc()
>
> nit: what does git_pc mean?

> [...]
> > +     GIT_TRACE2="$(pwd)/trace" git \
> > +             -c checkout.workers=$workers \
> > +             -c checkout.thresholdForParallelism=$threshold \
> > +             -c advice.detachedHead=0 \
> > +             $@ &&
>
> $@ needs to be quoted, or else it will act like $* (and in particular it
> won't handle parameters with embedded spaces).

> > +
> > +     # Check that the expected number of workers has been used. Note that it
> > +     # can be different than the requested number in two cases: when the
> > +     # quantity of entries to be checked out is less than the number of
> > +     # workers; and when the threshold has not been reached.
> > +     #
> > +     local workers_in_trace=$(grep "child_start\[.\+\] git checkout--helper" trace | wc -l) &&
>
> Do we use grep's \+ operator in other tests?  I thought we preferred to
> use the more portable *, but it may be that I'm out of date.

> [...]
> > +# Verify that both the working tree and the index were created correctly
> > +verify_checkout()
> > +{
> > +     git -C $1 diff-index --quiet HEAD -- &&
> > +     git -C $1 diff-index --quiet --cached HEAD -- &&
> > +     git -C $1 status --porcelain >$1.status &&
> > +     test_must_be_empty $1.status
> > +}
>
> Like git_pc, this is not easy to take in at a glance.
>
> "$1" needs to be quoted if we are to handle paths with spaces.

> [...]
> > --- /dev/null
> > +++ b/t/t2080-parallel-checkout-basics.sh
> > @@ -0,0 +1,197 @@
> > +#!/bin/sh
> > +
> > +test_description='parallel-checkout basics
> > +
> > +Ensure that parallel-checkout basically works on clone and checkout, spawning
> > +the required number of workers and correctly populating both the index and
> > +working tree.
> > +'
> > +
> > +TEST_NO_CREATE_REPO=1
> > +. ./test-lib.sh
> > +. "$TEST_DIRECTORY/lib-parallel-checkout.sh"
> > +
> > +# NEEDSWORK: cloning a SHA1 repo with GIT_TEST_DEFAULT_HASH set to "sha256"
> > +# currently produces a wrong result (See
> > +# https://lore.kernel.org/git/20200911151717.43475-1-matheus.bernardino@usp.br/).
> > +# So we skip the "parallel-checkout during clone" tests when this test flag is
> > +# set to "sha256". Remove this when the bug is fixed.
> > +#
> > +if test "$GIT_TEST_DEFAULT_HASH" = "sha256"
> > +then
> > +     skip_all="t2080 currently don't work with GIT_TEST_DEFAULT_HASH=sha256"
> > +     test_done
> > +fi
> > +
> > +R_BASE=$GIT_BUILD_DIR
> > +
> > +test_expect_success 'sequential clone' '
> > +     git_pc 1 0 0 clone --quiet -- $R_BASE r_sequential &&
>
> This fails when I run it when building from a tarball, which is
> presenting me from releasing this patch series to Debian experimental.