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

Re: [PATCH] Several tests: cd inside subshell instead of around

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 7, 2010, 05:08 UTC
Message-ID
<7vlj7ep2ch.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20100907023713.GJ1182@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 9 quoted lines
> Junio C Hamano wrote:
>
>> If we were to insist that no matter how an individual test fail, the test
>> that follows it must start in a known location (namely, $TRASH), then we
>> might want to use something like the attached patch.
>
> I like it.  Affected test scripts:
> ... (many) ...
> and probably some others.

For the record, I _do not_ like it at all. Is it worth being that draconian and say each test must start at $TRASH? What do we gain by it?

We certainly do _not_ want any test to wander around, escape out of the $TRASH directory and running random "git" command, but as long as N+1th test knows and expects Nth test might move to a subdirectory of $TRASH and is prepared to start, I personally do not think it is such a big deal. It certainly makes it harder to insert a new test between such a pair of tests, but like it or not, many of the tests already depend on the state of the repository that earlier tests left behind. I think we could just declare that $cwd is just a small part of that initial state for individual test that was left behind the earlier test.

After we vet and apply Jens and your "turn 'cd there && do this && cd ..' into a subshell" series, and the "modernize indentation style" series, perhaps we should stop and think? In an ideal world, it would be really nice if each "test_expect_success/failure" in a single script were independent from each other (if we did so, GIT_SKIP_TESTS=4321.4 starts to be more useful), but realistically, I don't think it is worth the effort.

So it may be worth it to check $(pwd) against $TRASH and make sure we didn't escape $TRASH by mistake at the beginning of each test in test_run_ function, but to me, "must be at $TRASH" feels like a rule for the sake of it, without really helping us very much.

Previous: Jonathan NiederNext: Jonathan Nieder
Message 25 of 27 in “Several tests: cd inside subshell instead of around”
  1. Several tests: cd inside subshell instead of aroundJens Lehmann, Sep 6, 2010
  2. Jonathan NiederSep 6, 2010
  3. Jens LehmannSep 6, 2010
  4. 0/7 Re: [PATCH] Several tests: cd inside subshell instead of aroundJonathan Nieder, Sep 7, 2010
  5. 1/7 tests: subshell indentation stylefixJonathan Nieder, Sep 7, 2010
  6. Jonathan NiederSep 7, 2010
  7. 2/7 t1450 (fsck): remove dangling objectsJonathan Nieder, Sep 7, 2010
  8. 3/7 t2105 (gitfile): add missing &&Jonathan Nieder, Sep 7, 2010
  9. Brad KingSep 7, 2010
  10. 4/7 t0004 (unwritable files): simplify error handlingJonathan Nieder, Sep 7, 2010
  11. 5/7 t1302 (core.repositoryversion): style tweaksJonathan Nieder, Sep 7, 2010
  12. Nguyen Thai Ngoc DuySep 7, 2010
  13. 6/7 t1303 (config): style tweaksJonathan Nieder, Sep 7, 2010
  14. Jeff KingSep 7, 2010
  15. Junio C HamanoSep 7, 2010
  16. Jonathan NiederSep 7, 2010
  17. guarding everything with test_expect_success (Re: [PATCH 6/7] t1303 (config): style tweaks)Jonathan Nieder, Sep 7, 2010
  18. Jeff KingSep 7, 2010
  19. Jonathan NiederSep 7, 2010
  20. 7/7 t2016 (checkout -p): use printf for multiline y/n inputJonathan Nieder, Sep 7, 2010
  21. Thomas RastSep 7, 2010
  22. Jonathan NiederSep 7, 2010
  23. Junio C HamanoSep 6, 2010
  24. Jonathan NiederSep 7, 2010
  25. Junio C HamanoSep 7, 2010
  26. Jonathan NiederSep 7, 2010
  27. t1020: Get rid of 'cd "$HERE"' at the start of each testJens Lehmann, Sep 7, 2010

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.