git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v8 8/8] add tests for `git_config_get_string_const()`

From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date
Aug 7, 2014, 08:30 UTC
Message-ID
<vpq38d8pupy.fsf@anie.imag.fr>
In-Reply-To
<xmqq4mxpmhyb.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 25 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Tanay Abhra <tanayabh@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/
Previous: Junio C HamanoNext: Matthieu Moy
Message 14 of 22 in “Rewrite `git_config()` using config-set API”
  1. 0/8 Rewrite `git_config()` using config-set APITanay Abhra, Aug 6, 2014
  2. 1/8 config.c: mark error and warnings strings for translationTanay Abhra, Aug 6, 2014
  3. 2/8 config.c: fix accuracy of line number in errorsTanay Abhra, Aug 6, 2014
  4. 3/8 add line number and file name info to `config_set`Tanay Abhra, Aug 6, 2014
  5. Ramsay JonesAug 6, 2014
  6. 4/8 change `git_config()` return value to voidTanay Abhra, Aug 6, 2014
  7. 5/8 config: add `git_die_config()` to the config-set APITanay Abhra, Aug 6, 2014
  8. 6/8 rewrite git_config() to use the config-set APITanay Abhra, Aug 6, 2014
  9. 7/8 add a test for semantic errors in config filesTanay Abhra, Aug 6, 2014
  10. 8/8 add tests for `git_config_get_string_const()`Tanay Abhra, Aug 6, 2014
  11. Matthieu MoyAug 6, 2014
  12. Tanay AbhraAug 6, 2014
  13. Junio C HamanoAug 6, 2014
  14. Matthieu MoyAug 7, 2014
  15. Matthieu MoyAug 6, 2014
  16. Junio C HamanoAug 7, 2014
  17. Matthieu MoyAug 7, 2014
  18. Junio C HamanoAug 7, 2014
  19. Tanay AbhraAug 8, 2014
  20. Ramsay JonesAug 8, 2014
  21. Junio C HamanoAug 10, 2014
  22. Ramsay JonesAug 11, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.