t7900: untangle test dependencies

Untangle test dependencies so that all tests only need setup tests to have been run.

For example:

```
./t7900-maintenance.sh --run=setup,31
```

Test with:

```
#!/bin/sh
cd t
# Every test run together with `setup` should pass
for i in $(seq 1 42)
do
    ./t7900-maintenance.sh --quiet --run=setup,$i || return 1
done &&
# Whole test suite should pass
./t7900-maintenance.sh --quiet &&
# The tests that used to depend on each other should still pass
# when run together
./t7900-maintenance.sh --quiet --run=setup,30,31 &&
./t7900-maintenance.sh --quiet --run=setup,11,12 &&
./t7900-maintenance.sh --quiet --run=setup,3,19 &&
./t7900-maintenance.sh --quiet --run=setup,23,24 &&
./t7900-maintenance.sh --quiet --run=setup,33,34,35 &&
./t7900-maintenance.sh --quiet --run=setup,36,40 &&
./t7900-maintenance.sh --quiet --run=setup,36,40 &&
./t7900-maintenance.sh --quiet --run=setup,36,37 &&
./t7900-maintenance.sh --quiet --run=setup,15,23,24 &&
printf "\nAll passed\n" ||
printf '\n***Failed***\n'
```

§ CI

The CI failed but it didn't look relevant.

https://github.com/LemmingAvalanche/git/actions/runs/6518415327/job/17703822606

Cheers

Kristoffer Haugsbakk (8):
t7900: remove register dependency
t7900: setup and tear down clones
t7900: create commit so that branch is born
t7900: factor out inheritance test dependency
t7900: factor out common schedule setup
t7900: fix pfx dependency
t7900: fix print-args dependency
t7900: factor out packfile dependency

 t/t7900-maintenance.sh | 49 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 6 deletions(-)

base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40 --
2.42.0.2.g879ad04204

stop from existing schedule depends on the preceding test start from empty cron table because the preceding test registers the repository. Without it, the “stop” test fails because config fails to get the repository:

    git config --get --global --fixed-value maintenance.repo "$(pwd)"

Remove this dependency by setting up the state and tearing it down independently.

---
 t/t7900-maintenance.sh | 3 +++
 1 file changed, 3 insertions(+)

index 487e326b3fa..ca86b2ba687 100755
@@ -588,6 +588,7 @@ test_expect_success 'start --scheduler=<scheduler>' '
 '
 
 test_expect_success 'start from empty cron table' '
+	test_when_finished git maintenance unregister &&
 	GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start --scheduler=crontab &&
 
 	# start registers the repo

@@ -599,6 +600,8 @@ test_expect_success 'start from empty cron table' '
 '
 
 test_expect_success 'stop from existing schedule' '
+	test_when_finished git maintenance unregister &&
+	git maintenance register &&
 	GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance stop &&
 
 	# stop does not unregister the repo

2.42.0.2.g879ad04204

Test loose-objects task depends on the two clones setup in prefetch multiple remotes.

Reuse the two clones setup and tear down the clones afterwards in both tests.

---
 t/t7900-maintenance.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

index ca86b2ba687..ebc207f1a58 100755
@@ -145,6 +145,12 @@ test_expect_success 'run --task=prefetch with no remotes' '
 '
 
 test_expect_success 'prefetch multiple remotes' '
+	test_when_finished rm -r clone1 &&
+	test_when_finished rm -r clone2 &&
+	test_when_finished git remote remove remote1 &&
+	test_when_finished git remote remove remote2 &&
+	test_when_finished git tag --delete one &&
+	test_when_finished git tag --delete two &&
 	git clone . clone1 &&
 	git clone . clone2 &&
 	git remote add remote1 "file://$(pwd)/clone1" &&

@@ -175,6 +181,22 @@ test_expect_success 'prefetch multiple remotes' '
 '
 
 test_expect_success 'loose-objects task' '
+	test_when_finished rm -r clone1 &&
+	test_when_finished rm -r clone2 &&
+	test_when_finished git remote remove remote1 &&
+	test_when_finished git remote remove remote2 &&
+	test_when_finished git tag --delete one &&
+	test_when_finished git tag --delete two &&
+	git clone . clone1 &&
+	git clone . clone2 &&
+	git remote add remote1 "file://$(pwd)/clone1" &&
+	git remote add remote2 "file://$(pwd)/clone2" &&
+	git -C clone1 switch -c one &&
+	git -C clone2 switch -c two &&
+	test_commit -C clone1 one &&
+	test_commit -C clone2 two &&
+	git fetch --all &&
+
 	# Repack everything so we know the state of the object dir
 	git repack -adk &&
 

2.42.0.2.g879ad04204

I'll have to think more about this part in order to understand the ramifications. Thanks for the feedback.

I can rewrite this one—as well as others—to use the setup keyword in the original test instead.

But dropping the series is also fine. I am still very new to this test suite.

Cheers

Kristoffer Haugsbakk

pack-refs task cannot be run in isolation but does pass if maintenance.auto config option is run first.

Create a commit so that HEAD does not point to an unborn branch.

---
 t/t7900-maintenance.sh | 1 +
 1 file changed, 1 insertion(+)

index ebc207f1a58..4bfb4ec5cf6 100755
@@ -388,6 +388,7 @@ test_expect_success 'maintenance.incremental-repack.auto (when config is unset)'
 '
 
 test_expect_success 'pack-refs task' '
+	test_commit message &&
 	for n in $(test_seq 1 5)
 	do
 		git branch -f to-pack/$n HEAD || return 1

2.42.0.2.g879ad04204

Factor out the dependency that test maintenance.strategy inheritance has on test --schedule inheritance weekly -> daily -> hourly.

---
 t/t7900-maintenance.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 4bfb4ec5cf6..6e3ee365ccd 100755
@@ -408,14 +408,16 @@ test_expect_success 'invalid --schedule value' '
 	test_i18ngrep "unrecognized --schedule" err
 '
 
-test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
+test_expect_success 'setup for inheritance' '
 	git config maintenance.loose-objects.enabled true &&
 	git config maintenance.loose-objects.schedule hourly &&
 	git config maintenance.commit-graph.enabled true &&
 	git config maintenance.commit-graph.schedule daily &&
 	git config maintenance.incremental-repack.enabled true &&
-	git config maintenance.incremental-repack.schedule weekly &&
+	git config maintenance.incremental-repack.schedule weekly
+'
 
+test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
 	GIT_TRACE2_EVENT="$(pwd)/hourly.txt" \
 		git maintenance run --schedule=hourly 2>/dev/null &&
 	test_subcommand git prune-packed --quiet <hourly.txt &&

2.42.0.2.g879ad04204

Tests magic markers are correct and stop preserves surrounding schedule depend on some setup in start preserves existing schedule.

Factor out the setup code.

---
 t/t7900-maintenance.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

index 6e3ee365ccd..ebde3e8a212 100755
@@ -637,9 +637,12 @@ test_expect_success 'stop from existing schedule' '
 	test_must_be_empty cron.txt
 '
 
-test_expect_success 'start preserves existing schedule' '
+test_expect_success 'setup important information for schedule' '
 	echo "Important information!" >cron.txt &&
-	GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start --scheduler=crontab &&
+	GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start --scheduler=crontab
+'
+
+test_expect_success 'start preserves existing schedule' '
 	grep "Important information!" cron.txt
 '
 

2.42.0.2.g879ad04204

Test start and stop when several schedulers are available depends on pfx from start and stop macOS maintenance.

Duplicate the behavior.

---
 t/t7900-maintenance.sh | 1 +
 1 file changed, 1 insertion(+)

index ebde3e8a212..15a8653b583 100755
@@ -794,6 +794,7 @@ test_expect_success 'start and stop Linux/systemd maintenance' '
 '
 
 test_expect_success 'start and stop when several schedulers are available' '
+	pfx=$(cd "$HOME" && pwd) &&
 	write_script print-args <<-\EOF &&
 	printf "%s\n" "$*" | sed "s:gui/[0-9][0-9]*:gui/[UID]:; s:\(schtasks /create .* /xml\).*:\1:;" >>args
 	EOF

2.42.0.2.g879ad04204

Test use launchctl list to prevent extra work depends on print-args from start and stop macOS maintenance.

Duplicate the script writing.

---
 t/t7900-maintenance.sh | 3 +++
 1 file changed, 3 insertions(+)

index 15a8653b583..99279e41787 100755
@@ -709,6 +709,9 @@ test_expect_success 'start and stop macOS maintenance' '
 '
 
 test_expect_success 'use launchctl list to prevent extra work' '
+	write_script print-args <<-\EOF &&
+	echo $* | sed "s:gui/[0-9][0-9]*:gui/[UID]:" >>args
+	EOF
 	# ensure we are registered
 	GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start --scheduler=launchctl &&
 

2.42.0.2.g879ad04204

Tests '--schedule inheritance weekly -> daily -> hourly and maintenance.strategy inheritance depend on the packfile made in incremental-repack task.

Factor out the packfile creation.

---
 t/t7900-maintenance.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 99279e41787..bc417b518b5 100755
@@ -257,13 +257,15 @@ test_expect_success 'maintenance.loose-objects.auto' '
 	test_subcommand git prune-packed --quiet <trace-loC
 '
 
-test_expect_success 'incremental-repack task' '
+test_expect_success 'setup packfile' '
 	packDir=.git/objects/pack &&
 	for i in $(test_seq 1 5)
 	do
 		test_commit $i || return 1
-	done &&
+	done
+'
 
+test_expect_success 'incremental-repack task' '
 	# Create three disjoint pack-files with size BIG, small, small.
 	echo HEAD~2 | git pack-objects --revs $packDir/test-1 &&
 	test_tick &&

2.42.0.2.g879ad04204

The test maintenance.auto config option will fail if any preceding test has run git maintenance register since that turns maintenance.auto off for that repository and later calls to unregister will not turn it back to the default true value.

Start with a fresh repository in this test.

---

Notes (series):
I found this after publishing the series.

 t/t7900-maintenance.sh | 2 ++
 1 file changed, 2 insertions(+)

index bc417b518b..dbc5e1eb44 100755
@@ -55,6 +55,8 @@ test_expect_success 'run [--auto|--quiet]' '
 '
 
 test_expect_success 'maintenance.auto config option' '
+	rm -rf .git &&
+	git init &&
 	GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 &&
 	test_subcommand git maintenance run --auto --quiet <default &&
 	GIT_TRACE2_EVENT="$(pwd)/true" \

2.42.0.2.g879ad04204

From a brief look, I think it is that your branch is based on v2.42.0, which does not contain 0763c3a2c4 (http: update curl http/2 info matching for curl 8.3.0, 2023-09-15). And the macos CI image has since been updated to a more recent version of curl.

So yeah, not anything to worry about for your series.

-Peff

It is kind-of surprising that with only 8 patches you can reach such a state, but ...

... this puzzles me. What does it mean for tests to "depend on each other"? Does this mean running #31 with or without running #30 runs under different condition and potentially run different things?

One might argue that, in an ideal world, our tests should work when any non-setup tests are omitted (so, instead of $i above, you'll have an arbitrary subsequence of 1..42 and your tests still pass), and it may be a worthy goal, but at the same time, it may be a bit impractical, as setting things up is costly, but what you can do in the common "setup" will be very small. Or you'll have so much "recovering from damage" in test_when_finished for each test that makes such untangling of dependencies too costly.

What I mean is that some preceding test has a side-effect that a test depends on. Or that the test depends on some test *not* having done something; patch 9/8 changes maintenance.auto config option to delete and init the repository since it depends on the preceding tests *not* having run git maintenance register, since that turns off the default true value of maintenance.auto.

(Maybe those last meta-tests with combining tests like number 30 and 31 was a bit silly.)

I don't know what the policy is. :) My motivation was that I was working on something else which seemed to break the suite, then I tried to reduce the tests that were run to get rid of the noise (--verbose), but then it got confusing because I didn't know if I had really broken some tests myself or if more tests would start failing by only running a subset of them.

That last patch 9/8 deals with what I discovered when I added two tests before maintenance.auto config option; the test started failing, which made me think that my changes might have some side-effect on what the test is testing. But it was just an invisible dependency on git maintenance register *not* having been run in the whole suite up until that point.

Cheers

I see. And 31 used to depend on the side effect of having ran 30, but in the updated test, the precondition 31 depends on is created by itself without relying on what 30 did (and in fact, perhaps in the updated test, 30 may rewind what it did as part of the clean-up process using test_when_finished). That makes sense.

Yeah, it is a laudable goal, but I am not sure how practical it is to expect developers to maintain that propertly. Unless there is some automated test to enforce the independence of the tests, that is.

Thanks.