On Tue, Jun 23, 2020 at 06:28:01AM -0400, Denton Liu wrote:
> I've taken your suggestions and rerolled the last patch. Notably, I
> combined $before and $command into a single function and then passed in
> `test_must_fail` as the second argument if it's expected to fail. I also
> decided to use a helper function to reduce the duplication of the
> calling logic but it's still a little verbose because I opted to make
> use of argument passing over global variables where possible.
>
> One thing that I'm leaning towards is completely removing $after. We can
> combine $after with $command like I did in the first few iterations of
> this series and just write it as:
>
> git_test_thing () {
> # before...
> $2 git do-the-thing "$1" &&
> if test -n "$2"
> then
> return
> fi &&
> # after...
> }
>
> but I'm also okay with leaving it as is.
Yeah, as soon as I saw "combined $before and $command", then I wondered
why we needed to split anything (since clearly we're passing
responsibility to handle must_fail to the callback function).
So I think we should either have the full before/command/after split, or
have a single callback function which understands that it has to respect
"$2" as a prefix).
I'm OK with either. I do like both of them better than the magic
$OVERWRITING_FAIL thing, but this series seems to be taking a lot more
energy than I think its outcome is worth. Let's resolve it somehow and
work on something more exciting. ;)
> Also, one problem that's present is that for tests that use
> test_submodule_switch() and test_submodule_forced_switch(), $command
> will be listed as git_test_func, which might make debugging more
> difficult. I think I'd prefer the version we currently have queued for
> this reason (and why I did the whole song and dance of $OVERWRITING_FAIL
> in the first place), but if you think that this is fine, I'm fine with
> this approach as well.
I agree it's not ideal, but I don't think it's too bad. There are lots
of tests that have output you can't just cut-and-paste due to code
generation, helper functions, etc.
-Peff
