Re: Git Test Coverage Report (v2.27.0-rc0)
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- May 19, 2020, 23:42 UTC
- Message-ID
- <20200519234208.GR6362@camp.crustytoothpaste.net>
- In-Reply-To
- <853759d3-97c3-241f-98e1-990883cd204e@gmail.com>
On 2020-05-19 at 12:11:15, Derrick Stolee wrote:
Show 24 quoted lines
> On 5/15/2020 1:22 PM, Derrick Stolee wrote:
> > brian m. carlson 13e7ed6a builtin/checkout: compute checkout metadata for checkouts
> > builtin/checkout.c
> > 13e7ed6a 625) is_null_oid(&info->oid) ? &tree->object.oid :
>
> This is part of the following hunk:
>
> @@ -619,6 +620,11 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
> opts.verbose_update = o->show_progress;
> opts.src_index = &the_index;
> opts.dst_index = &the_index;
> + init_checkout_metadata(&opts.meta, info->refname,
> + info->commit ? &info->commit->object.oid :
> + is_null_oid(&info->oid) ? &tree->object.oid :
> + &info->oid,
> + NULL);
> parse_tree(tree);
> init_tree_desc(&tree_desc, tree->buffer, tree->size);
> switch (unpack_trees(1, &tree_desc, &opts)) {
>
> The double-nested ternary definitely complicates the coverage here.
> It also points out that all tests have `info->commit` a non-NULL.
>
> This certainly looks safe, but I don't know.This is me trying to be defensive. I think the code path that is not covered here that can be is the info->oid code path; I don't technically believe the other case (using the tree) is possible, although the checkout code is complex enough that I can't be certain.
It would look like this, I believe:
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index 4bfffa9c31..a69ec3c4b7 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -408,6 +408,25 @@ test_expect_success PERL 'required process filter should filter data' ' EOF test_cmp_exclude_clean expected.log debug.log && + # Make sure that the file appears dirty, so checkout below has to + # run the configured filter. + META="treeish=$MASTER" && + rm -f test2.r "testsubdir/test3 '\''sq'\'',\$x=.r" && + + filter_git checkout --quiet --no-progress $(git rev-parse master) && + cat >expected.log <<-EOF && + START + init handshake complete + IN: smudge test.r $META blob=$M $S [OK] -- OUT: $S . [OK] + IN: smudge test2.r $META blob=$M2 $S2 [OK] -- OUT: $S2 . [OK] + IN: smudge test4-empty.r $META blob=$EMPTY 0 [OK] -- OUT: 0 [OK] + IN: smudge testsubdir/test3 '\''sq'\'',\$x=.r $META blob=$M3 $S3 [OK] -- OUT: $S3 . [OK] + STOP + EOF + test_cmp_exclude_clean expected.log debug.log && + + git checkout empty-branch && + META="ref=refs/heads/master treeish=$MASTER" && filter_git checkout --quiet --no-progress master && cat >expected.log <<-EOF && START I'm about to go make dinner, but I'll try to get a patch written up with this tonight or tomorrow. If it's more urgent than that, you may add my sign-off.
-- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204