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

[PATCH v2] t1309: use a neutral branch name in the `onbranch` test cases

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Nov 19, 2020, 11:41 UTC
Message-ID
<pull.791.v2.git.1605786094533.gitgitgadget@gmail.com>
In-Reply-To
<pull.791.git.1605709410465.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `onbranch` test cases touched by this patch do not actually try to include any other config. Their purpose is to avoid regressing on two bugs in the `include.onbranch:<name>.path` code that we fixed in the past, bugs that are actually unrelated to any concrete branch name.

The first bug was fixed in 85fe0e800ca (config: work around bug with includeif:onbranch and early config, 2019-07-31). Essentially, when reading early config, there would be a catch-22 trying to access the refs, and therefore we simply cannot evaluate the condition at that point. The test case ensures that we avoid emitting this bogus message:

	BUG: refs.c:1851: attempting to get main_ref_store outside of repository

The second test case concerns the non-Git scenario, where we simply do not have a current branch to begin with (because we don't have a repository in the first place), and the test case was introduced in 22932d9169f (config: stop checking whether the_repository is NULL, 2019-08-06) to ensure that we don't cause a segmentation fault should the code still incorrectly try to look at any ref.

In short, neither of these two test cases will ever look at a current branch name, even in case of regressions. Therefore, the actual branch name does not matter at all. We can therefore easily avoid racially-charged branch names here, and that's what this patch does.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    t1309: use a non-loaded branch name in the onbranch test cases
    
    Just something I stumbled over while working on 
    https://github.com/gitgitgadget/git/pull/762.
    
    Changes since v1:
    
     * The commit message was obviously not clear at all, which has been
       addressed.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-791%2Fdscho%2Ft1309-onbranch-tests-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-791/dscho/t1309-onbranch-tests-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/791
Range-diff vs v1:
 1:  c78ddaf9ad ! 1:  7897650556 t1309: use a non-loaded branch name in the `onbranch` test cases
     @@ Metadata
      Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
      
       ## Commit message ##
     -    t1309: use a non-loaded branch name in the `onbranch` test cases
     +    t1309: use a neutral branch name in the `onbranch` test cases
      
     -    The `onbranch` test cases in question do not actually want to include
     -    anything; Instead, they want to verify that the `onbranch` code path
     -    does not regress in the early-config case or in the non-Git case, where
     -    the `onbranch` include is actually ignored.
     +    The `onbranch` test cases touched by this patch do not actually try to
     +    include any other config. Their purpose is to avoid regressing on two
     +    bugs in the `include.onbranch:<name>.path` code that we fixed in the
     +    past, bugs that are actually unrelated to any concrete branch name.
      
     -    Therefore, the actual branch name does not matter at all. We might just
     -    as well avoid racially-charged names here.
     +    The first bug was fixed in 85fe0e800ca (config: work around bug with
     +    includeif:onbranch and early config, 2019-07-31). Essentially, when
     +    reading early config, there would be a catch-22 trying to access the
     +    refs, and therefore we simply cannot evaluate the condition at that
     +    point. The test case ensures that we avoid emitting this bogus message:
     +
     +            BUG: refs.c:1851: attempting to get main_ref_store outside of repository
     +
     +    The second test case concerns the non-Git scenario, where we simply do
     +    not have a current branch to begin with (because we don't have a
     +    repository in the first place), and the test case was introduced in
     +    22932d9169f (config: stop checking whether the_repository is NULL,
     +    2019-08-06) to ensure that we don't cause a segmentation fault should
     +    the code still incorrectly try to look at any ref.
     +
     +    In short, neither of these two test cases will ever look at a current
     +    branch name, even in case of regressions. Therefore, the actual branch
     +    name does not matter at all. We can therefore easily avoid
     +    racially-charged branch names here, and that's what this patch does.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
 t/t1309-early-config.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh
index ebb8e1aecb..b4a9158307 100755
--- a/t/t1309-early-config.sh
+++ b/t/t1309-early-config.sh
@@ -91,11 +91,11 @@ test_expect_failure 'ignore .git/ with invalid config' '
 
 test_expect_success 'early config and onbranch' '
 	echo "[broken" >broken &&
-	test_with_config "[includeif \"onbranch:master\"]path=../broken"
+	test_with_config "[includeif \"onbranch:topic\"]path=../broken"
 '
 
 test_expect_success 'onbranch config outside of git repo' '
-	test_config_global includeIf.onbranch:master.path non-existent &&
+	test_config_global includeIf.onbranch:topic.path non-existent &&
 	nongit git help
 '
 

base-commit: e31aba42fb12bdeb0f850829e008e1e3f43af500
-- 
gitgitgadget
Previous: Johannes Schindelin
Message 6 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.