git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] t1309: use a non-loaded branch name in the `onbranch` test cases

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Nov 19, 2020, 10:49 UTC
Message-ID
<nycvar.QRO.7.76.6.2011191147490.56@tvgsbejvaqbjf.bet>
In-Reply-To
<874kllre8f.fsf@evledraar.gmail.com>
Hi Ævar,
On Thu, 19 Nov 2020, Ævar Arnfjörð Bjarmason wrote:
Show 50 quoted lines
> On Thu, Nov 19 2020, Johannes Schindelin wrote:
>
> > On Wed, 18 Nov 2020, Ævar Arnfjörð Bjarmason wrote:
> >
> >> On Wed, Nov 18 2020, Johannes Schindelin via GitGitGadget wrote:
> >>
> >> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >> >
> >> > Therefore, the actual branch name does not matter at all. We might
> >> > just as well avoid racially-charged names here.
> >>
> >> It seems to me the actual name matters a lot, and it must whatever
> >> the default branch name is.
> >
> > Nope. Not at all. Because what we're exercising is the code paths when
> > we _don't_ have a branch name to work with.
> >
> > In the non-Git case, this is trivial to see. There is not even a
> > repository! How can there be a branch?
> >
> > In the early config case, it is too early to access the refs. I meant
> > to reference (but forgot) the commit 85fe0e800ca (config: work around
> > bug with includeif:onbranch and early config, 2019-07-31) because that
> > commit's commit message describes the catch-22 that is the reason why
> > the early config cannot see the current branch name (if any).
> >
> > I should probably have thought of referencing 22932d9169f (config:
> > stop checking whether the_repository is NULL, 2019-08-06) for the
> > second test case, too.
> >
> > So again, these two test cases do _not_ exercise the code path where
> > another config file is included. To the contrary, they try to prevent
> > a regression where `onbranch` would segfault in one case, and BUG in
> > the other (in both cases because the now-fixed code used to try to
> > look at the current branch name _anyway_).
> >
> >> I.e. what the test is doing is producing intentionally broken config,
> >> and asserting that we don't read it at an early stage.
> >>
> >> Therefore if we regressed and started doing that the test wouldn't
> >> catch it, because the default branch name is "master", or "main"
> >> if/when that refs.c change lands, neither of which is "topic".
> >
> > No, if we regressed, the code would start to throw a BUG, or a
> > segfault, respectively.
> >
> > We never expect these two test cases to look at any branch name at
> > all.
>
> Thanks. I mis(understood|read) it.
I guess the commit message could use an update.

Ciao, Dscho

Previous: Ævar Arnfjörð BjarmasonNext: Johannes Schindelin via GitGitGadget
Message 5 of 6 in “t1309: use a non-loaded branch name in the `onbranch` test cases”
  1. t1309: use a non-loaded branch name in the `onbranch` test casesJohannes Schindelin via GitGitGadget, Nov 18, 2020
  2. Ævar Arnfjörð BjarmasonNov 18, 2020
  3. Johannes SchindelinNov 19, 2020
  4. Ævar Arnfjörð BjarmasonNov 19, 2020
  5. Johannes SchindelinNov 19, 2020
  6. t1309: use a neutral branch name in the `onbranch` test casesJohannes Schindelin via GitGitGadget, Nov 19, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.