git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Lucien Kong <Lucien.Kong@ensimag.imag.fr>,
	git@vger.kernel.org, tboegi@web.de,
	Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>,
	Franck Jonas <Franck.Jonas@ensimag.imag.fr>,
	Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>,
	Huynh Khoi Nguyen Nguyen 
	<Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Subject: Re: [PATCHv4] rebase [-i --exec | -ix] <CMD>...
Date: Tue, 12 Jun 2012 20:55:43 +0200	[thread overview]
Message-ID: <4FD790AF.5070108@kdbg.org> (raw)
In-Reply-To: <7vpq9598yb.fsf@alter.siamese.dyndns.org>

Am 11.06.2012 17:14, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Am 10.06.2012 12:44, schrieb Lucien Kong:
>>> +test_expect_success 'rebase -i --exec without <CMD> shows error message and usage' '
>>> +	git reset --hard execute &&
>>> +	test_must_fail git rebase -i --exec 2>actual &&
>>> +	sed '1d' actual >tmp &&
>>> +	mv tmp actual &&
>>> +	test_must_fail git rebase -h >expected &&
>>> +	test_cmp expected actual &&
>>> +	git checkout master
>>> +'
>>
>> IMO, it is more important to check that the error message is present
>> rather than whether the usage blurb is correct. But since the error is
>> generated by the option parsing machinery, it is probably sufficient to
>> check *only* for failure, and don't verify the output at all.

I changed my mind. If option parsing regresses in such a way that --exec
suddenly works without command, then the 'test_must_fail git rebase ...'
could pass because there is a failure much later in the rebase process,
which we would not detect if we do not check the output. It is therefore
good to check whether the expected failure happens during option parsing.

Checking the usage blurb rather than the error message even has
advantages: it is immune to changes of the error messages and to i18n
poisoning.

In this light, the version you have queued is fine.

-- Hannes

  reply	other threads:[~2012-06-12 18:55 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-04 13:47 [PATCH] rebase [-i --exec | -ix] <CMD> Kong Lucien
2012-06-04 17:42 ` Junio C Hamano
2012-06-04 20:30   ` Matthieu Moy
2012-06-04 21:06     ` Junio C Hamano
2012-06-05 17:59   ` konglu
2012-06-05 18:13     ` Junio C Hamano
2012-06-04 17:48 ` Matthieu Moy
2012-06-06 10:34 ` [PATCHv2] " Lucien Kong
2012-06-06 20:03   ` Matthieu Moy
2012-06-06 22:54   ` Junio C Hamano
2012-06-07  8:25   ` Zbigniew Jędrzejewski-Szmek
2012-06-07  8:40   ` Johannes Sixt
2012-06-07 12:04     ` konglu
2012-06-07 13:43       ` Matthieu Moy
2012-06-08 14:53   ` [PATCHv3 1/2] git-rebase.txt: "--onto" option updated Lucien Kong
2012-06-08 14:53     ` [PATCHv3 2/2] rebase [-i --exec | -ix] <CMD> Lucien Kong
2012-06-08 17:02       ` Johannes Sixt
2012-06-08 18:56       ` Torsten Bögershausen
2012-06-08 19:15         ` konglu
2012-06-08 19:55           ` Torsten Bögershausen
2012-06-08 20:07             ` konglu
2012-06-08 20:51               ` Torsten Bögershausen
2012-06-08 21:03                 ` konglu
2012-06-09  6:14                   ` Torsten Bögershausen
2012-06-09  6:47                     ` konglu
2012-06-10 10:44       ` [PATCHv4] " Lucien Kong
2012-06-10 11:56         ` Johannes Sixt
2012-06-11 15:14           ` Junio C Hamano
2012-06-12 18:55             ` Johannes Sixt [this message]
2012-06-12 20:29               ` Junio C Hamano
2012-06-12  8:05         ` [PATCHv5] " Lucien Kong
2012-06-12  9:23           ` Zbigniew Jędrzejewski-Szmek
2012-06-12 14:46             ` Junio C Hamano
2012-06-13 14:04               ` Zbigniew Jędrzejewski-Szmek
2012-06-13 17:32                 ` Junio C Hamano
2012-06-13 18:05                 ` konglu
2012-06-13 18:22                   ` Junio C Hamano
2012-06-13 19:38                     ` konglu
2012-06-13 20:59                       ` Johannes Sixt
2012-06-13 21:07                         ` Zbigniew Jędrzejewski-Szmek
2012-06-13 22:25                         ` Junio C Hamano
2012-06-13 22:35                         ` Junio C Hamano
2012-06-13 22:43                           ` Zbigniew Jędrzejewski-Szmek
2012-06-14  6:57                             ` Matthieu Moy
2012-06-14 14:08                               ` Marc Branchaud
2012-06-08 15:00     ` [PATCHv3 1/2] git-rebase.txt: "--onto" option updated Matthieu Moy
2012-06-08 17:07     ` Junio C Hamano
2012-06-08 19:06       ` konglu
2012-06-08 19:52         ` Junio C Hamano
2012-06-08 20:08           ` konglu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FD790AF.5070108@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=Franck.Jonas@ensimag.imag.fr \
    --cc=Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr \
    --cc=Lucien.Kong@ensimag.imag.fr \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=Thomas.Nguy@ensimag.imag.fr \
    --cc=Valentin.Duperray@ensimag.imag.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).