On Fri, Jun 01 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <ava...@gmail.com> writes:
>
>> Assert that whenever there's a DWIM checkout that the index should be
>> clean afterwards, in addition to the correct branch being checked-out.
>> ...
>> So let's amend the tests (mostly added in) 399e4a1c56 ("t2024: Add
>> tests verifying current DWIM behavior of 'git checkout <branch>'",
>> 2013-04-21) to always assert that "status" is clean after we run
>> "checkout", that's being done with "-uno" because there's going to be
>> some untracked files related to the test itself which we don't care
>> about.
>
> It might not be absolutely necessary to state, but it would be
> helpful to say that you are assuming to start a checkout (DWIM or
> otherwise) from a clean state; without the assumption, the readers
> need to think for a few breaths why "the index should be clean" is
> true.
>
> The intention and the implementation of the change both mostly look
> good to me from a quick read.

Makes sense, will fix.

>>  test_expect_success 'setup' '
>>      test_commit my_master &&
>>      git init repo_a &&
>> @@ -55,6 +61,7 @@ test_expect_success 'checkout of non-existing branch 
>> fails' '
>>      test_might_fail git branch -D xyzzy &&
>>
>>      test_must_fail git checkout xyzzy &&
>> +    status_uno_is_clean &&
>>      test_must_fail git rev-parse --verify refs/heads/xyzzy &&
>>      test_branch master
>>  '
>> @@ -64,8 +71,10 @@ test_expect_success 'checkout of branch from multiple 
>> remotes fails #1' '
>>      test_might_fail git branch -D foo &&
>>
>>      test_must_fail git checkout foo &&
>> +    status_uno_is_clean &&
>>      test_must_fail git rev-parse --verify refs/heads/foo &&
>> -    test_branch master
>> +    test_branch master &&
>> +    status_uno_is_clean
>
> Hmm, what's the point of this second one?
>
>>  '

Slipped in, will remove. Thanks.

Reply via email to