From: Slavica Djukic Date: Sat, 17 Nov 2018 18:47:53 GMT Subject: Re: [PATCH v2 1/2] [Outreachy] t3903-stash: test without configured user.name and user.email Message-ID: In-Reply-To: Hi Junio, On 16-Nov-18 11:12 AM, Junio C Hamano wrote: > Slavica Djukic writes: > >>>> + git var GIT_COMMITTER_IDENT >actual && >>>> + test_cmp expected actual && >>> I am not sure what you are testing with this step. There is nothing >>> that changed environment variables or configuration since we ran >>> "git var" above. Why does this test suspect that somebody in the >>> future may break the expectation that after running 'git add' and/or >>> 'git stash', our committer identity may have been changed, and how >>> would such a breakage happen? >> In previous re-roll, you suggested that test should be improved so >> that when >> reasonable identity is present, git stash executes under that >> identity, and not >> under the fallback one. > Yes, but for that you'd need to be checking the resulting commit > object that represents the stash entry. It should be created under > the substitute identity. Would it be correct to check it like this:         git reset &&         >1 &&         git add 1 &&         echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" >expect &&         git stash &&         git show -s --format="%an <%ae>" refs/stash >actual &&         test_cmp expect actual It is similar to your suggestion when there is no ident present. >> Here I'm just making sure that after calling >> git stash, >> we still have same reasonable identity present. > I do not think such a test would detect it, even when "git stash" > incorrectly used the fallback identity to create the stash entry. > > Thank you, Slavica