Re: [PATCH v8 8/8] add tests for `git_config_get_string_const()`
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: ... + grep line 7.*.git/config\|.git/config.*line 7 result +' This is still dependant on the locale (line is translated). You need to use test_i18ngrep instead of grep here (see its definition and comment in t/test-lib.sh). I don't think you need these two alternatives OTOH. BTW, Junio, I don't understand your remark This test is too tight (the full string) in the previous iteration. Can you elaborate? The original test was trying to match the string fully, i.e. +grep fatal: bad config variable '\''alias.br'\'' at file line 2 in .git/config result As I already was feeling funny about seeing the phrase file line in the message and expecting it to be corrected, I thought I should encourage a check that does not depend on minor phrasing changes, if it can be done without bending backwards. OK. I personally prefer tight tests in this case, as the patch doing the rephrase sees what changed by running the tests, and documents the visible change by changing the expected in the tests scripts. But no strong opinion here, I'd be fine with e.g. test_i18ngrep fatal: .* alias.br.*line 2 in .git/config result -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
[PATCH v8 8/8] add tests for `git_config_get_string_const()`
Add tests for `git_config_get_string_const()`, check whether it dies printing the line number and the file name if a NULL value is retrieved for the given key. Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1308-config-set.sh | 10 ++ test-config.c | 10 ++ 2 files changed, 20 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 5ea0aa4..ecc757a 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -119,6 +119,16 @@ test_expect_success 'find integer value for a key' ' check_config get_int lamb.chop 65 ' +test_expect_success 'find string value for a key' ' + check_config get_string case.baz hask + check_config expect_code 1 get_string case.ba Value not found for \case.ba\ +' + +test_expect_success 'check line error when NULL string is queried' ' + test_expect_code 128 test-config get_string case.foo 2result + grep line 7.*.git/config\|.git/config.*line 7 result +' + test_expect_success 'find integer if value is non parse-able' ' check_config expect_code 128 get_int lamb.head ' diff --git a/test-config.c b/test-config.c index 9dd1b22..6a77552 100644 --- a/test-config.c +++ b/test-config.c @@ -16,6 +16,8 @@ * * get_bool - print bool value for the entered key or die * + * get_string - print string value for the entered key or die + * * configset_get_value - returns value with the highest priority for the entered key * from a config_set constructed from files entered as arguments. * @@ -84,6 +86,14 @@ int main(int argc, char **argv) printf(Value not found for \%s\\n, argv[2]); goto exit1; } + } else if (argc == 3 !strcmp(argv[1], get_string)) { + if (!git_config_get_string_const(argv[2], v)) { + printf(%s\n, v); + goto exit0; + } else { + printf(Value not found for \%s\\n, argv[2]); + goto exit1; + } } else if (!strcmp(argv[1], configset_get_value)) { for (i = 3; i argc; i++) { int err; -- 1.9.0.GIT -- 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 v8 8/8] add tests for `git_config_get_string_const()`
Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'find string value for a key' ' + check_config get_string case.baz hask + check_config expect_code 1 get_string case.ba Value not found for \case.ba\ +' + +test_expect_success 'check line error when NULL string is queried' ' + test_expect_code 128 test-config get_string case.foo 2result + grep line 7.*.git/config\|.git/config.*line 7 result +' This is still dependant on the locale (line is translated). You need to use test_i18ngrep instead of grep here (see its definition and comment in t/test-lib.sh). I don't think you need these two alternatives OTOH. BTW, Junio, I don't understand your remark This test is too tight (the full string) in the previous iteration. Can you elaborate? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v8 8/8] add tests for `git_config_get_string_const()`
On 8/6/2014 9:02 PM, Matthieu Moy wrote: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'find string value for a key' ' +check_config get_string case.baz hask +check_config expect_code 1 get_string case.ba Value not found for \case.ba\ +' + +test_expect_success 'check line error when NULL string is queried' ' +test_expect_code 128 test-config get_string case.foo 2result +grep line 7.*.git/config\|.git/config.*line 7 result +' This is still dependant on the locale (line is translated). You need to use test_i18ngrep instead of grep here (see its definition and comment in t/test-lib.sh). Oh, and I was searching t/README for a hint. Maybe we should add a line there to hint future readers. Thanks. :) I don't think you need these two alternatives OTOH. BTW, Junio, I don't understand your remark This test is too tight (the full string) in the previous iteration. Can you elaborate? I think he meant we must search for the relevant snippets instead of the whole string. -- 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 v8 8/8] add tests for `git_config_get_string_const()`
Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: ... +grep line 7.*.git/config\|.git/config.*line 7 result +' This is still dependant on the locale (line is translated). You need to use test_i18ngrep instead of grep here (see its definition and comment in t/test-lib.sh). I don't think you need these two alternatives OTOH. BTW, Junio, I don't understand your remark This test is too tight (the full string) in the previous iteration. Can you elaborate? The original test was trying to match the string fully, i.e. + grep fatal: bad config variable '\''alias.br'\'' at file line 2 in .git/config result As I already was feeling funny about seeing the phrase file line in the message and expecting it to be corrected, I thought I should encourage a check that does not depend on minor phrasing changes, if it can be done without bending backwards. I do agree with you that using \| in grep a pattern to trigger ERE Alternation counts as bending backwards as that is a GNU extension and not portable. -- 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