Re: [PATCH v14 01/16] test: add test cases for relative_path
2013/6/25 Junio C Hamano gits...@pobox.com: Add prefix _ to workaround the absolute path rewritten issue in msysGit is interesting, but these test cases have already been tested in Linux, right? The most important thing is what we want to test in these tests. The special test program is to try running the underlying relative_path() by driving it directly, but the only real caller is in setup_work_tree(), where two return values from real_path() is compared. On POSIX systems, we know we are feeding paths that both begin with /. Now, on Windows systems, what do we get back from real_path()? C:\git\Documentation? /git/Documentation? Confirm that on Windows real_path() returns absolute path in Windows style, such as: C:/msysgit/git/.git Unlike relative_path() used in setup.c:setup_work_tree(), for path_relative() from quote.c, IIRC, the expected inputs are both full pathnames within the working tree. A typical question the callers to this function asks is like The current directory obtained from prefix is the Documentation/ directory and we need to show that compat/mkdir.c is modified, relative to the current directory. ../compat/mkdir.c is what I want to show. So in that sense, it does not matter if /a/b/c is given as /a/b/c or C:\a\b\c as we do not care the leading common part (either / or C:\) very much. On the other hand, the test vector you preoared in the first test that all begin with / may not be very useful to make sure that the function behaves the same way before and after your rewrite. Yes, I should add more test cases without the leading '/'. -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 01/16] test: add test cases for relative_path
Jiang Xin worldhello@gmail.com writes:
diff --git a/test-path-utils.c b/test-path-utils.c
index 0092cb..dcc530 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -28,6 +28,19 @@ static int normalize_ceiling_entry(struct string_list_item
*item, void *unused)
return 1;
}
+static void normalize_argv_string(const char **var, const char *input)
+{
+ if (!strcmp(input, null))
+ *var = NULL;
+ else if (!strcmp(input, empty))
+ *var = ;
+ else
+ *var = input;
+
+ if (*var (**var == '' || **var == '('))
+ die(Bad value: %s\n, input);
+}
+
If you have to munge the input string like this anyway, perhaps you
can work around the command line mangling done by Windows bash
runtime, perhaps add something like:
if (*input == '_')
input++;
and then protecting the path with the underscore, like so?
relative_path _/a/b/c/ _/a/b/ c/
Wouldn't that let you avoid having to handle POSIX prereq for
these paths?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 01/16] test: add test cases for relative_path
2013/6/25 Junio C Hamano gits...@pobox.com:
Jiang Xin worldhello@gmail.com writes:
diff --git a/test-path-utils.c b/test-path-utils.c
index 0092cb..dcc530 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -28,6 +28,19 @@ static int normalize_ceiling_entry(struct
string_list_item *item, void *unused)
return 1;
}
+static void normalize_argv_string(const char **var, const char *input)
+{
+ if (!strcmp(input, null))
+ *var = NULL;
+ else if (!strcmp(input, empty))
+ *var = ;
+ else
+ *var = input;
+
+ if (*var (**var == '' || **var == '('))
+ die(Bad value: %s\n, input);
+}
+
If you have to munge the input string like this anyway, perhaps you
can work around the command line mangling done by Windows bash
runtime, perhaps add something like:
if (*input == '_')
input++;
and then protecting the path with the underscore, like so?
relative_path _/a/b/c/ _/a/b/ c/
Wouldn't that let you avoid having to handle POSIX prereq for
these paths?
In order to test NULL pointer in t/t0060, I have to write normalize_argv_string
to convert null to NULL. So that I can write test case in
t/t0060-path-utils.sh like this:
relative_path null null./
And for the same reason, output would be (null) for NULL pointer.
Use (null) not null for output, because I want to make sure the
conversion must happen for input and output.
I found it would be nice to wrap input empty string as empty and
wrap output of empty string as (empty) for the readability of both
input and output of test cases.
Add prefix _ to workaround the absolute path rewritten issue in
msysGit is interesting, but these test cases have already been
tested in Linux, right? Patch 16/16 turns on most of the test cases
which could only run under POSIX previously, and test these test
cases in Windows way.
--
Jiang Xin
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 01/16] test: add test cases for relative_path
Jiang Xin worldhello@gmail.com writes: In order to test NULL pointer in t/t0060, I have to write normalize_argv_string to convert null to NULL Yes, null and empty I already understand. Add prefix _ to workaround the absolute path rewritten issue in msysGit is interesting, but these test cases have already been tested in Linux, right? The most important thing is what we want to test in these tests. The special test program is to try running the underlying relative_path() by driving it directly, but the only real caller is in setup_work_tree(), where two return values from real_path() is compared. On POSIX systems, we know we are feeding paths that both begin with /. Now, on Windows systems, what do we get back from real_path()? C:\git\Documentation? /git/Documentation? If the former, then writing /a/b/c in the test input and letting Windows bash convert it to C:\a\b\c before it calls the test-path-utils would feed what we expect to see in the real caller, which would be a more meaningful test than what I suggested (i.e. to feed _/a/b/c to test-path-utils (unmolested by Windows bash), strip _ to feed /a/b/c to underlying relative_path()). Unlike relative_path() used in setup.c:setup_work_tree(), for path_relative() from quote.c, IIRC, the expected inputs are both full pathnames within the working tree. A typical question the callers to this function asks is like The current directory obtained from prefix is the Documentation/ directory and we need to show that compat/mkdir.c is modified, relative to the current directory. ../compat/mkdir.c is what I want to show. So in that sense, it does not matter if /a/b/c is given as /a/b/c or C:\a\b\c as we do not care the leading common part (either / or C:\) very much. On the other hand, the test vector you preoared in the first test that all begin with / may not be very useful to make sure that the function behaves the same way before and after your rewrite. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
