Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands

From: Stefan Beller <sbeller@google.com>
Date: May 1, 2018, 20:23 UTC
Message-ID: <CAGZ79kY686ptTPY8DVwr79F2vC6eC_BP=5M7isu9Gfu+eX7vDg@mail.gmail.com>
In-Reply-To: <20180501180908.17443-3-kcghost@gmail.com>

On Tue, May 1, 2018 at 11:09 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:

Show 9 quoted lines

>
> +test_expect_success 'submodule add --reference with --dissociate doesnt use alternates' '
> +       (
> +               cd super &&
> +               git submodule add --reference ../B --dissociate "file://$base_dir/A" sub-dissociate &&
> +               git commit -m B-super-added &&
> +               git repack -ad
> +       ) &&
> +       test_must_fail test_alternate_is_used super/.git/modules/sub-dissociate/objects/info/alternates super/sub-dissociate

We do already have the unfortunate precedent of using "test_must_fail test_alternate_is_used" (and it was partially included by me in 31224cbdc72 (clone: recursive and reference option triggers submodule alternates, 2016-08-17), further used in bf03b790471 (submodule--helper: set alternateLocation for cloned submodules, 2016-12-08).

I think it is the wrong approach though, as the test_must_fail only ensure that *something* doesn't return success. Usually we use it with "test_must_fail git ..." to ensure we properly error out on user requests, that are impossible to fulfill in the current implementation.

For this test case I'd suggest we rather test that the no alternate is setup,

    test_path_is_missing
super/.git/modules/sub-dissociate/objects/info/alternates

as that tests the correctness of the --dissociate option?

Thanks, Stefan