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

Re: [PATCH] t7406: correct test case for submodule-update initial population

From
Stefan Beller <sbeller@google.com>
Date
Mar 22, 2017, 22:07 UTC
Message-ID
<CAGZ79kbFjCQB_k8E1dMVop_EGF6TpnY04U1NV60VmSs1zHOnZw@mail.gmail.com>
In-Reply-To
<20170322215951.f4xbxehzvsd64c4y@sigill.intra.peff.net>
On Wed, Mar 22, 2017 at 2:59 PM, Jeff King <peff@peff.net> wrote:
Show 47 quoted lines
> On Wed, Mar 22, 2017 at 02:49:48PM -0700, Stefan Beller wrote:
>
>> * The syntax of the here-doc was wrong, such that the entire test was
>>   sucked into the here-doc, which is why the test succeeded successfully.
>
> As opposed to succeeding unsuccessfully? :)
>
>> * The variable $submodulesha1 was not expanded as it was inside a single
>>   quoted string. Use double quote to expand the variable.
>
> Hmm. Sort of. It was inside a non-interpolating here-doc inside a
> single-quoted string which was being eval'd. The second half is fine
> (the eval adds an extra layer of evaluation).
>
> Your fix:
>
>> +     cat >expect <<-\EOF &&
>> +     Execution of '\'"false $submodulesha1"\'' failed in submodule path '\''submodule'\''
>> +     EOF
>
> _does_ work, but it does so because it's evaluating $submodulesha1 in
> the shell snippet and handing the result off to test_expect_success to
> eval. So it would have problems if:
>
>   - that variable contained "\nEOF\n" itself ;)
>
>   - the variable was modified inside the shell snippet.
>
> Neither of those is true, but I think:
>
>   cat >expect <<-EOF &&
>   Execution of '\''false $submodulesha1'\'' failed in ...
>   EOF
>
> is safer and less surprising. The single-quote handling is unfortunate and
> ugly, but necessary to get them into the shell snippet in the first
> place. I notice the others tests in this script set up the expect file
> outside of a block. You could also do something like:
>
>   sq=\'
>   test_expect_success '...' '
>         cat >expect <<-EOF
>         Execution of ${sq}false $submodulesha1${sq} ...
>   '
>
> but I'm not sure if that is any more readable.
>

If I recall correctly, I made a big fuss about single quotes used correctly when writing that patch (which is why I may have lost track of the actual work there) to be told the one and only blessed way to use single quotes in our test suite.

Your proposal to use ${sq} sounds good to me, though we did not follow through with it for some other reason. I can reroll with that, though.

Thanks, Stefan

Previous: Jeff KingNext: Jeff King
Message 14 of 46 in “EOF test fixes (t5615/t7004)”
  1. Jan PalusMar 22, 2017
  2. Jan PalusMar 22, 2017
  3. Stefan BellerMar 22, 2017
  4. Junio C HamanoMar 22, 2017
  5. 0/3 fix "here-doc" syntax errorsJunio C Hamano, Mar 22, 2017
  6. 1/3 t5615: fix a here-doc syntax errorJunio C Hamano, Mar 22, 2017
  7. Jeff KingMar 22, 2017
  8. 2/3 t7406: fix here-doc syntax errorsJunio C Hamano, Mar 22, 2017
  9. Jeff KingMar 22, 2017
  10. Stefan BellerMar 22, 2017
  11. Jeff KingMar 22, 2017
  12. t7406: correct test case for submodule-update initial populationStefan Beller, Mar 22, 2017
  13. Jeff KingMar 22, 2017
  14. Stefan BellerMar 22, 2017
  15. Jeff KingMar 22, 2017
  16. Jonathan NiederMar 22, 2017
  17. Junio C HamanoMar 22, 2017
  18. Jeff KingMar 22, 2017
  19. Stefan BellerMar 22, 2017
  20. Junio C HamanoMar 22, 2017
  21. 3/3 t7004, t7030: fix here-doc syntax errorsJunio C Hamano, Mar 22, 2017
  22. Jeff KingMar 22, 2017
  23. Santiago TorresMar 22, 2017
  24. Jeff KingMar 22, 2017
  25. Junio C HamanoMar 22, 2017
  26. Santiago TorresMar 22, 2017
  27. Jeff KingMar 22, 2017
  28. Santiago TorresMar 22, 2017
  29. Jeff KingMar 22, 2017
  30. Junio C HamanoMar 22, 2017
  31. Santiago TorresMar 22, 2017
  32. Junio C HamanoMar 23, 2017
  33. Santiago TorresMar 23, 2017
  34. Jeff KingMar 23, 2017
  35. Junio C HamanoMar 24, 2017
  36. Jeff KingMar 24, 2017
  37. Junio C HamanoMar 24, 2017
  38. Jeff KingMar 24, 2017
  39. Jeff KingMar 24, 2017
  40. Junio C HamanoMar 22, 2017
  41. Jan PalusMar 22, 2017
  42. tests: lint for run-away here-docJunio C Hamano, Mar 23, 2017
  43. Junio C HamanoMar 23, 2017
  44. Jonathan NiederMar 24, 2017
  45. Junio C HamanoMar 24, 2017
  46. Jeff KingMar 24, 2017

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.