Re: [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact

[Stefan Beller <sbeller@google.com>]

On Mon, Dec 10, 2018 at 8:09 PM Yaroslav Halchenko
<debian@onerussian.com> wrote:

Thanks for the patches. The first patch looks good to me!

> [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact

The subject is a bit cryptic (specifically the first part before the
colon), maybe

  t7406: compare entire submodule status for --reset-hard mode

?


> For submodule update --reset-hard the best test is comparison of the
> entire status as shown by submodule status --recursive.  Upon update
> --reset-hard we should get back to the original state, with all the
> branches being the same (no detached HEAD) and commits identical to
> original  (so no merges, new commits, etc).

"original state" can mean different things to different people. I'd think
we could be more precise:

   ... we should get to the state that the submodule is reset to the
    object id as the superprojects gitlink points at, irrespective of the
    submodule branch.


>  test_expect_success 'submodule update --merge staying on master' '
>         (cd super/submodule &&
> -         git reset --hard HEAD~1
> +        git reset --hard HEAD~1

unrelated white space change?

>         ) &&
>         (cd super &&
>          (cd submodule &&
> @@ -307,16 +318,28 @@ test_expect_success 'submodule update --merge staying on master' '
>  '
>
>  test_expect_success 'submodule update --reset-hard staying on master' '
> [..]
> +'
> +

The tests look good to me, though I wonder if we'd rather want to inline
{record/compare}_submodule_status as then you'd not need to look it up
and the functions are rather short?