Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage

From: Duy Nguyen <pclouds@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: gitgitgadget@gmail.com, Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 0/1] Define GIT_TEST_COMMIT_GRAPH for commit-graph test coverage
Date: Tue, 4 Sep 2018 19:18:01 +0200	[thread overview]
Message-ID: <20180904171801.GA31791@duynguyen.home> (raw)
In-Reply-To: <faebdf82-7e9b-b662-2884-1398d90ab26c@gmail.com>

On Tue, Sep 04, 2018 at 01:12:55PM -0400, Derrick Stolee wrote:
> On 9/4/2018 12:49 PM, Duy Nguyen wrote:
> > On Wed, Aug 29, 2018 at 2:49 PM Derrick Stolee via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >> The commit-graph (and multi-pack-index) features are optional data
> >> structures that can make Git operations faster. Since they are optional, we
> >> do not enable them in most Git tests. The commit-graph is tested in
> >> t5318-commit-graph.sh (and t6600-test-reach.sh in ds/reachable), but that
> >> one script cannot cover the data shapes present in the rest of the test
> >> suite.
> >>
> >> This patch introduces a new test environment variable, GIT_TEST_COMMIT_GRAPH
> >> . Similar to GIT_TEST_SPLIT_INDEX, it enables the commit-graph and writes it
> >> with every git commit command. Thanks, Duy, for pointing out this direction
> >> [1].
> > Any reason to not add this new flag in ci/run-build-and-tests.sh
> > (which is used by Travis)? I see your note about VSTS but I don't see
> > why it has to be exclusive to VSTS.
> 
> I only wanted to volunteer resources that I know are available. I am 
> looking into an additional test run in Travis CI builds, but didn't want 
> to presume the extra resources would be available.
> 

OK I think there's a misunderstanding. You would not need a separate
test run for this, instead the second (existing) run just activates
both split index and commit-graph together. The run may be a bit
slower (doubtful since commit-graph is all about speed) but it would
not cost much more resources. Such a change would look like this

diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 2a5bff4a1c..185305e3e2 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -15,6 +15,7 @@ then
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
 	export GIT_TEST_OE_SIZE=10
 	export GIT_TEST_OE_DELTA_SIZE=5
+	export GIT_TEST_COMMIT_GRAPH=yes
 	make --quiet test
 fi
 

--
Duy
