Skip to content

Conversation

@dscho
Copy link
Member

@dscho dscho commented Jan 16, 2026

I leaned heavily on AI to implement this patch, in particular when analyzing the logs. That's why I added that trailer talking about Claude Sonnet. If this is undesirable, please let me know.

cc: Phillip Wood phillip.wood123@gmail.com
cc: "Kristoffer Haugsbakk" kristofferhaugsbakk@fastmail.com

I noticed recently that the leak-checking jobs still take a lot of time,
and upon analysis, the git-svn tests contribute significantly to this.

Analyzing a recent CI run, I saw that the Git test suite contains
1,017 tests, running for approximately 5¼ hours total. Of these, 65
git-svn-related tests (~6% of test count) took 42.24 minutes combined,
accounting for ~13.% of the total runtime. This implies that the git-svn
tests are roughly twice as expernsive compared to the other tests.

However, testing git-svn in the leak-checking jobs provides minimal
value: git-svn is implemented as a Perl script, and leak checking only
handles C code. While git-svn does call into Git's built-in commands
that are implemented in C, these are standard Git operations that are
already thoroughly exercised elsewhere in the test suite. Therefore,
running the git-svn tests in the leak-checking jobs only adds to the
overall run time with little value in return.

Given that the leak-checking jobs are particularly time-intensive and
these 42+ minutes of SVN tests per job provide no additional leak
detection value, skip them in the *-leaks jobs to reduce CI runtime.

Assisted-by: Claude Sonnet 4.5
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho self-assigned this Jan 16, 2026
@dscho
Copy link
Member Author

dscho commented Jan 16, 2026

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 16, 2026

Submitted as pull.2031.git.1768584676520.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2031/dscho/skip-svn-and-leak-tests-v1

To fetch this version to local tag pr-2031/dscho/skip-svn-and-leak-tests-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2031/dscho/skip-svn-and-leak-tests-v1

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 16, 2026

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> However, testing git-svn in the leak-checking jobs provides minimal
> value: git-svn is implemented as a Perl script, and leak checking only
> handles C code. While git-svn does call into Git's built-in commands
> that are implemented in C, these are standard Git operations that are
> already thoroughly exercised elsewhere in the test suite. Therefore,
> running the git-svn tests in the leak-checking jobs only adds to the
> overall run time with little value in return.

Very nicely reasoned.  And the implementation of this idea is ...

> diff --git a/ci/lib.sh b/ci/lib.sh
> index f561884d40..a165c7f268 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -356,6 +356,7 @@ linux-musl-meson)
>  	;;
>  linux-leaks|linux-reftable-leaks)
>  	export SANITIZE=leak
> +	export NO_SVN_TESTS=LetsSaveSomeTime
>  	;;

... surprisingly simple.  I very much like it.

Thanks.  Will queue.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2026

Phillip Wood wrote on the Git mailing list (how to reply to this email):

Hi Johannes

On 16/01/2026 17:31, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > I noticed recently that the leak-checking jobs still take a lot of time,
> and upon analysis, the git-svn tests contribute significantly to this.
> > Analyzing a recent CI run, I saw that the Git test suite contains
> 1,017 tests, running for approximately 5¼ hours total. Of these, 65
> git-svn-related tests (~6% of test count) took 42.24 minutes combined,
> accounting for ~13.% of the total runtime. This implies that the git-svn
> tests are roughly twice as expernsive compared to the other tests.

Looking at the CI logs for this PR the p4 and cvs tests account for another 24 minutes of test time and I suspect they also offer little in the way of extra coverage. Unfortunately there is no equivalent of NO_SVN_TESTS to disable them - I wonder if building with NO_PYTHON and NO_PERL would make sense for the leak test job?

Either way I like the direction of this patch

Thanks

Phillip

> However, testing git-svn in the leak-checking jobs provides minimal
> value: git-svn is implemented as a Perl script, and leak checking only
> handles C code. While git-svn does call into Git's built-in commands
> that are implemented in C, these are standard Git operations that are
> already thoroughly exercised elsewhere in the test suite. Therefore,
> running the git-svn tests in the leak-checking jobs only adds to the
> overall run time with little value in return.
> > Given that the leak-checking jobs are particularly time-intensive and
> these 42+ minutes of SVN tests per job provide no additional leak
> detection value, skip them in the *-leaks jobs to reduce CI runtime.
> > Assisted-by: Claude Sonnet 4.5
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>      ci(*-leaks): skip the git-svn tests to save time
>      >      I leaned heavily on AI to implement this patch, in particular when
>      analyzing the logs. That's why I added that trailer talking about Claude
>      Sonnet. If this is undesirable, please let me know.
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2031%2Fdscho%2Fskip-svn-and-leak-tests-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2031/dscho/skip-svn-and-leak-tests-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2031
> >   ci/lib.sh | 1 +
>   1 file changed, 1 insertion(+)
> > diff --git a/ci/lib.sh b/ci/lib.sh
> index f561884d40..a165c7f268 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -356,6 +356,7 @@ linux-musl-meson)
>   	;;
>   linux-leaks|linux-reftable-leaks)
>   	export SANITIZE=leak
> +	export NO_SVN_TESTS=LetsSaveSomeTime
>   	;;
>   linux-asan-ubsan)
>   	export SANITIZE=address,undefined
> > base-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2026

User Phillip Wood <phillip.wood123@gmail.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2026

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Phillip Wood <phillip.wood123@gmail.com> writes:

> Looking at the CI logs for this PR the p4 and cvs tests account for 
> another 24 minutes of test time and I suspect they also offer little in 
> the way of extra coverage. Unfortunately there is no equivalent of 
> NO_SVN_TESTS to disable them - I wonder if building with NO_PYTHON and 
> NO_PERL would make sense for the leak test job?
>
> Either way I like the direction of this patch
>
> Thanks
>
> Phillip

Yup, I generally like this direction, and introducing NO_P4_TESTS
and NO_CVS_TESTS would not be so bad.  Here is how it looks on top
of Dscho's patch.

--- >8 ---
Subject: [PATCH] ci: skip CVS and P4 tests in leaks job, too

Looking at the CI logs, the p4 and cvs tests account for another 24
minutes of test time and they offer minimal value for quite a
similar reason as the previous step.

Let's introduce and use a mechanism to skip these tests to save
some resources.

Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ci/lib.sh       | 2 ++
 t/lib-cvs.sh    | 6 ++++++
 t/lib-git-p4.sh | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/ci/lib.sh b/ci/lib.sh
index a165c7f268..3ecbf147db 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -356,7 +356,9 @@ linux-musl-meson)
 	;;
 linux-leaks|linux-reftable-leaks)
 	export SANITIZE=leak
+	export NO_CVS_TESTS=LetsSaveSomeTime
 	export NO_SVN_TESTS=LetsSaveSomeTime
+	export NO_P4_TESTS=LetsSaveSomeTime
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index 57b9b2db9b..c8b4404888 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -2,6 +2,12 @@
 
 . ./test-lib.sh
 
+if test -n "$NO_CVS_TESTS"
+then
+	skip_all='skipping git cvs tests, NO_CVS_TESTS defined'
+	test_done
+fi
+
 unset CVS_SERVER
 
 if ! type cvs >/dev/null 2>&1
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 2a5b8738ea..d22e9c684a 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -16,6 +16,11 @@ P4D_TIMEOUT=300
 
 . ./test-lib.sh
 
+if test -n "$NO_P4_TESTS"
+then
+	skip_all='skipping git p4 tests, NO_P4_TESTS defined'
+	test_done
+fi
 if ! test_have_prereq PYTHON
 then
 	skip_all='skipping git p4 tests; python not available'
-- 
2.53.0-rc0-217-gd590ba4684

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2026

"Kristoffer Haugsbakk" wrote on the Git mailing list (how to reply to this email):

On Sat, Jan 17, 2026, at 19:34, Junio C Hamano wrote:
>>[snip]
> Yup, I generally like this direction, and introducing NO_P4_TESTS
> and NO_CVS_TESTS would not be so bad.  Here is how it looks on top
> of Dscho's patch.
>
> --- >8 ---
> Subject: [PATCH] ci: skip CVS and P4 tests in leaks job, too
>
> Looking at the CI logs, the p4 and cvs tests account for another 24
> minutes of test time and they offer minimal value for quite a
> similar reason as the previous step.
>
> Let's introduce and use a mechanism to skip these tests to save
> some resources.
>
> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>

Nitpick: Using the commit ident

    Phillip Wood <phillip.wood@dunelm.org.uk>

might be slightly better?

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>[snip]

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2026

User "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2026

This patch series was integrated into seen via git@dde09b8.

@gitgitgadget gitgitgadget bot added the seen label Jan 17, 2026
@gitgitgadget
Copy link

gitgitgadget bot commented Jan 18, 2026

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

> On Sat, Jan 17, 2026, at 19:34, Junio C Hamano wrote:
>>>[snip]
>> Yup, I generally like this direction, and introducing NO_P4_TESTS
>> and NO_CVS_TESTS would not be so bad.  Here is how it looks on top
>> of Dscho's patch.
>>
>> --- >8 ---
>> Subject: [PATCH] ci: skip CVS and P4 tests in leaks job, too
>>
>> Looking at the CI logs, the p4 and cvs tests account for another 24
>> minutes of test time and they offer minimal value for quite a
>> similar reason as the previous step.
>>
>> Let's introduce and use a mechanism to skip these tests to save
>> some resources.
>>
>> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
>
> Nitpick: Using the commit ident
>
>     Phillip Wood <phillip.wood@dunelm.org.uk>
>
> might be slightly better?

I didn't even realize there are multiple addresses in play,
actually.  I just took it from the e-mail header's Cc: field,
which my MUA copied from From: field of the message I was responding
to, which was the identity of the person who suggested the change
after all ;-).

So, I dunno.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant