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

Re: [PATCH v2 2/4] t7610: make tests more independent and debuggable

From
Stefan Beller <sbeller@google.com>
Date
Jan 6, 2017, 01:31 UTC
Message-ID
<CAGZ79kbRee+3MbAHCSFB0QqGMMF5bcZMiEHV-coRh87vFfq0Ag@mail.gmail.com>
In-Reply-To
<20170106010945.79382-3-hansenr@google.com>
On Thu, Jan 5, 2017 at 5:09 PM, Richard Hansen <hansenr@google.com> wrote:
> Reduce how much a test can interfere other tests:

A bullet point list as an unordered list often indicates that you're doing multiple things at once, possibly unrelated, so they could go into different patches. ;)

While telling you to make even more commits: you may also want to make a patch with an entry to the .mailmap file (assuming you're the same Richard Hansen that contributed from rhansen@bbn.com); Welcome to Google!

Show 16 quoted lines
>
>   * Move setup code that multiple tests depend on to the 'setup' test
>     case.
>   * Run 'git reset --hard' after every test (pass or fail) to clean up
>     in case the test fails and leaves the repository in a strange
>     state.
>   * If the repository must be in a particular state (beyond what is
>     already done by the 'setup' test case) before the test can run,
>     make the necessary repository changes in the test script even if
>     it means duplicating some lines of code from the previous test
>     case.
>   * Never assume that a particular commit is checked out.
>   * Always work on a test-specific branch when the test might create a
>     commit.  This is not always necessary for correctness, but it
>     improves debuggability by ensuring a commit created by test #N
>     shows up on the testN branch, not the branch for test #N-1.
Show 12 quoted lines
> @@ -112,6 +146,7 @@ test_expect_success 'custom mergetool' '
>  '
>
>  test_expect_success 'mergetool crlf' '
> +       test_when_finished "git reset --hard" &&
>         test_config core.autocrlf true &&
>         git checkout -b test$test_count branch1 &&
>         test_must_fail git merge master >/dev/null 2>&1 &&
> @@ -129,11 +164,11 @@ test_expect_success 'mergetool crlf' '
>         git submodule update -N &&
>         test "$(cat submod/bar)" = "master submodule" &&
>         git commit -m "branch1 resolved with mergetool - autocrlf" &&
> -       test_config core.autocrlf false &&
> -       git reset --hard
> +       test_config core.autocrlf false
>  '

This is the nit that led me to writing this email in the first place: test_config is a function that sets a configuration for a single test only, so it makes no sense as the last statement of a test. (In its implementation it un-configures with test_when_finished)

So I think we do not want to add it back, but rather remove this test_config statement.

But to do this we need to actually be careful with the order of the newly added test_when_finished "git reset --hard" and test_config core.autocrlf true, which uses test_when_finished internally.

The order seems correct to me, as the reset would be executed after the "test_config core.autocrlf true" is un-configured.

Previous: Richard HansenNext: Richard Hansen
Message 18 of 76 in “fix mergetool+rerere+subdir regression”
  1. 0/4 fix mergetool+rerere+subdir regressionRichard Hansen, Jan 4, 2017
  2. 2/4 t7610: make tests more independent and debuggableRichard Hansen, Jan 4, 2017
  3. Stefan BellerJan 4, 2017
  4. Richard HansenJan 5, 2017
  5. Richard HansenJan 5, 2017
  6. Simon RuderichJan 5, 2017
  7. Richard HansenJan 5, 2017
  8. 3/4 t7610: add test case for rerere+mergetool+subdir bugRichard Hansen, Jan 4, 2017
  9. 1/4 t7610: update branch names to match test numberRichard Hansen, Jan 4, 2017
  10. 4/4 mergetool: fix running in subdir when rerere enabledRichard Hansen, Jan 4, 2017
  11. 0/4 fix mergetool+rerere+subdir regressionRichard Hansen, Jan 6, 2017
  12. 4/4 mergetool: fix running in subdir when rerere enabledRichard Hansen, Jan 6, 2017
  13. Johannes SixtJan 6, 2017
  14. Richard HansenJan 7, 2017
  15. 3/4 t7610: add test case for rerere+mergetool+subdir bugRichard Hansen, Jan 6, 2017
  16. 1/4 t7610: update branch names to match test numberRichard Hansen, Jan 6, 2017
  17. 2/4 t7610: make tests more independent and debuggableRichard Hansen, Jan 6, 2017
  18. Stefan BellerJan 6, 2017
  19. Richard HansenJan 7, 2017
  20. 00/13 fix mergetool+rerere+subdir regressionRichard Hansen, Jan 9, 2017
  21. 01/13 .mailmap: Use my personal email address as my canonicalRichard Hansen, Jan 9, 2017
  22. 05/13 t7610: don't rely on state from previous testRichard Hansen, Jan 9, 2017
  23. 06/13 t7610: run 'git reset --hard' after each test to clean upRichard Hansen, Jan 9, 2017
  24. 03/13 t7610: Move setup code to the 'setup' test case.Richard Hansen, Jan 9, 2017
  25. 04/13 t7610: use test_when_finished for cleanup tasksRichard Hansen, Jan 9, 2017
  26. 07/13 t7610: delete some now-unnecessary 'git reset --hard' linesRichard Hansen, Jan 9, 2017
  27. 08/13 t7610: always work on a test-specific branchRichard Hansen, Jan 9, 2017
  28. 13/13 mergetool: fix running in subdir when rerere enabledRichard Hansen, Jan 9, 2017
  29. Junio C HamanoJan 9, 2017
  30. Junio C HamanoJan 9, 2017
  31. Johannes SixtJan 9, 2017
  32. Richard HansenJan 9, 2017
  33. Junio C HamanoJan 9, 2017
  34. Junio C HamanoJan 9, 2017
  35. Richard HansenJan 9, 2017
  36. 09/13 t7610: don't assume the checked-out commitRichard Hansen, Jan 9, 2017
  37. 12/13 mergetool: take the "-O" out of $orderfileRichard Hansen, Jan 9, 2017
  38. 11/13 t7610: add test case for rerere+mergetool+subdir bugRichard Hansen, Jan 9, 2017
  39. 10/13 t7610: spell 'git reset --hard' consistentlyRichard Hansen, Jan 9, 2017
  40. 02/13 t7610: update branch names to match test numberRichard Hansen, Jan 9, 2017
  41. Stefan BellerJan 9, 2017
  42. 00/14 fix mergetool+rerere+subdir regressionRichard Hansen, Jan 9, 2017
  43. 01/14 .mailmap: Use my personal email address as my canonicalRichard Hansen, Jan 9, 2017
  44. 02/14 rev-parse doc: use "--" in the --prefix exampleRichard Hansen, Jan 9, 2017
  45. 04/14 t7610: Move setup code to the 'setup' test case.Richard Hansen, Jan 9, 2017
  46. 05/14 t7610: use test_when_finished for cleanup tasksRichard Hansen, Jan 9, 2017
  47. 08/14 t7610: delete some now-unnecessary 'git reset --hard' linesRichard Hansen, Jan 9, 2017
  48. 10/14 t7610: don't assume the checked-out commitRichard Hansen, Jan 9, 2017
  49. 13/14 mergetool: take the "-O" out of $orderfileRichard Hansen, Jan 9, 2017
  50. 11/14 t7610: spell 'git reset --hard' consistentlyRichard Hansen, Jan 9, 2017
  51. 14/14 mergetool: fix running in subdir when rerere enabledRichard Hansen, Jan 9, 2017
  52. Johannes SixtJan 10, 2017
  53. Richard HansenJan 10, 2017
  54. Junio C HamanoJan 10, 2017
  55. Johannes SixtJan 10, 2017
  56. 12/14 t7610: add test case for rerere+mergetool+subdir bugRichard Hansen, Jan 9, 2017
  57. 09/14 t7610: always work on a test-specific branchRichard Hansen, Jan 9, 2017
  58. 07/14 t7610: run 'git reset --hard' after each test to clean upRichard Hansen, Jan 9, 2017
  59. 06/14 t7610: don't rely on state from previous testRichard Hansen, Jan 9, 2017
  60. 03/14 t7610: update branch names to match test numberRichard Hansen, Jan 9, 2017
  61. 00/14 fix mergetool+rerere+subdir regressionRichard Hansen, Jan 10, 2017
  62. 01/14 .mailmap: Use my personal email address as my canonicalRichard Hansen, Jan 10, 2017
  63. 09/14 t7610: always work on a test-specific branchRichard Hansen, Jan 10, 2017
  64. 11/14 t7610: spell 'git reset --hard' consistentlyRichard Hansen, Jan 10, 2017
  65. 10/14 t7610: don't assume the checked-out commitRichard Hansen, Jan 10, 2017
  66. 12/14 t7610: add test case for rerere+mergetool+subdir bugRichard Hansen, Jan 10, 2017
  67. 13/14 mergetool: take the "-O" out of $orderfileRichard Hansen, Jan 10, 2017
  68. 14/14 mergetool: fix running in subdir when rerere enabledRichard Hansen, Jan 10, 2017
  69. 08/14 t7610: delete some now-unnecessary 'git reset --hard' linesRichard Hansen, Jan 10, 2017
  70. 07/14 t7610: run 'git reset --hard' after each test to clean upRichard Hansen, Jan 10, 2017
  71. 06/14 t7610: don't rely on state from previous testRichard Hansen, Jan 10, 2017
  72. 05/14 t7610: use test_when_finished for cleanup tasksRichard Hansen, Jan 10, 2017
  73. 04/14 t7610: Move setup code to the 'setup' test case.Richard Hansen, Jan 10, 2017
  74. 03/14 t7610: update branch names to match test numberRichard Hansen, Jan 10, 2017
  75. 02/14 rev-parse doc: pass "--" to rev-parse in the --prefix exampleRichard Hansen, Jan 10, 2017
  76. David AguilarJan 10, 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.