Subject: Re: [PATCH v3 07/20] commit-graph: verify catches corrupt signature

From: Jakub Narebski <[email-masked]>
Date: Sun, 03 Jun 2018 00:30:19 +0200

Derrick Stolee <[email-masked]> writes:
> On 5/28/2018 10:05 AM, Jakub Narebski wrote:
>> Derrick Stolee <[email-masked]> writes:

[...]
>>> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
>>> index 6ca451dfd2..bd64481c7a 100755
>>> --- a/t/t5318-commit-graph.sh
>>> +++ b/t/t5318-commit-graph.sh
>>> @@ -235,9 +235,52 @@ test_expect_success 'perform fast-forward merge in full repo' '
>>>   	test_cmp expect output
>>>   '
>>>   +# the verify tests below expect the commit-graph to contain
>>> +# exactly the commits reachable from the commits/8 branch.
>>> +# If the file changes the set of commits in the list, then the
>>> +# offsets into the binary file will result in different edits
>>> +# and the tests will likely break.
>>> +
>>>   test_expect_success 'git commit-graph verify' '
>>>   	cd "$TRASH_DIRECTORY/full" &&
>>> +	git rev-parse commits/8 | git commit-graph write --stdin-commits &&
>>>   	git commit-graph verify >output
>>>   '
>> I don't quite understand what the change is meant to do.
>
> This gives us a constant commit-graph file to work with in the later tests.
>
> To get the "independent test" structure you want for the tests that
> are coming, we need to do one of the following:
>
> 1. Write a new commit-graph file for every test (slows things down).

Or check if correct graph-file exists, and if it doesn't only then write
a new commit-graph file (like I have proposed elsewhere in this thread).

Barring this, I think it would be better if the preparation step was
separated into a 'setup <something>' step, so that one can easier select
which tests to run, at least by hand.

> 2. Do all corruption/verify checks in a single test (reduces the
> information from a failed test, as it only reports the first failure).
>
> I don't like either of these options, so I went with this "prepare" step.

These are not the only possible options.

>> Also, as I said earlier, I'd prefer if tests were as indpendent of each
>> other as possible, to make running individual tests (e.g. only
>> previously falling tests) easier.
>>
>> I especially do not like mixing running actual test with setting up the
>> repository for future tests, as here.