From: Junio C Hamano <gitster@pobox.com>
To: Michael Rappazzo <rappazzo@gmail.com>
Cc: git@vger.kernel.org, pclouds@gmail.com, szeder@ira.uka.de
Subject: Re: [PATCH v3 1/2] rev-parse tests: add tests executed from a subdirectory
Date: Fri, 06 May 2016 15:10:07 -0700 [thread overview]
Message-ID: <xmqqy47m25z4.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1462541720-79553-2-git-send-email-rappazzo@gmail.com> (Michael Rappazzo's message of "Fri, 6 May 2016 09:35:19 -0400")
Michael Rappazzo <rappazzo@gmail.com> writes:
> t1500-rev-parse contains envrionment leaks (changing dir without
> changing back, setting config variables, etc). Add a test to clean this
> up up so that future tests can be added without worry of any setting
> from a previous test.
This is a wonderful thing to do, but...
> test_rev_parse toplevel false false true '' .git
>
> @@ -84,4 +85,41 @@ test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false ''
> git config --unset core.bare
> test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
>
> +test_expect_success 'cleanup from the previous tests' '
> + cd .. &&
> + rm -r work &&
Instead of cleaning things up like this, could you please please
please fix these existing tests that chdir around without being in a
subshell? If the "previous tests" failed before going down as this
step expects, the "cd .. && rm -r" can make things worse.
> + mv repo.git .git &&
> + unset GIT_DIR &&
> + unset GIT_CONFIG &&
The spirit of this change is to make the test more independent from
the effects of what happened previously. Use sane_unset so that
we do not have to worry about previous step that may have failed
before it has a chance to set GIT_DIR and GIT_CONFIG (which would
cause these unset to fail).
> + git config core.bare $original_core_bare
Is this (rather, the capturing of $original_core_bare up above)
necessary? We are in the default 'trash' repository when the
capturing happens, and we know it is not a bare repository, right?
next prev parent reply other threads:[~2016-05-06 22:10 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-06 13:35 [PATCH v3 0/2] [PATCH v3 0/2] rev-parse: fix some options when executed from subpath of main tree Michael Rappazzo
2016-05-06 13:35 ` [PATCH v3 1/2] rev-parse tests: add tests executed from a subdirectory Michael Rappazzo
2016-05-06 22:10 ` Junio C Hamano [this message]
2016-05-07 13:35 ` Mike Rappazzo
2016-05-08 18:05 ` Junio C Hamano
2016-05-08 18:46 ` Junio C Hamano
2016-05-06 13:35 ` [PATCH v3 2/2] rev-parse: fix some options when executed from subpath of main tree Michael Rappazzo
2016-05-10 7:38 ` Eric Sunshine
-- strict thread matches above, loose matches on Subject: below --
2017-02-10 15:33 [PATCH v2 0/2] Fix bugs in rev-parse's output when run in a subdirectory Johannes Schindelin
2017-02-17 16:58 ` [PATCH v3 " Johannes Schindelin
2017-02-17 16:59 ` [PATCH v3 1/2] rev-parse tests: add tests executed from " Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy47m25z4.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
--cc=rappazzo@gmail.com \
--cc=szeder@ira.uka.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.