From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junio C Hamano Subject: [PATCH for post 1.5.4] Sane use of test_expect_failure Date: Fri, 01 Feb 2008 01:50:53 -0800 Message-ID: <7vwspp9f9e.fsf_-_@gitster.siamese.dyndns.org> References: <47975FE6.4050709@viscovery.net> <7v7ihtqfm8.fsf@gitster.siamese.dyndns.org> <7vve5dox0o.fsf_-_@gitster.siamese.dyndns.org> <200802010534.55925.robin.rosenberg.lists@dewire.com> <7vabmlb0y0.fsf@gitster.siamese.dyndns.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii To: git@vger.kernel.org X-From: git-owner@vger.kernel.org Fri Feb 01 10:52:51 2008 Return-path: Envelope-to: gcvg-git-2@gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1JKsZl-0004mN-HA for gcvg-git-2@gmane.org; Fri, 01 Feb 2008 10:52:50 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758113AbYBAJv1 (ORCPT ); Fri, 1 Feb 2008 04:51:27 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757359AbYBAJv0 (ORCPT ); Fri, 1 Feb 2008 04:51:26 -0500 Received: from a-sasl-quonix.sasl.smtp.pobox.com ([208.72.237.25]:45970 "EHLO sasl.smtp.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757470AbYBAJvI (ORCPT ); Fri, 1 Feb 2008 04:51:08 -0500 Received: from a-sasl-quonix (localhost [127.0.0.1]) by a-sasl-quonix.pobox.com (Postfix) with ESMTP id E819F38C6; Fri, 1 Feb 2008 04:51:05 -0500 (EST) Received: from pobox.com (ip68-225-240-77.oc.oc.cox.net [68.225.240.77]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by a-sasl-quonix.pobox.com (Postfix) with ESMTP id 3F48C38C3; Fri, 1 Feb 2008 04:50:56 -0500 (EST) In-Reply-To: <7vabmlb0y0.fsf@gitster.siamese.dyndns.org> (Junio C. Hamano's message of "Thu, 31 Jan 2008 23:17:11 -0800") User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: Originally, test_expect_failure was designed to be the opposite of test_expect_success, but this was a bad decision. Most tests run a series of commands that leads to the single command that needs to be tested, like this: test_expect_{success,failure} 'test title' ' setup1 && setup2 && setup3 && what is to be tested ' And expecting a failure exit from the whole sequence misses the point of writing tests. Your setup$N that are supposed to succeed may have failed without even reaching what you are trying to test. The only valid use of test_expect_failure is to check a trivial single command that is expected to fail, which is a minority in tests of Porcelain-ish commands. This large-ish patch rewrites all uses of test_expect_failure to use test_expect_success and rewrites the condition of what is tested, like this: test_expect_success 'test title' ' setup1 && setup2 && setup3 && ! this command should fail ' test_expect_failure is redefined to serve as a reminder that that test *should* succeed but due to a known breakage in git it currently does not pass. So if git-foo command should create a file 'bar' but you discovered a bug that it doesn't, you can write a test like this: test_expect_failure 'git-foo should create bar' ' rm -f bar && git foo && test -f bar ' This construct acts similar to test_expect_success, but instead of reporting "ok/FAIL" like test_expect_success does, the outcome is reported as "FIXED/still broken". Signed-off-by: Junio C Hamano --- Junio C Hamano writes: > I'd like to make the _first_ patch after 1.5.4 to be a fix-up > for tests that misuse test_expect_failure. After that, we can > use test_expect_failure to mark tests that ought to pass but > don't because of bugs in the commands. That way, people who are > absolutely bored can grep for test_expect_failure to see what > existing issues to tackle ;-). This turned out to be a huge patch. I tried to be careful by keeping the conversion mostly mechanical, but I am not sure about some of the git-svn and git-cvsserver tests, some of which may already have been using test_expect_failure to mark a known breakage. Eyeballing by area experts are very much appreciated. t/README | 14 +-- t/t0000-basic.sh | 30 ++++-- t/t0030-stripspace.sh | 40 ++++---- t/t0040-parse-options.sh | 4 +- t/t1000-read-tree-m-3way.sh | 161 ++++++++++++++++------------- t/t1200-tutorial.sh | 8 +- t/t1300-repo-config.sh | 39 ++++--- t/t1302-repo-version.sh | 5 +- t/t1400-update-ref.sh | 24 ++-- t/t2000-checkout-cache-clash.sh | 4 +- t/t2002-checkout-cache-u.sh | 4 +- t/t2008-checkout-subdir.sh | 16 ++-- t/t2100-update-cache-badpath.sh | 4 +- t/t3020-ls-files-error-unmatch.sh | 4 +- t/t3200-branch.sh | 36 ++++--- t/t3210-pack-refs.sh | 26 +++--- t/t3400-rebase.sh | 4 +- t/t3403-rebase-skip.sh | 6 +- t/t3600-rm.sh | 17 ++-- t/t4103-apply-binary.sh | 68 +++++++------ t/t4113-apply-ending.sh | 8 +- t/t5300-pack-object.sh | 4 +- t/t5302-pack-index.sh | 32 +++--- t/t5401-update-hooks.sh | 8 +- t/t5402-post-merge-hook.sh | 4 +- t/t5500-fetch-pack.sh | 4 +- t/t5510-fetch.sh | 12 +- t/t5530-upload-pack-error.sh | 14 +-- t/t5600-clone-fail-cleanup.sh | 12 +- t/t5710-info-alternate.sh | 14 ++- t/t6023-merge-file.sh | 12 +- t/t6024-recursive-merge.sh | 2 +- t/t6025-merge-symlinks.sh | 21 ++-- t/t6101-rev-parse-parents.sh | 2 +- t/t6300-for-each-ref.sh | 8 +- t/t7001-mv.sh | 4 +- t/t7002-grep.sh | 4 +- t/t7004-tag.sh | 36 +++--- t/t7101-reset.sh | 24 ++-- t/t7501-commit.sh | 40 ++++---- t/t7503-pre-commit-hook.sh | 4 +- t/t7504-commit-msg-hook.sh | 8 +- t/t9100-git-svn-basic.sh | 28 +++--- t/t9106-git-svn-commit-diff-clobber.sh | 18 ++-- t/t9106-git-svn-dcommit-clobber-series.sh | 4 +- t/t9300-fast-import.sh | 24 ++-- t/t9400-git-cvsserver-server.sh | 30 +++-- t/test-lib.sh | 30 +++++- 48 files changed, 498 insertions(+), 427 deletions(-) diff --git a/t/README b/t/README index 36f2517..73ed11b 100644 --- a/t/README +++ b/t/README @@ -160,14 +160,12 @@ library for your script to use. - test_expect_failure