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
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