On Wed, Mar 2, 2022 at 5:10 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Keller, Jacob E" <jacob.e.keller@xxxxxxxxx> writes: > > > Let's clarify. There are two versions of the test in this version: > > > > 1) test which enables commit graph and tests that it does the right behavior. > > > > 2) test which removes commit graph and tests that it behaves the old way. > > > > > > test (1) checks the flow with the commit graph enabled, and verifies that with a commit graph the new behavior is used. This test will fail if you revert the name-rev commit-graph support. > > > > test (2) always performs the way we don't like. (since we disable the commit graph and the new flow doesn't kick in) This is the test I think I will eliminate in the next revision. > > > > > > I will remove the 2nd test, since the first test covers the change > > in behavior just fine, and I think I agree we don't need to set > > in-stone the implementation without commit graph. > > > > I will also look at adding a test which performs a count of which > > revisions get inspected and makes sure that we actually are doing > > the optimization. > > Sounds like a sensible thing to do. > > In any case, in the current patch, #2 is not working in > linux-TEST-vars job at CI. You can visit this URL > > https://github.com/git/git/runs/5400048732?check_suite_focus=true#step:4:68062 > > while logged into your GitHub account for details. Looks like this job sets all the TEST variables including GIT_TEST_COMMIT_GRAPH=1? The negative test passes because the commit graph is enforced on and we then succeed even though we were trying to test the negative case. I'm going to remove that test in v3 anyways, so I don't think it is a big deal. However, I wonder is there some way to mark a test as explicitely "don't run if GIT_TEST_COMMIT_GRAPH is set"? Thanks, Jake
