git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Johannes Sixt <j.sixt@viscovery.net>,
	Shawn Bohrer <shawn.bohrer@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] More test cases for sanitized path names
Date: Thu, 31 Jan 2008 23:17:11 -0800	[thread overview]
Message-ID: <7vabmlb0y0.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <200802010534.55925.robin.rosenberg.lists@dewire.com> (Robin Rosenberg's message of "Fri, 1 Feb 2008 05:34:55 +0100")

Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:

> +test_expect_failure 'add a directory outside the work tree' '
> +	d1="$(cd .. ; pwd)" &&
> +	git add "$d1"
> +	echo $?
> +'

This test will always fail as the final exit status is that of
"echo", which will exit with success and you are expecting a
failure.

> +test_expect_failure 'add a file outside the work tree, nasty case 1' '(
> +	f="$(pwd)x" &&
> +	touch "$f" &&
> +	git add "$f"
> +)'

You are in the directory "t/trash", and try to add t/trashx, so
this should fail and you would want to make sure it fails.

But this has a few problems:

 * First, the obvious one.  You are creating a garbage file
   outside of t/trash directory.  Don't.  If you need to, dig a
   test directory one level lower inside t/trash and play around
   there.

 * In general you should stay away from test_expect_failure.  If
   any of the command in && chain fails, it fails the whole
   thing, but you cannot tell if the sequence failed at the
   command you expected to fail or something else that is much
   earlier.  For example, it may be that somebody created t/trashx
   file in the source tree that is read-only, and the comand
   that failed in the sequence could be 'touch' before the
   command you are testing.

   Instead, write it like (after fixing it not to go outside
   t/trash):

	test_expect_success 'add a path outside repo (1)' '

		file=path_to_outside_repo &&
                touch "$file" &&
		! git add "$f"

	'

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 ;-).

  reply	other threads:[~2008-02-01  7:18 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-23 15:14 git-clean buglet Johannes Sixt
2008-01-23 15:24 ` Johannes Sixt
2008-01-23 15:29 ` Johannes Schindelin
2008-01-23 15:40   ` Johannes Sixt
2008-01-27 19:55     ` [PATCH] Fix off by one error in prep_exclude Shawn Bohrer
2008-01-27 20:44       ` Johannes Schindelin
2008-01-27 21:15         ` Shawn Bohrer
2008-01-27 22:34         ` Junio C Hamano
2008-01-28  0:34           ` Shawn Bohrer
2008-01-28  0:37             ` Shawn Bohrer
2008-01-28 11:59               ` Johannes Schindelin
2008-01-28 12:04                 ` Junio C Hamano
2008-01-28  2:52             ` Junio C Hamano
2008-01-28  7:12               ` Johannes Sixt
2008-01-28  8:46                 ` Junio C Hamano
2008-01-28  9:05                   ` Johannes Sixt
2008-01-28  9:22                     ` Junio C Hamano
2008-01-28 12:33                     ` [RFH/PATCH] prefix_path(): disallow absolute paths Johannes Schindelin
2008-01-28 15:05                       ` [PATCH] " Johannes Schindelin
2008-01-29  1:23                       ` [RFH/PATCH] " Junio C Hamano
2008-01-29  2:03                         ` Junio C Hamano
2008-01-29  2:03                         ` Junio C Hamano
2008-01-29  7:02                           ` Junio C Hamano
2008-01-29  8:29                             ` [PATCH] setup: sanitize absolute and funny paths in get_pathspec() Junio C Hamano
2008-02-01  4:07                               ` [PATCH] Make blame accept absolute paths Robin Rosenberg
2008-02-01  4:34                               ` [PATCH] More test cases for sanitized path names Robin Rosenberg
2008-02-01  7:17                                 ` Junio C Hamano [this message]
2008-02-01  9:10                                   ` Robin Rosenberg
2008-02-01 10:22                                     ` Junio C Hamano
2008-02-01 10:51                                       ` Junio C Hamano
2008-02-01 11:10                                         ` Junio C Hamano
2008-02-01 14:17                                       ` Robin Rosenberg
2008-02-01 17:45                                         ` Junio C Hamano
2008-02-01  9:16                                   ` Karl Hasselström
2008-02-01  9:50                                   ` [PATCH for post 1.5.4] Sane use of test_expect_failure Junio C Hamano
2008-02-02 10:06                                     ` [PATCH] " Junio C Hamano
2008-03-07  8:23                                 ` [PATCH] More test cases for sanitized path names Junio C Hamano
2008-03-07 15:24                                   ` Robin Rosenberg
2008-01-29  2:37                         ` [RFH/PATCH] prefix_path(): disallow absolute paths Johannes Schindelin
2008-01-29  2:45                           ` Junio C Hamano
2008-01-29  2:59                             ` Johannes Schindelin
2008-01-29  7:20                         ` Johannes Sixt
2008-01-29  7:28                           ` Junio C Hamano
2008-01-29  7:43                             ` Johannes Sixt
2008-01-29  8:31                               ` Junio C Hamano
2008-01-29 21:53                       ` しらいしななこ
2008-01-30  0:43                         ` Junio C Hamano

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=7vabmlb0y0.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=j.sixt@viscovery.net \
    --cc=robin.rosenberg.lists@dewire.com \
    --cc=shawn.bohrer@gmail.com \
    /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).