Hey Stefan,

On Tue, Jan 3, 2017 at 11:28 PM, Stefan Beller <sbel...@google.com> wrote:
> On Mon, Jan 2, 2017 at 10:45 AM, Pranit Bauva <pranit.ba...@gmail.com> wrote:
>> The exit code of the upstream in a pipe is ignored thus we should avoid
>> using it.
>
> for commands under test, i.e. git things. Other parts can be piped if that 
> makes
> the test easier. Though I guess that can be guessed by the reader as well,
> as you only convert git commands on upstream pipes.
>
>> By writing out the output of the git command to a file, we can
>> test the exit codes of both the commands.
>>
>> Signed-off-by: Pranit Bauva <pranit.ba...@gmail.com>
>
> Thanks for taking ownership of this issue as well. :)

Welcome! ;)

>> ---
>>  t/t9813-git-p4-preserve-users.sh | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/t/t9813-git-p4-preserve-users.sh 
>> b/t/t9813-git-p4-preserve-users.sh
>> index 798bf2b67..9d7550ff3 100755
>> --- a/t/t9813-git-p4-preserve-users.sh
>> +++ b/t/t9813-git-p4-preserve-users.sh
>> @@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed 
>> authorship' '
>>                 make_change_by_user usernamefile3 Derek de...@example.com &&
>>                 P4EDITOR=cat P4USER=alice P4PASSWD=secret &&
>>                 export P4EDITOR P4USER P4PASSWD &&
>> -               git p4 commit |\
>> -               grep "git author de...@example.com does not match" &&
>> +               git p4 commit >actual 2>&1 &&
>
> Why do we need to pipe 2>&1 here?
> Originally the piping only fed the stdout to grep, so this patch changes the
> test? Maybe
>
>     2>actual.err &&
>     test_must_be_empty actual.err
>
> instead?

I tried this out but it seems that travis-ci build fails[1]. And I
don't have p4 on my machine to test what's happening actually. But I
just pushed out a few thing modifications to travis and it seems that
actual.err isn't really empty for some reason. So I think, I just
leave it as,

git p4 commit >actual &&
grep "git author de...@example.com does not match" actual &&

What do you think?

[1]: https://travis-ci.org/pranitbauva1997/git/jobs/188633734

Regards,
Pranit Bauva

Reply via email to