| Message ID | cover.1697319294.git.code@khaugsbakk.name (mailing list archive) |
|---|---|
| Headers | show |
| Series | t7900: untangle test dependencies | expand |
On Sat, Oct 14, 2023 at 11:45:51PM +0200, Kristoffer Haugsbakk wrote: > § CI > > The CI failed but it didn't look relevant. > > https://github.com/LemmingAvalanche/git/actions/runs/6518415327/job/17703822606 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
Kristoffer Haugsbakk <code@khaugsbakk.name> writes: > #!/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 && It is kind-of surprising that with only 8 patches you can reach such a state, but ... > # The tests that used to depend on each other should still pass > # when run together > ./t7900-maintenance.sh --quiet --run=setup,30,31 && ... 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.
On Tue, Oct 17, 2023, at 21:59, Junio C Hamano wrote: > It is kind-of surprising that with only 8 patches you can reach such > a state, but ... > >> # The tests that used to depend on each other should still pass >> # when run together >> ./t7900-maintenance.sh --quiet --run=setup,30,31 && > > ... 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? 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.) > 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. 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
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes: > On Tue, Oct 17, 2023, at 21:59, Junio C Hamano wrote: >> It is kind-of surprising that with only 8 patches you can reach such >> a state, but ... >> >>> # The tests that used to depend on each other should still pass >>> # when run together >>> ./t7900-maintenance.sh --quiet --run=setup,30,31 && >> >> ... 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? > > What I mean is that some preceding test has a side-effect that a test > depends on. 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. > 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. 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.
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