Re: [PATCH 06/28] t0000: annotate with SHA1 prerequisite
- Date: Tue, 8 May 2018 20:28:47 +0200
- From: Martin Ågren <martin.agren@xxxxxxxxx>
- Subject: Re: [PATCH 06/28] t0000: annotate with SHA1 prerequisite
On 8 May 2018 at 01:40, brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, May 07, 2018 at 12:24:13PM +0200, Martin Ågren wrote:
>> This could be more centralized at the top of the file, more likely to be
>> noticed during a `make test` and easier to adapt in the future. Maybe
>> something like this at the top of the file:
>> if hash for storage is SHA-1 then
>> skip_all='unknown hash, but you really need to expand this test'
>> # or even error out!
> As I mentioned in an earlier email, I plan to set an environment
> variable for the algorithms in use and then do something like:
> test "$tree" = "$(test-tool hash-helper --output known-tree)"
> where "known-tree" is some key we can use to look up the SHA-1 or
> NewHash value, and we've specified we want the output format (as opposed
> to input format or on-disk format).
My first thought was, can't we introduce such a helper already now? It
would not check with the environment, but would always output SHA-1
values. Thinking about it some more, maybe the exact usage/interface
would be a bit volatile in the beginning, making it just as good an idea
to gain some more experience and feeling first.
>> When we add NewHash, we get to add an "else if". Otherwise, we'd be
>> plugging in NewHash without testing some very basic stuff.
>> Ok, so `skip_all` is quite hard, but skipping certain basic tests also
>> feels really shaky. Adding a new hash algo without adapting this test
>> should be a no-go, IMHO.
> I agree that this test needs to be updated for NewHash, but since we
> haven't decided what that's going to be, it makes sense during
> development to still test the rest of the code if possible so that we
> know what does and doesn't work.
> This is a first step in making it obvious what doesn't work, and when we
> know what the data is supposed to be, we can adjust it by fixing the
> tests so that it works in all cases.
A lingering feeling is, how do we find all these tests that we "hide"
(for lack of a better word)? Maybe introduce some standardized comment
keyword. Or use a more grep-friendly prereq-name. Or, if all occurances
of "SHA1" (outside of t????-*sha*) are expected to go away real soon
now, maybe this is not an issue.
Anyway, thanks for putting up with me, and thanks a big lot for driving
this major undertaking forward.