Re: [PATCH v3 1/2] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting

From: Alexey Shumkin <alex.crezoff@gmail.com>
Date: Sep 20, 2011, 20:46 UTC
Message-ID: <20110921004602.3d59bed4@zappedws>
In-Reply-To: <7vaa9z6n9p.fsf@alter.siamese.dyndns.org>

Show 11 quoted lines

> Alexey Shumkin <zapped@mail.ru> writes:
> 
> > diff --git a/t/t4041-diff-submodule-option.sh
> > b/t/t4041-diff-submodule-option.sh ...
> > @@ -48,28 +53,25 @@ head2=$(add_file sm1 foo3)
> >  
> >  test_expect_success 'modified submodule(forward)' "
> 
> As this is [PATCH 1/2], doesn't this patch make this test fail,
> calling for test_expect_failure here (and then later in 2/2 to be
> flipped back to test_expect_success)?

I did not know how is right. AFAIK, XP way tells to write failing tests and then patch/fix the code to pass them.

Here, As I understand now all tests must be successful The difference is in expectation of failure or success. Ok. I got it and I will reroll the patches.

Show 16 quoted lines

> 
> >  	git diff-index -p --submodule=log HEAD >actual &&
> > -	cat >expected <<-EOF &&
> > -Submodule sm1 $head1..$head2:
> > -  > Add foo3
> > -EOF
> > +	printf \"Submodule sm1 $head1..$head2:\n\
> > +  > Add foo3 ($added foo3)\n\
> > +\" > expected &&
> >  	test_cmp expected actual
> >  "
> 
> Hmmm... why printf?  Is it worth to force reviewers wonder what would
> happen if any of these $variables happened to have "%" in them?  Are
> you benefiting from any printf features that you cannot achieve with
> the original cat?

This is a remained from v1 patch code. It is not required now. I'll fix it. Thanks.

Show 5 quoted lines

> This hunk and others throughout your patch that change cat with here
> doc into printf seem to make the tests less legible, at least to me.
> 
> Perhaps like this instead, if the "flushed left" of the original
> looked ugly to your eyes?

No, the reason was not in "flushed left". And, BTW, this code generates expected output that has predefined format...

Show 14 quoted lines

> 
> @@ -49,27 +54,25 @@ head2=$(add_file sm1 foo3)
>  test_expect_success 'modified submodule(forward)' "
>  	git diff-index -p --submodule=log HEAD >actual &&
>  	cat >expected <<-EOF &&
> -Submodule sm1 $head1..$head2:
> -  > Add foo3
> -EOF
> +	Submodule sm1 $head1..$head2:
> +	  > Add foo3 ($added foo3)
> +	EOF
>  	test_cmp expected actual
>  "
>

-- 
С уважением, Алексей Шумкин
Программист компании 1С-Рарус
E-mail: ashu@rarus.ru
ICQ: 118001447
Jabber (GoogleTalk): ashu@rarus.ru
Skype: crezoff