mbox series

[00/12] Group reffiles tests

Message ID pull.1647.git.git.1705521155.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Group reffiles tests | expand

Message

Jean-Noël Avila via GitGitGadget Jan. 17, 2024, 7:52 p.m. UTC
This series groups REFFILES specific tests together. These tests are
currently grouped together across the test suite based on functionality.
However, since they exercise low-level behavior specific to the refs backend
being used (in these cases, the ref-files backend), group them together
based on which refs backend they test. This way, in the near future when the
reftables backend gets upstreamed we can add tests that exercise the
reftables backend close by in the t06xx area.

These patches also remove the REFFILES prerequisite, since all the tests in
t06xx are reffiles specific. In the near future, once the reftable backend
is upstreamed, all the tests in t06xx will be forced to run with the
reffiles backend.

John Cai (12):
  t3210: move to t0602
  remove REFFILES prerequisite
  t1414: convert test to use Git commands instead of writing refs
    manually
  t1404: move reffiles specific tests to t0600
  t1405: move reffiles specific tests to t0600
  t1406: move reffiles specific tests to t0600
  t1410: move reffiles specific tests to t0600
  t1415: move reffiles specific tests to t0600
  t1503: move reffiles specific tests to t0600
  t3903: move reffiles specific tests to t0600
  t4202: move reffiles specific tests to t0600
  t5312: move reffiles specific tests to t0600

 t/t0600-reffiles-backend.sh                   | 604 ++++++++++++++++++
 ...ck-refs.sh => t0602-reffiles-pack-refs.sh} |   0
 t/t1404-update-ref-errors.sh                  | 378 -----------
 t/t1405-main-ref-store.sh                     |  10 +-
 t/t1407-worktree-ref-store.sh                 |  37 --
 t/t1410-reflog.sh                             |  42 --
 t/t1414-reflog-walk.sh                        |  11 +-
 t/t1415-worktree-refs.sh                      |  11 -
 t/t1503-rev-parse-verify.sh                   |   5 -
 t/t2017-checkout-orphan.sh                    |   2 +-
 t/t3903-stash.sh                              |  43 --
 t/t4202-log.sh                                |  17 -
 t/t5312-prune-corruption.sh                   |  26 -
 t/test-lib-functions.sh                       |  16 +
 14 files changed, 628 insertions(+), 574 deletions(-)
 create mode 100755 t/t0600-reffiles-backend.sh
 rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)


base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1647%2Fjohn-cai%2Fjc%2Fgroup-reffiles-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1647/john-cai/jc/group-reffiles-tests-v1
Pull-Request: https://github.com/git/git/pull/1647

Comments

Junio C Hamano Jan. 18, 2024, 1:17 a.m. UTC | #1
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This series groups REFFILES specific tests together. These tests are
> currently grouped together across the test suite based on functionality.
> However, since they exercise low-level behavior specific to the refs backend
> being used (in these cases, the ref-files backend), group them together
> based on which refs backend they test. This way, in the near future when the
> reftables backend gets upstreamed we can add tests that exercise the
> reftables backend close by in the t06xx area.
>
> These patches also remove the REFFILES prerequisite, since all the tests in
> t06xx are reffiles specific.

As we already have REFFILES lazy prereq, even _before_ we enable the
reftable backend, I think that we should start t0600 and t0602 with

	. ./test-lib.sh
	if ! test_have_prereq REFFILES
	then
		skip_all='skipping reffiles specific tests'
		test_done
	fi

which is more in line with the existing convention.  It is more
efficient than "forcing t0600 and t0602 to run always with reffiles"
when you have a CI job that uses reftable for all tests and another
CI job that uses reffiles for all tests.

> In the near future, once the reftable backend is upstreamed, all
> the tests in t06xx will be forced to run with the reffiles
> backend.

Presumably if there are reftable backend specific tests, they will
also be given names out of t06xx range, right?  And then they will
be skipped when the test is not using reftable as the default ref
backend, using the REFTABLE prerequisite in a similar way as shown
above for REFFILES, right?

Thanks.
Patrick Steinhardt Jan. 18, 2024, 11:38 a.m. UTC | #2
On Wed, Jan 17, 2024 at 05:17:17PM -0800, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > This series groups REFFILES specific tests together. These tests are
> > currently grouped together across the test suite based on functionality.
> > However, since they exercise low-level behavior specific to the refs backend
> > being used (in these cases, the ref-files backend), group them together
> > based on which refs backend they test. This way, in the near future when the
> > reftables backend gets upstreamed we can add tests that exercise the
> > reftables backend close by in the t06xx area.
> >
> > These patches also remove the REFFILES prerequisite, since all the tests in
> > t06xx are reffiles specific.
> 
> As we already have REFFILES lazy prereq, even _before_ we enable the
> reftable backend, I think that we should start t0600 and t0602 with
> 
> 	. ./test-lib.sh
> 	if ! test_have_prereq REFFILES
> 	then
> 		skip_all='skipping reffiles specific tests'
> 		test_done
> 	fi
> 
> which is more in line with the existing convention.  It is more
> efficient than "forcing t0600 and t0602 to run always with reffiles"
> when you have a CI job that uses reftable for all tests and another
> CI job that uses reffiles for all tests.

I think it depends. If we use the REFFILES prereq for the files-specific
tests, then we should likely also use the REFTABLE prereq for the
reftable-specific tests.

But that raises the question of whether we want to add a CI job that
exercises code with the reftable backend for every major platform
(Linux, macOS, Windows). If so then your proposal would be fine with me
as we make sure that things work alright on all of them. But if we think
that this would be too expensive then I'd like to at least have very
basic test coverage on all platforms by always running these
backend-specific tests.

Patrick
Junio C Hamano Jan. 18, 2024, 8 p.m. UTC | #3
Patrick Steinhardt <ps@pks.im> writes:

> I think it depends. If we use the REFFILES prereq for the files-specific
> tests, then we should likely also use the REFTABLE prereq for the
> reftable-specific tests.

Correct.  I've assumed that as a given; while introducing any new
implementation of a subsystem that has widespread impact, we would
test things with the original and new implementations.  It happened
while we were moving "ort" to replace "recursive" as an internal
tree merge machinery, for example.  linux-TEST-vars job that is
available both in GitHub and GitLab CI is an example of a separate
job that runs everything with non-default configurations, and "use
reftable as the default backend" GIT_TEST_REFTABLE knob may be an
appropriate thing to set there.

> But that raises the question of whether we want to add a CI job that
> exercises code with the reftable backend for every major platform
> (Linux, macOS, Windows). If so then your proposal would be fine with me
> as we make sure that things work alright on all of them. But if we think
> that this would be too expensive then I'd like to at least have very
> basic test coverage on all platforms by always running these
> backend-specific tests.
>
> Patrick