Re: [PATCH 1/3] remote-helpers: fix the run of all tests
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] remote-helpers: fix the run of all tests
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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] remote-helpers: fix the run of all tests
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) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] remote-helpers: fix the run of all tests
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] remote-helpers: fix the run of all tests
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] remote-helpers: fix the run of all tests
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] remote-helpers: fix the run of all tests
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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] remote-helpers: fix the run of all tests
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
