All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.