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

Re: [PATCH v2 6/7] t5520: reduce commom lines of code

From
Johannes Sixt <j6t@kdbg.org>
Date
Apr 3, 2016, 06:46 UTC
Message-ID
<5700BC52.1040404@kdbg.org>
In-Reply-To
<CA+DCAeT4rF2dqL9iU9WYQJuxiSYstY9AaT2Hc7OvhmFNyKEgAg@mail.gmail.com>
Am 03.04.2016 um 08:24 schrieb Mehul Jain:
Show 22 quoted lines
> On Sun, Apr 3, 2016 at 12:20 AM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Am 02.04.2016 um 19:58 schrieb Mehul Jain:
>>> +for i in --autostash --no-autostash
>>> +do
>>> +       test_expect_success "pull $i (without --rebase) is illegal" '
>>> +               test_must_fail git pull $i . copy 2>err &&
>>> +               test_i18ngrep "only valid with --rebase" err
>>> +       '
>>> +done
>>
>>
>> Hm. If the implementation of test_expect_success uses the variable, too, its
>> value is lost when the test snippet runs. Fortunately, it does not.
>>
>> You can make this code a bit more robust by using double-quotes around the
>> test code so that $i is expanded before test_expect_success is evaluated.
>
> I think that the current format is preferred over the one you suggest.
> Here[1] Junio
> has given a descriptive explanation.
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/283350/focus=284769
Junio has a point there, of course.
In this light, I suggest that you use a more verbose variable name.
-- Hannes
Previous: Mehul JainNext: Mehul Jain
Message 12 of 20 in “t5520: tests for --[no-]autostash option”
  1. 0/7 t5520: tests for --[no-]autostash optionMehul Jain, Apr 2, 2016
  2. 1/7 t5520: use consistent capitalization in test titlesMehul Jain, Apr 2, 2016
  3. 2/7 t5520: ensure consistent test conditionsMehul Jain, Apr 2, 2016
  4. 3/7 t5520: use better test to check stderr outputMehul Jain, Apr 2, 2016
  5. 4/7 t5520: factor out common codeMehul Jain, Apr 2, 2016
  6. Eric SunshineApr 3, 2016
  7. 5/7 t5520: factor out common codeMehul Jain, Apr 2, 2016
  8. Eric SunshineApr 3, 2016
  9. 6/7 t5520: reduce commom lines of codeMehul Jain, Apr 2, 2016
  10. Johannes SixtApr 2, 2016
  11. Mehul JainApr 3, 2016
  12. Johannes SixtApr 3, 2016
  13. 7/7 t5520: test --[no-]autostash with pull.rebase=trueMehul Jain, Apr 2, 2016
  14. Eric SunshineApr 3, 2016
  15. 6/7 t5520: reduce commom lines of codeMehul Jain, Apr 3, 2016
  16. Eric SunshineApr 3, 2016
  17. Matthieu MoyApr 4, 2016
  18. Mehul JainApr 4, 2016
  19. Junio C HamanoApr 4, 2016
  20. Mehul JainApr 4, 2016

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.