On 2023.04.28 11:37, Glen Choo wrote:
> Josh Steadmon <steadmon@xxxxxxxxxx> writes:
>
> > diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
> > index 11c15a48aa..993f5bdc7d 100755
> > --- a/t/t0035-safe-bare-repository.sh
> > +++ b/t/t0035-safe-bare-repository.sh
> > @@ -7,13 +7,24 @@ TEST_PASSES_SANITIZE_LEAK=true
> >
> > pwd="$(pwd)"
> >
> > -expect_accepted () {
> > - git "$@" rev-parse --git-dir
> > +expect_accepted_implicit () {
> > + test_when_finished 'rm "$pwd/trace.perf"' &&
> > + GIT_TRACE2_PERF="$pwd/trace.perf" git "$@" rev-parse --git-dir &&
> > + grep -F "implicit-bare-repository:$pwd" "$pwd/trace.perf"
> > +}
> > +
> > +expect_accepted_explicit () {
> > + test_when_finished 'rm "$pwd/trace.perf"' &&
> > + GIT_DIR="$1" GIT_TRACE2_PERF="$pwd/trace.perf" git rev-parse --git-dir &&
> > + ! grep -F "implicit-bare-repository:$pwd" "$pwd/trace.perf"
> > }
>
> (Not new in v2) This wasn't obvious to me at first, but
>
> grep -F "implicit-bare-repository:$pwd" "$pwd/trace.perf"
>
> looks like it's asserting that we trace that the bare repository is
> $pwd, but it's actually only asserting that is starts with $pwd. If it
> might trip up other reasers, a comment here would be helpful.
Added a note in V3.
> > @@ -21,13 +32,9 @@ test_expect_success 'setup bare repo in worktree' '
> > git init --bare outer-repo/bare-repo
> > '
> >
> > -test_expect_success 'safe.bareRepository unset' '
> > - expect_accepted -C outer-repo/bare-repo
> > -'
> > -
>
> I found this surprising; I thought it was quite valuable to test the
> default behavior. I'd prefer to keep this test.
>
> Perhaps there was a miscommunication? You mentioned:
>
> Actually, explicitly setting the variable here is equivalent to the
> following test case, so I'll just remove this one.
>
> which is true, but I think Junio meant _un_setting the variable,
> something like:
>
> test_expect_success 'safe.bareRepository unset' '
> + test_unconfig --global safe.bareRepository &&
> expect_accepted -C outer-repo/bare-repo
> '
Yeah, misunderstood. However, see my reply to Junio about this being a
"change detector" test.
