Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
Derrick Stolee writes: > I see these failures, too, but I believe they are due to > ds/commit-graph-with-grafts not being merged to 'next' yet. The > purpose of that branch is to fix these test breaks. The environment > variable got merged a lot faster. A separate "ping" would have helped me. Will merge it down to 'next'.
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On 10/8/2018 10:58 AM, Ævar Arnfjörð Bjarmason wrote: On Mon, Oct 08 2018, Derrick Stolee wrote: On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote: On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: From: Derrick Stolee The commit-graph feature is tested in isolation by t5318-commit-graph.sh and t6600-test-reach.sh, but there are many more interesting scenarios involving commit walks. Many of these scenarios are covered by the existing test suite, but we need to maintain coverage when the optional commit-graph structure is not present. To allow running the full test suite with the commit-graph present, add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try to load the commit-graph when parsing commits, and writes the commit-graph file after every 'git commit' command. There are a few tests that rely on commits not existing in pack-files to trigger important events, so manually set GIT_TEST_COMMIT_GRAPH to false for the necessary commands. There is one test in t6024-recursive-merge.sh that relies on the merge-base algorithm picking one of two ambiguous merge-bases, and the commit-graph feature changes which merge-base is picked. The test feature itself seems fine, but this consistently fails ever since it got introduced (a reset --hard on the commit merged to msater in git.git): GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh Test Summary Report --- t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) Failed tests: 3, 5, 7, 9, 11, 13 Non-zero exit status: 1 t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) Failed tests: 12-16, 24-25, 30, 35 Non-zero exit status: 1 t5500-fetch-pack.sh(Wstat: 256 Tests: 357 Failed: 1) Failed test: 351 Non-zero exit status: 1 This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I can provide more info (-x output etc..). I see these failures, too, but I believe they are due to ds/commit-graph-with-grafts not being merged to 'next' yet. The purpose of that branch is to fix these test breaks. The environment variable got merged a lot faster. I just built & tested the 'jch' branch at 515d82d9 with GIT_TEST_COMMIT_GRAPH=1 and they all passed. I should have tested "pu" first. These failures are indeed fixed there. Thanks, and sorry about the noise. Thanks for testing with the optional features! It's good to keep them exercised.
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On Mon, Oct 08 2018, Derrick Stolee wrote: > On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote: >> On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: >> >>> From: Derrick Stolee >>> >>> The commit-graph feature is tested in isolation by >>> t5318-commit-graph.sh and t6600-test-reach.sh, but there are many >>> more interesting scenarios involving commit walks. Many of these >>> scenarios are covered by the existing test suite, but we need to >>> maintain coverage when the optional commit-graph structure is not >>> present. >>> >>> To allow running the full test suite with the commit-graph present, >>> add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar >>> to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try >>> to load the commit-graph when parsing commits, and writes the >>> commit-graph file after every 'git commit' command. >>> >>> There are a few tests that rely on commits not existing in >>> pack-files to trigger important events, so manually set >>> GIT_TEST_COMMIT_GRAPH to false for the necessary commands. >>> >>> There is one test in t6024-recursive-merge.sh that relies on the >>> merge-base algorithm picking one of two ambiguous merge-bases, and >>> the commit-graph feature changes which merge-base is picked. >>> >> The test feature itself seems fine, but this consistently fails ever >> since it got introduced (a reset --hard on the commit merged to msater >> in git.git): >> >> GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) >> t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh >> Test Summary Report >> --- >> t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) >>Failed tests: 3, 5, 7, 9, 11, 13 >>Non-zero exit status: 1 >> t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) >>Failed tests: 12-16, 24-25, 30, 35 >>Non-zero exit status: 1 >> t5500-fetch-pack.sh(Wstat: 256 Tests: 357 Failed: 1) >>Failed test: 351 >>Non-zero exit status: 1 >> >> This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I >> can provide more info (-x output etc..). > I see these failures, too, but I believe they are due to > ds/commit-graph-with-grafts not being merged to 'next' yet. The > purpose of that branch is to fix these test breaks. The environment > variable got merged a lot faster. > > I just built & tested the 'jch' branch at 515d82d9 with > GIT_TEST_COMMIT_GRAPH=1 and they all passed. I should have tested "pu" first. These failures are indeed fixed there. Thanks, and sorry about the noise.
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On 10/8/2018 9:43 AM, Ævar Arnfjörð Bjarmason wrote: On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: From: Derrick Stolee The commit-graph feature is tested in isolation by t5318-commit-graph.sh and t6600-test-reach.sh, but there are many more interesting scenarios involving commit walks. Many of these scenarios are covered by the existing test suite, but we need to maintain coverage when the optional commit-graph structure is not present. To allow running the full test suite with the commit-graph present, add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try to load the commit-graph when parsing commits, and writes the commit-graph file after every 'git commit' command. There are a few tests that rely on commits not existing in pack-files to trigger important events, so manually set GIT_TEST_COMMIT_GRAPH to false for the necessary commands. There is one test in t6024-recursive-merge.sh that relies on the merge-base algorithm picking one of two ambiguous merge-bases, and the commit-graph feature changes which merge-base is picked. The test feature itself seems fine, but this consistently fails ever since it got introduced (a reset --hard on the commit merged to msater in git.git): GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh Test Summary Report --- t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) Failed tests: 3, 5, 7, 9, 11, 13 Non-zero exit status: 1 t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) Failed tests: 12-16, 24-25, 30, 35 Non-zero exit status: 1 t5500-fetch-pack.sh(Wstat: 256 Tests: 357 Failed: 1) Failed test: 351 Non-zero exit status: 1 This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I can provide more info (-x output etc..). I see these failures, too, but I believe they are due to ds/commit-graph-with-grafts not being merged to 'next' yet. The purpose of that branch is to fix these test breaks. The environment variable got merged a lot faster. I just built & tested the 'jch' branch at 515d82d9 with GIT_TEST_COMMIT_GRAPH=1 and they all passed. Thanks, -Stolee
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On Tue, Aug 28 2018, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee > > The commit-graph feature is tested in isolation by > t5318-commit-graph.sh and t6600-test-reach.sh, but there are many > more interesting scenarios involving commit walks. Many of these > scenarios are covered by the existing test suite, but we need to > maintain coverage when the optional commit-graph structure is not > present. > > To allow running the full test suite with the commit-graph present, > add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar > to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try > to load the commit-graph when parsing commits, and writes the > commit-graph file after every 'git commit' command. > > There are a few tests that rely on commits not existing in > pack-files to trigger important events, so manually set > GIT_TEST_COMMIT_GRAPH to false for the necessary commands. > > There is one test in t6024-recursive-merge.sh that relies on the > merge-base algorithm picking one of two ambiguous merge-bases, and > the commit-graph feature changes which merge-base is picked. > The test feature itself seems fine, but this consistently fails ever since it got introduced (a reset --hard on the commit merged to msater in git.git): GIT_TEST_COMMIT_GRAPH=true prove -j$(parallel --number-of-cores) t5500-fetch-pack.sh t6001-rev-list-graft.sh t6050-replace.sh Test Summary Report --- t6001-rev-list-graft.sh (Wstat: 256 Tests: 14 Failed: 6) Failed tests: 3, 5, 7, 9, 11, 13 Non-zero exit status: 1 t6050-replace.sh (Wstat: 256 Tests: 35 Failed: 9) Failed tests: 12-16, 24-25, 30, 35 Non-zero exit status: 1 t5500-fetch-pack.sh(Wstat: 256 Tests: 357 Failed: 1) Failed test: 351 Non-zero exit status: 1 This is on Linux/Debian 4.17.0-1-amd64. Can you reproduce this? If not I can provide more info (-x output etc..).
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On 8/28/2018 5:59 PM, Eric Sunshine wrote: On Tue, Aug 28, 2018 at 5:31 PM Derrick Stolee wrote: On 8/28/2018 4:41 PM, Stefan Beller wrote: On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget wrote: + GIT_TEST_COMMIT_GRAPH=0 && + test_must_fail git merge -m final G This could go on the same line without the && in between, setting the variable as a prefix. It cannot! The Linux build I ran complained that you can't put environment variables through test_must_fail. Is GIT_TEST_COMMIT_GRAPH exported? If not, it won't have an impact on git-merge anyhow. In my testing this changed the behavior from fail to pass when passing GIT_TEST_COMMIT_GRAPH=1 from the command. As for the special case of one-shot environment variable and test_must_fail(), you'll find "env" used as a workaround in a number of tests: test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge ... && Thanks for this! This is clearly the better solution. -Stolee
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On Tue, Aug 28, 2018 at 5:31 PM Derrick Stolee wrote: > On 8/28/2018 4:41 PM, Stefan Beller wrote: > > On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget > > wrote: > >> + GIT_TEST_COMMIT_GRAPH=0 && > >> + test_must_fail git merge -m final G > > This could go on the same line without the && in between, setting the > > variable as a prefix. > > It cannot! The Linux build I ran complained that you can't put > environment variables through test_must_fail. Is GIT_TEST_COMMIT_GRAPH exported? If not, it won't have an impact on git-merge anyhow. As for the special case of one-shot environment variable and test_must_fail(), you'll find "env" used as a workaround in a number of tests: test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge ... &&
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On 8/28/2018 4:41 PM, Stefan Beller wrote: On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget wrote: From: Derrick Stolee The commit-graph feature is tested in isolation by t5318-commit-graph.sh and t6600-test-reach.sh, but there are many more interesting scenarios involving commit walks. Many of these scenarios are covered by the existing test suite, but we need to maintain coverage when the optional commit-graph structure is not present. To allow running the full test suite with the commit-graph present, add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try to load the commit-graph when parsing commits, and writes the commit-graph file after every 'git commit' command. There are a few tests that rely on commits not existing in pack-files to trigger important events, so manually set GIT_TEST_COMMIT_GRAPH to false for the necessary commands. So the plan is to turn on the commit graph for the whole test suite excluding these selected tests? Excluding these specific _steps_, but yes. + GIT_TEST_COMMIT_GRAPH=0 && + test_must_fail git merge -m final G This could go on the same line without the && in between, setting the variable as a prefix. It cannot! The Linux build I ran complained that you can't put environment variables through test_must_fail. -Stolee
Re: [PATCH 1/1] commit-graph: define GIT_TEST_COMMIT_GRAPH
On Tue, Aug 28, 2018 at 1:33 PM Derrick Stolee via GitGitGadget wrote: > > From: Derrick Stolee > > The commit-graph feature is tested in isolation by > t5318-commit-graph.sh and t6600-test-reach.sh, but there are many > more interesting scenarios involving commit walks. Many of these > scenarios are covered by the existing test suite, but we need to > maintain coverage when the optional commit-graph structure is not > present. > > To allow running the full test suite with the commit-graph present, > add a new test environment variable, GIT_TEST_COMMIT_GRAPH. Similar > to GIT_TEST_SPLIT_INDEX, this variable makes every Git command try > to load the commit-graph when parsing commits, and writes the > commit-graph file after every 'git commit' command. > > There are a few tests that rely on commits not existing in > pack-files to trigger important events, so manually set > GIT_TEST_COMMIT_GRAPH to false for the necessary commands. So the plan is to turn on the commit graph for the whole test suite excluding these selected tests? > diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh > index 4984ca583d..73d5284a91 100755 > --- a/t/t0410-partial-clone.sh > +++ b/t/t0410-partial-clone.sh > @@ -181,7 +181,7 @@ test_expect_success 'rev-list stops traversal at missing > and promised commit' ' > > git -C repo config core.repositoryformatversion 1 && > git -C repo config extensions.partialclone "arbitrary string" && > - git -C repo rev-list --exclude-promisor-objects --objects bar >out && > + GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list > --exclude-promisor-objects --objects bar >out && > + GIT_TEST_COMMIT_GRAPH=0 && > + test_must_fail git merge -m final G This could go on the same line without the && in between, setting the variable as a prefix.
