Re: [PATCH v2 2/2] t9813: avoid using pipes

[Stefan Beller]

>
> git p4 commit >actual &&
> grep "git author de...@example.com does not match" actual &&
>
> What do you think?

>From the travis logs:

'actual.err' is not empty, it contains:
... - file(s) up-to-date.

I think(/hope) such a progress is tested for at another test,
and not relevant here so I'd think the proposed

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

is fine here.

Thanks,
Stefan

Re: [PATCH v2 2/2] t9813: avoid using pipes

[Pranit Bauva]

Hey Stefan,

On Tue, Jan 3, 2017 at 11:28 PM, Stefan Beller  wrote:
> On Mon, Jan 2, 2017 at 10:45 AM, Pranit Bauva  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 
>
> 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

Re: [PATCH v2 2/2] t9813: avoid using pipes

[Stefan Beller]

On Mon, Jan 2, 2017 at 10:45 AM, Pranit Bauva  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 

Thanks for taking ownership of this issue as well. :)

> ---
>  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?

Thanks,
Stefan

[PATCH v2 2/2] t9813: avoid using pipes

[Pranit Bauva]

The exit code of the upstream in a pipe is ignored thus we should avoid
using it. 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 
---
 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 &&
+   grep "git author de...@example.com does not match" actual &&
 
make_change_by_user usernamefile3 Charlie char...@example.com &&
-   git p4 commit |\
-   grep "git author char...@example.com does not match" &&
+   git p4 commit >actual 2>&1 &&
+   grep "git author char...@example.com does not match" actual &&
 
make_change_by_user usernamefile3 alice al...@example.com &&
git p4 commit >actual 2>&1 &&
-- 
2.11.0