# [PATCH 1/3] remote-helpers: fix the run of all tests 15 messages from 2013-04-01 to 2013-04-04. Participants: Felipe Contreras, Antoine Pelisse, Junio C Hamano, Jeff King, Torsten Bögershausen. Thread: https://gitlist.dev/t/33344 ## Felipe Contreras, 2013-04-01 21:14 Subject: [PATCH 0/3] remote-helpers: trivial test fixes Message-ID: <1364850872-21135-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1364850872-21135-1-git-send-email-felipe.contreras%40gmail.com ``` Hi, Here are a few fixes for the test framework on the remote-helpers. Felipe Contreras (3): remote-helpers: fix the run of all tests remote-bzr: remove stale check code for tests remote-hg: fix hg-git test-case contrib/remote-helpers/Makefile | 1 + contrib/remote-helpers/test-bzr.sh | 14 -------------- contrib/remote-helpers/test-hg-hg-git.sh | 1 - 3 files changed, 1 insertion(+), 15 deletions(-) -- 1.8.2 ``` ## Felipe Contreras, 2013-04-01 21:14 Subject: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: <1364850872-21135-2-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1364850872-21135-2-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1364850872-21135-1-git-send-email-felipe.contreras@gmail.com> ``` Since test-lint doesn't seem to work here, lets disable it. It wouldn't help either way. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile index 9a76575..9c18ed8 100644 --- a/contrib/remote-helpers/Makefile +++ b/contrib/remote-helpers/Makefile @@ -3,6 +3,7 @@ TESTS := $(wildcard test*.sh) export T := $(addprefix $(CURDIR)/,$(TESTS)) export MAKE := $(MAKE) -e export PATH := $(CURDIR):$(PATH) +export TEST_LINT := test: $(MAKE) -C ../../t $@ -- 1.8.2 ``` ## Felipe Contreras, 2013-04-01 21:14 Subject: [PATCH 2/3] remote-bzr: remove stale check code for tests Message-ID: <1364850872-21135-3-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1364850872-21135-3-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1364850872-21135-1-git-send-email-felipe.contreras@gmail.com> ``` The fastimport plugin was only required in the early stage of development. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/test-bzr.sh | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh index 70aa8a0..5dac171 100755 --- a/contrib/remote-helpers/test-bzr.sh +++ b/contrib/remote-helpers/test-bzr.sh @@ -17,20 +17,6 @@ if ! "$PYTHON_PATH" -c 'import bzrlib'; then test_done fi -cmd=' -import bzrlib -bzrlib.initialize() -import bzrlib.plugin -bzrlib.plugin.load_plugins() -import bzrlib.plugins.fastimport -' - -if ! "$PYTHON_PATH" -c "$cmd"; then - echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2 - skip_all='skipping remote-bzr tests; bzr-fastimport not available' - test_done -fi - check () { (cd $1 && git log --format='%s' -1 && -- 1.8.2 ``` ## Felipe Contreras, 2013-04-01 21:14 Subject: [PATCH 3/3] remote-hg: fix hg-git test-case Message-ID: <1364850872-21135-4-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1364850872-21135-4-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1364850872-21135-1-git-send-email-felipe.contreras@gmail.com> ``` There was some lingering code that shouldn't have been there in the first place. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/test-hg-hg-git.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh index 7e3967f..3f253b7 100755 --- a/contrib/remote-helpers/test-hg-hg-git.sh +++ b/contrib/remote-helpers/test-hg-hg-git.sh @@ -140,7 +140,6 @@ test_expect_success 'executable bit' ' git_clone_$x hgrepo-$x gitrepo2-$x && git_log gitrepo2-$x > log-$x done && - cp -r log-* output-* /tmp/foo/ && test_cmp output-hg output-git && test_cmp log-hg log-git -- 1.8.2 ``` ## Antoine Pelisse, 2013-04-01 21:46 Subject: Re: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: URL: https://gitlist.dev/e/CALWbr2wAqudzbfiSHmD1uAVQX3mtbf5vgr2JXbXEg7yzaKXJcw%40mail.gmail.com In-Reply-To: <1364850872-21135-2-git-send-email-felipe.contreras@gmail.com> ``` Hey Felipe, On Mon, Apr 1, 2013 at 11:14 PM, Felipe Contreras wrote: > +export TEST_LINT := I think "test-lint-executable" still makes sense here. ``` ## Antoine Pelisse, 2013-04-01 21:48 Subject: Re: [PATCH 0/3] remote-helpers: trivial test fixes Message-ID: URL: https://gitlist.dev/e/CALWbr2wAJGemYziPsOkArJveK0F%2B4Pv80gdR1SGUxchb0ci-sg%40mail.gmail.com In-Reply-To: <1364850872-21135-1-git-send-email-felipe.contreras@gmail.com> ``` I'm having an error in "contrib/remote-helpers/test-hg.sh" and it reproduces on every version I tested since you implemented it. Is it something in my environment that would make it wrong ? Cheers, Antoine --- expecting success: test_when_finished "rm -rf gitrepo*" && ( cd hgrepo && hg update -r 0 ) && git clone "hg::$PWD/hgrepo" gitrepo && check gitrepo zero master 1 files updated, 0 files merged, 0 files removed, 0 files unresolved Cloning into 'gitrepo'... --- expected 2013-04-01 21:46:32.086796549 +0000 +++ actual 2013-04-01 21:46:32.086796549 +0000 @@ -1,2 +1,2 @@ -zero -refs/heads/master +feature-a +refs/heads/feature-a not ok 4 - cloning with detached head On Mon, Apr 1, 2013 at 11:14 PM, Felipe Contreras wrote: > Hi, > > Here are a few fixes for the test framework on the remote-helpers. > > Felipe Contreras (3): > remote-helpers: fix the run of all tests > remote-bzr: remove stale check code for tests > remote-hg: fix hg-git test-case > > contrib/remote-helpers/Makefile | 1 + > contrib/remote-helpers/test-bzr.sh | 14 -------------- > contrib/remote-helpers/test-hg-hg-git.sh | 1 - > 3 files changed, 1 insertion(+), 15 deletions(-) > > -- > 1.8.2 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ``` ## Felipe Contreras, 2013-04-01 21:56 Subject: Re: [PATCH 0/3] remote-helpers: trivial test fixes Message-ID: URL: https://gitlist.dev/e/CAMP44s2d-gZHsgibq2E8vZ25vciJRBh5Z0hYmx5pNa%2BCW5gvyg%40mail.gmail.com In-Reply-To: ``` Hi, On Mon, Apr 1, 2013 at 3:48 PM, Antoine Pelisse wrote: > I'm having an error in "contrib/remote-helpers/test-hg.sh" and it > reproduces on every version I tested since you implemented it. Is it > something in my environment that would make it wrong ? > > Cheers, > Antoine > > --- > expecting success: > test_when_finished "rm -rf gitrepo*" && > > ( > cd hgrepo && > hg update -r 0 > ) && > > git clone "hg::$PWD/hgrepo" gitrepo && > check gitrepo zero master > > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > Cloning into 'gitrepo'... > --- expected 2013-04-01 21:46:32.086796549 +0000 > +++ actual 2013-04-01 21:46:32.086796549 +0000 > @@ -1,2 +1,2 @@ > -zero > -refs/heads/master > +feature-a > +refs/heads/feature-a > not ok 4 - cloning with detached head You probably have a very old version of mercurial that has a bug, this should workaround the issue: http://thread.gmane.org/gmane.comp.version-control.git/209491 But it shouldn't be a big deal, it's just the default branch (HEAD). -- Felipe Contreras ``` ## Junio C Hamano, 2013-04-01 22:46 Subject: Re: [PATCH 2/3] remote-bzr: remove stale check code for tests Message-ID: <7vli91j2ds.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vli91j2ds.fsf%40alter.siamese.dyndns.org In-Reply-To: <1364850872-21135-3-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras writes: > The fastimport plugin was only required in the early stage of > development. OK, thanks, will queue. > > Signed-off-by: Felipe Contreras > --- > contrib/remote-helpers/test-bzr.sh | 14 -------------- > 1 file changed, 14 deletions(-) > > diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh > index 70aa8a0..5dac171 100755 > --- a/contrib/remote-helpers/test-bzr.sh > +++ b/contrib/remote-helpers/test-bzr.sh > @@ -17,20 +17,6 @@ if ! "$PYTHON_PATH" -c 'import bzrlib'; then > test_done > fi > > -cmd=' > -import bzrlib > -bzrlib.initialize() > -import bzrlib.plugin > -bzrlib.plugin.load_plugins() > -import bzrlib.plugins.fastimport > -' > - > -if ! "$PYTHON_PATH" -c "$cmd"; then > - echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2 > - skip_all='skipping remote-bzr tests; bzr-fastimport not available' > - test_done > -fi > - > check () { > (cd $1 && > git log --format='%s' -1 && ``` ## Felipe Contreras, 2013-04-01 22:46 Subject: Re: [PATCH 0/3] remote-helpers: trivial test fixes Message-ID: URL: https://gitlist.dev/e/CAMP44s0cUZDuzdzR9k_UBgmKn4%3Ddy1he8o%3DXuPag%3DLhjWMWJoA%40mail.gmail.com In-Reply-To: ``` On Mon, Apr 1, 2013 at 3:56 PM, Felipe Contreras wrote: > You probably have a very old version of mercurial that has a bug, this > should workaround the issue: > http://thread.gmane.org/gmane.comp.version-control.git/209491 I meant: http://article.gmane.org/gmane.comp.version-control.git/209581 -- Felipe Contreras ``` ## Jeff King, 2013-04-01 23:40 Subject: Re: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: <20130401234016.GC30935@sigill.intra.peff.net> URL: https://gitlist.dev/e/20130401234016.GC30935%40sigill.intra.peff.net In-Reply-To: ``` On Mon, Apr 01, 2013 at 11:46:00PM +0200, Antoine Pelisse wrote: > On Mon, Apr 1, 2013 at 11:14 PM, Felipe Contreras > wrote: > > +export TEST_LINT := > > I think "test-lint-executable" still makes sense here. Also test-lint-shell-syntax, which finds a problem with the current code: $ cd contrib/remote-helpers $ make test TEST_LINT=test-lint-shell-syntax make -e -C ../../t test make[1]: Entering directory `/home/peff/compile/git/t' rm -f -r test-results /home/peff/compile/git/contrib/remote-helpers/test-bzr.sh:139: error: echo -n is not portable (please use printf): echo -n content > expected && make[1]: *** [test-lint-shell-syntax] Error 1 make[1]: Leaving directory `/home/peff/compile/git/t' make: *** [test] Error 2 I think the check for duplicate-numbers is the only one that does not make sense. Unfortunately it is not as simple as using $(filter-out) to get rid of it from TEST_LINT, as that variable may not even be set until we chain to the other Makefile (e.g., if it comes from loading config.mak). -Peff ``` ## Torsten Bögershausen, 2013-04-02 16:53 Subject: Re: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: <515B0D08.4020405@web.de> URL: https://gitlist.dev/e/515B0D08.4020405%40web.de In-Reply-To: <20130401234016.GC30935@sigill.intra.peff.net> ``` On 02.04.13 01:40, Jeff King wrote: > On Mon, Apr 01, 2013 at 11:46:00PM +0200, Antoine Pelisse wrote: > >> On Mon, Apr 1, 2013 at 11:14 PM, Felipe Contreras >> wrote: >>> +export TEST_LINT := >> >> I think "test-lint-executable" still makes sense here. > > Also test-lint-shell-syntax, which finds a problem with the current > code: > > $ cd contrib/remote-helpers > $ make test TEST_LINT=test-lint-shell-syntax > make -e -C ../../t test > make[1]: Entering directory `/home/peff/compile/git/t' > rm -f -r test-results > /home/peff/compile/git/contrib/remote-helpers/test-bzr.sh:139: error: echo -n is not portable (please use printf): echo -n content > expected && > make[1]: *** [test-lint-shell-syntax] Error 1 > make[1]: Leaving directory `/home/peff/compile/git/t' > make: *** [test] Error 2 > > I think the check for duplicate-numbers is the only one that does not > make sense. [] Not sure about that, I send a suggestion of a patch in a minute. Highlights: 1) - rename the contrib test cases and assigns real TC numbers 2) - Forward the numbers into the main "test Makefile" 1) Will probably collide with Felipe's changes, so we just can pick up the idea. 2) Is for only review. If we agree on the re-numbering of TC's in contrib, we can apply a second round of the patch later. /Torsten ``` ## Jeff King, 2013-04-02 17:36 Subject: Re: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: <20130402173652.GC24698@sigill.intra.peff.net> URL: https://gitlist.dev/e/20130402173652.GC24698%40sigill.intra.peff.net In-Reply-To: <515B0D08.4020405@web.de> ``` On Tue, Apr 02, 2013 at 06:53:28PM +0200, Torsten Bögershausen wrote: > > I think the check for duplicate-numbers is the only one that does not > > make sense. > [] > Not sure about that, I send a suggestion of a patch in a minute. > Highlights: > 1) - rename the contrib test cases and assigns real TC numbers > 2) - Forward the numbers into the main "test Makefile" I'm not sure if this is a good idea or not. It puts the contrib/remote-helpers into the same "number namespace" as the rest of the test scripts, and enforces uniqueness with test-lint-duplicates, when "make test" is run from contrib/remote-helpers. But people working on the main test scripts would not get any such check, and would happily break contrib/remote-helpers by adding duplicate test numbers. It makes sense to me to either: 1. Have the contrib/remote-helpers test live in their own test namespace completely, with their own numbers and test-results, and pull in relevant bits from the main test harness. We do this already with contrib/subtree. I suggested this when the tests first appeared, but there was some argument, and I don't remember the details. 2. Just integrate contrib test scripts into the main repository, but leave them off by default. For example, add: if test -z "$GIT_TEST_REMOTE_HELPERS"; then skip_all="Remote helper tests disabled (define GIT_TEST_REMOTE_HELPERS)" test_done fi to the top of the scripts, and then set GIT_TEST_REMOTE_HELPERS in contrib/remote-helpers/Makefile before chaining to the test Makefile. -Peff ``` ## Torsten Bögershausen, 2013-04-03 05:53 Subject: Re: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: <515BC3E5.2020501@web.de> URL: https://gitlist.dev/e/515BC3E5.2020501%40web.de In-Reply-To: <20130402173652.GC24698@sigill.intra.peff.net> ``` On 02.04.13 19:36, Jeff King wrote: > On Tue, Apr 02, 2013 at 06:53:28PM +0200, Torsten Bögershausen wrote: > >>> I think the check for duplicate-numbers is the only one that does not >>> make sense. >> [] >> Not sure about that, I send a suggestion of a patch in a minute. >> Highlights: >> 1) - rename the contrib test cases and assigns real TC numbers >> 2) - Forward the numbers into the main "test Makefile" > > I'm not sure if this is a good idea or not. It puts the > contrib/remote-helpers into the same "number namespace" as the rest of > the test scripts, and enforces uniqueness with test-lint-duplicates, > when "make test" is run from contrib/remote-helpers. But people working > on the main test scripts would not get any such check, and would happily > break contrib/remote-helpers by adding duplicate test numbers. > > It makes sense to me to either: > > 1. Have the contrib/remote-helpers test live in their own test > namespace completely, with their own numbers and test-results, and > pull in relevant bits from the main test harness. We do this > already with contrib/subtree. I suggested this when the tests > first appeared, but there was some argument, and I don't remember > the details. > > 2. Just integrate contrib test scripts into the main repository, but > leave them off by default. For example, add: > > if test -z "$GIT_TEST_REMOTE_HELPERS"; then > skip_all="Remote helper tests disabled (define GIT_TEST_REMOTE_HELPERS)" > test_done > fi > > to the top of the scripts, and then set GIT_TEST_REMOTE_HELPERS > in contrib/remote-helpers/Makefile before chaining to the test > Makefile. > > -Peff I think I do my homework, and start with 3), just check duplicates on numbered test cases. http://comments.gmane.org/gmane.comp.version-control.git/214194 I send a patch in a minute (thanks Eric for reading) ``` ## Junio C Hamano, 2013-04-03 14:57 Subject: Re: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: <7v8v4z7jcv.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8v4z7jcv.fsf%40alter.siamese.dyndns.org In-Reply-To: <20130402173652.GC24698@sigill.intra.peff.net> ``` Jeff King writes: > On Tue, Apr 02, 2013 at 06:53:28PM +0200, Torsten Bögershausen wrote: > >> > I think the check for duplicate-numbers is the only one that does not >> > make sense. >> [] >> Not sure about that, I send a suggestion of a patch in a minute. >> Highlights: >> 1) - rename the contrib test cases and assigns real TC numbers >> 2) - Forward the numbers into the main "test Makefile" > > I'm not sure if this is a good idea or not. If that's a polite way to say that this is not a good idea, I'd agree for all the reasons you mentioned. > It puts the > contrib/remote-helpers into the same "number namespace" as the rest of > the test scripts, and enforces uniqueness with test-lint-duplicates, > when "make test" is run from contrib/remote-helpers. But people working > on the main test scripts would not get any such check, and would happily > break contrib/remote-helpers by adding duplicate test numbers. > > It makes sense to me to either: > > 1. Have the contrib/remote-helpers test live in their own test > namespace completely, with their own numbers and test-results, and > pull in relevant bits from the main test harness. We do this > already with contrib/subtree. I suggested this when the tests > first appeared, but there was some argument, and I don't remember > the details. This makes more sense than the alternative, given that contrib/ material is "optional" from the main tree's point of view, at least to me. Thanks. > 2. Just integrate contrib test scripts into the main repository, but > leave them off by default. For example, add: > > if test -z "$GIT_TEST_REMOTE_HELPERS"; then > skip_all="Remote helper tests disabled (define GIT_TEST_REMOTE_HELPERS)" > test_done > fi > > to the top of the scripts, and then set GIT_TEST_REMOTE_HELPERS > in contrib/remote-helpers/Makefile before chaining to the test > Makefile. ``` ## Felipe Contreras, 2013-04-04 15:36 Subject: Re: [PATCH 1/3] remote-helpers: fix the run of all tests Message-ID: <515d9e126cd2a_69fd13fde1816811@nysa.mail> URL: https://gitlist.dev/e/515d9e126cd2a_69fd13fde1816811%40nysa.mail In-Reply-To: <20130401234016.GC30935@sigill.intra.peff.net> ``` Jeff King wrote: > On Mon, Apr 01, 2013 at 11:46:00PM +0200, Antoine Pelisse wrote: > > > On Mon, Apr 1, 2013 at 11:14 PM, Felipe Contreras > > wrote: > > > +export TEST_LINT := > > > > I think "test-lint-executable" still makes sense here. > > Also test-lint-shell-syntax, which finds a problem with the current > code: > > $ cd contrib/remote-helpers > $ make test TEST_LINT=test-lint-shell-syntax > make -e -C ../../t test > make[1]: Entering directory `/home/peff/compile/git/t' > rm -f -r test-results > /home/peff/compile/git/contrib/remote-helpers/test-bzr.sh:139: error: echo -n is not portable (please use printf): echo -n content > expected && > make[1]: *** [test-lint-shell-syntax] Error 1 > make[1]: Leaving directory `/home/peff/compile/git/t' > make: *** [test] Error 2 Indeed. I've fixed he problem, and added the two lint checks to the new version of the series. -- Felipe Contreras ```