git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 4/5] Test and fix normalize_path_copy()
Date: Sun, 08 Feb 2009 15:46:00 +0100	[thread overview]
Message-ID: <498EF028.3040702@lsrfire.ath.cx> (raw)
In-Reply-To: <1234019311-6449-5-git-send-email-j6t@kdbg.org>

Johannes Sixt schrieb:
> Moreover, the test case descriptions of t0060 now include the test data and
> expected outcome.

The test still fails in a lot of cases for me, because "/" is translated to
the installation path of msys.  E.g. with the patch at the end, which shows
the results in addition to input and expected output, I get several cases
like this (the first test call checks the return code):

* FAIL 2: normalize absolute: '/' => '/'
	test 0 = 0 && test 'C:/Program Files (x86)/msysgit/msysgit' = '/'

* FAIL 32: longest ancestor: '/foo' '/' => '0'
	test 0 = 0 && test '38' = '0'

I'm not sure what to do about it.  Should test-path-utils translate it back
into "root is / is root"?  Should the test script look up "/" at the top
and just append this value to all paths?  Unpretty.


And then there's this:

* FAIL 7: normalize absolute: '/./..' => '++failed++'
	test 0 = 0 && test 'C:/Program Files (x86)/msysgit/msysgit' = '++failed++'
*   ok 8: normalize absolute: '/../.' => '++failed++'

A printf() added to the top of normalize_path_copy() tells me that you can
add as many "/.." as you want after a path starting with "/.", it will
always translate into the install directory of msys regardless.


Anyway, here's the patch mentioned above:

diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 4ed1f0b..d5a38a6 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -8,13 +8,17 @@ test_description='Test various path utilities'
 . ./test-lib.sh
 
 norm_abs() {
-	test_expect_success "normalize absolute: $1 => $2" \
-	"test \"\$(test-path-utils normalize_path_copy '$1')\" = '$2'"
+	result=$(test-path-utils normalize_path_copy "$1")
+	rc=$?
+	test_expect_success "normalize absolute: '$1' => '$2'" \
+	"test $rc = 0 && test '$result' = '$2'"
 }
 
 ancestor() {
-	test_expect_success "longest ancestor: $1 $2 => $3" \
-	"test \"\$(test-path-utils longest_ancestor_length '$1' '$2')\" = '$3'"
+	result=$(test-path-utils longest_ancestor_length "$1" "$2")
+	rc=$?
+	test_expect_success "longest ancestor: '$1' '$2' => '$3'" \
+	"test $rc = 0 && test '$result' = '$3'"
 }
 
 norm_abs "" ""

  parent reply	other threads:[~2009-02-08 14:54 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-04 23:00 [PATCH] fix crash in path.c on Windows René Scharfe
2009-02-04 23:51 ` Junio C Hamano
2009-02-05  7:57 ` Johannes Sixt
2009-02-05 16:48   ` René Scharfe
2009-02-05 17:13     ` Johannes Sixt
2009-02-05 20:41     ` Robin Rosenberg
2009-02-05 19:35 ` [PATCH] fix t1504 " René Scharfe
2009-02-06 12:55   ` Johannes Sixt
2009-02-06 13:11     ` Johannes Schindelin
2009-02-06 13:17       ` Johannes Sixt
2009-02-06 13:26         ` Johannes Schindelin
2009-02-06 13:36           ` Johannes Sixt
2009-02-06 17:18     ` René Scharfe
2009-02-06 19:23       ` Johannes Sixt
2009-02-06 21:45         ` René Scharfe
2009-02-07 15:08           ` [PATCH 0/5] Consolidate path normalization functions Johannes Sixt
2009-02-07 15:08             ` [PATCH 1/5] Make test-path-utils more robust against incorrect use Johannes Sixt
2009-02-07 15:08               ` [PATCH 2/5] Move sanitary_path_copy() to path.c and rename it to normalize_path_copy() Johannes Sixt
2009-02-07 15:08                 ` [PATCH 3/5] Fix GIT_CEILING_DIRECTORIES on Windows Johannes Sixt
2009-02-07 15:08                   ` [PATCH 4/5] Test and fix normalize_path_copy() Johannes Sixt
2009-02-07 15:08                     ` [PATCH 5/5] Remove unused normalize_absolute_path() Johannes Sixt
2009-02-08  0:08                     ` [PATCH 4/5] Test and fix normalize_path_copy() Robin Rosenberg
2009-02-08  8:52                       ` Johannes Sixt
2009-02-08 14:46                     ` René Scharfe [this message]
2009-02-08 15:50                       ` Johannes Sixt
2009-02-07  0:25     ` [PATCH] fix t1504 on Windows René Scharfe

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=498EF028.3040702@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /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).