{"thread":{"id":"36456","subject":"[PATCH 0/2] remote-helpers: graduate","startedAt":"2014-04-20T21:39:29Z","lastAt":"2014-04-21T19:24:10Z","messageCount":8,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"239186","messageId":"1398029971-1396-1-git-send-email-felipe.contreras@gmail.com","threadId":"36456","inReplyTo":null,"subject":"[PATCH 0/2] remote-helpers: graduate","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-20T21:39:29Z","receivedAt":"2014-04-20T21:39:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There doesn't seem to be any reason to keep these remote-helpers in the contrib\narea.\n\nFelipe Contreras (2):\n  remote-helpers: move out of contrib\n  remote-helpers: move tests out of contrib\n\n .gitignore                                                 |  2 ++\n Makefile                                                   |  2 ++\n contrib/remote-helpers/Makefile                            | 14 --------------\n contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py |  0\n contrib/remote-helpers/git-remote-hg => git-remote-hg.py   |  0\n t/Makefile                                                 |  8 +++++++-\n .../remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t   |  4 ++--\n .../test-hg-bidi.sh => t/remote-helpers/hg-bidi.t          |  4 ++--\n .../test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t      |  6 +++---\n contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t |  4 ++--\n 10 files changed, 20 insertions(+), 24 deletions(-)\n delete mode 100644 contrib/remote-helpers/Makefile\n rename contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py (100%)\n rename contrib/remote-helpers/git-remote-hg => git-remote-hg.py (100%)\n rename contrib/remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t (98%)\n rename contrib/remote-helpers/test-hg-bidi.sh => t/remote-helpers/hg-bidi.t (97%)\n rename contrib/remote-helpers/test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t (98%)\n rename contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t (99%)\n\n-- \n1.9.1+fc3.9.gc73078e\n"},{"id":"239188","messageId":"1398029971-1396-2-git-send-email-felipe.contreras@gmail.com","threadId":"36456","inReplyTo":"1398029971-1396-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/2] remote-helpers: move out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-20T21:39:30Z","receivedAt":"2014-04-20T21:39:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The remote-helpers in contrib/remote-helpers have proved to work, be\nreliable, and stable. It's time to move them out of contrib, and be\ndistributed by default.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n .gitignore                                                 | 2 ++\n Makefile                                                   | 2 ++\n contrib/remote-helpers/test-bzr.sh                         | 2 +-\n contrib/remote-helpers/test-hg-bidi.sh                     | 2 +-\n contrib/remote-helpers/test-hg-hg-git.sh                   | 4 ++--\n contrib/remote-helpers/test-hg.sh                          | 2 +-\n contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py | 0\n contrib/remote-helpers/git-remote-hg => git-remote-hg.py   | 0\n 8 files changed, 9 insertions(+), 5 deletions(-)\n rename contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py (100%)\n rename contrib/remote-helpers/git-remote-hg => git-remote-hg.py (100%)\n\ndiff --git a/.gitignore b/.gitignore\nindex dc600f9..db5f15e 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -119,11 +119,13 @@\n /git-reflog\n /git-relink\n /git-remote\n+/git-remote-bzr\n /git-remote-http\n /git-remote-https\n /git-remote-ftp\n /git-remote-ftps\n /git-remote-fd\n+/git-remote-hg\n /git-remote-ext\n /git-remote-testgit\n /git-remote-testpy\ndiff --git a/Makefile b/Makefile\nindex 2128ce3..fda4567 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -483,6 +483,8 @@ SCRIPT_PERL += git-send-email.perl\n SCRIPT_PERL += git-svn.perl\n \n SCRIPT_PYTHON += git-p4.py\n+SCRIPT_PYTHON += git-remote-hg.py\n+SCRIPT_PYTHON += git-remote-bzr.py\n \n NO_INSTALL += git-remote-testgit\n \ndiff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh\nindex a4656ce..330b147 100755\n--- a/contrib/remote-helpers/test-bzr.sh\n+++ b/contrib/remote-helpers/test-bzr.sh\n@@ -14,7 +14,7 @@ then\n \ttest_done\n fi\n \n-if ! python -c 'import bzrlib'\n+if ! \"$PYTHON_PATH\" -c 'import bzrlib'\n then\n \tskip_all='skipping remote-bzr tests; bzr not available'\n \ttest_done\ndiff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh\nindex d86e147..e963745 100755\n--- a/contrib/remote-helpers/test-hg-bidi.sh\n+++ b/contrib/remote-helpers/test-hg-bidi.sh\n@@ -17,7 +17,7 @@ then\n \ttest_done\n fi\n \n-if ! python -c 'import mercurial'\n+if ! \"$PYTHON_PATH\" -c 'import mercurial'\n then\n \tskip_all='skipping remote-hg tests; mercurial not available'\n \ttest_done\ndiff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh\nindex b23909a..c6b4541 100755\n--- a/contrib/remote-helpers/test-hg-hg-git.sh\n+++ b/contrib/remote-helpers/test-hg-hg-git.sh\n@@ -17,13 +17,13 @@ then\n \ttest_done\n fi\n \n-if ! python -c 'import mercurial'\n+if ! \"$PYTHON_PATH\" -c 'import mercurial'\n then\n \tskip_all='skipping remote-hg tests; mercurial not available'\n \ttest_done\n fi\n \n-if ! python -c 'import hggit'\n+if ! \"$PYTHON_PATH\" -c 'import hggit'\n then\n \tskip_all='skipping remote-hg tests; hg-git not available'\n \ttest_done\ndiff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh\nindex 7d90056..00b7dde 100755\n--- a/contrib/remote-helpers/test-hg.sh\n+++ b/contrib/remote-helpers/test-hg.sh\n@@ -17,7 +17,7 @@ then\n \ttest_done\n fi\n \n-if ! python -c 'import mercurial'\n+if ! \"$PYTHON_PATH\" -c 'import mercurial'\n then\n \tskip_all='skipping remote-hg tests; mercurial not available'\n \ttest_done\ndiff --git a/contrib/remote-helpers/git-remote-bzr b/git-remote-bzr.py\nsimilarity index 100%\nrename from contrib/remote-helpers/git-remote-bzr\nrename to git-remote-bzr.py\ndiff --git a/contrib/remote-helpers/git-remote-hg b/git-remote-hg.py\nsimilarity index 100%\nrename from contrib/remote-helpers/git-remote-hg\nrename to git-remote-hg.py\n-- \n1.9.1+fc3.9.gc73078e\n"},{"id":"239187","messageId":"1398029971-1396-3-git-send-email-felipe.contreras@gmail.com","threadId":"36456","inReplyTo":"1398029971-1396-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/2] remote-helpers: move tests out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-20T21:39:31Z","receivedAt":"2014-04-20T21:39:31Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"They should be tested by default.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/remote-helpers/Makefile                            | 14 --------------\n t/Makefile                                                 |  8 +++++++-\n .../remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t   |  2 +-\n .../test-hg-bidi.sh => t/remote-helpers/hg-bidi.t          |  2 +-\n .../test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t      |  2 +-\n contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t |  2 +-\n 6 files changed, 11 insertions(+), 19 deletions(-)\n delete mode 100644 contrib/remote-helpers/Makefile\n rename contrib/remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t (99%)\n rename contrib/remote-helpers/test-hg-bidi.sh => t/remote-helpers/hg-bidi.t (98%)\n rename contrib/remote-helpers/test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t (99%)\n rename contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t (99%)\n\ndiff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile\ndeleted file mode 100644\nindex 239161d..0000000\n--- a/contrib/remote-helpers/Makefile\n+++ /dev/null\n@@ -1,14 +0,0 @@\n-TESTS := $(wildcard test*.sh)\n-\n-export T := $(addprefix $(CURDIR)/,$(TESTS))\n-export MAKE := $(MAKE) -e\n-export PATH := $(CURDIR):$(PATH)\n-export TEST_LINT := test-lint-executable test-lint-shell-syntax\n-\n-test:\n-\t$(MAKE) -C ../../t $@\n-\n-$(TESTS):\n-\t$(MAKE) -C ../../t $(CURDIR)/$@\n-\n-.PHONY: $(TESTS)\ndiff --git a/t/Makefile b/t/Makefile\nindex 8fd1a72..818f4ed 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -14,6 +14,7 @@ RM ?= rm -f\n PROVE ?= prove\n DEFAULT_TEST_TARGET ?= test\n TEST_LINT ?= test-lint-duplicates test-lint-executable\n+export TEST_DIRECTORY = $(CURDIR)\n \n ifdef TEST_OUTPUT_DIRECTORY\n TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results\n@@ -29,6 +30,9 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\\'',$(TEST_RESULTS_DIRECTORY))\n T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))\n TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))\n+TREMOTE = $(wildcard remote-helpers/*.t)\n+\n+T += $(TREMOTE)\n \n all: $(DEFAULT_TEST_TARGET)\n \n@@ -55,7 +59,7 @@ clean: clean-except-prove-cache\n test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax\n \n test-lint-duplicates:\n-\t@dups=`echo $(T) | tr ' ' '\\n' | sed 's/-.*//' | sort | uniq -d` && \\\n+\t@dups=`echo $(filter-out $(TREMOTE),$(T)) | tr ' ' '\\n' | sed 's/-.*//' | sort | uniq -d` && \\\n \t\ttest -z \"$$dups\" || { \\\n \t\techo >&2 \"duplicate test numbers:\" $$dups; exit 1; }\n \n@@ -84,6 +88,8 @@ full-svn-test:\n gitweb-test:\n \t$(MAKE) $(TGITWEB)\n \n+remote-helpers-test: $(TREMOTE)\n+\n valgrind:\n \t$(MAKE) GIT_TEST_OPTS=\"$(GIT_TEST_OPTS) --valgrind\"\n \ndiff --git a/contrib/remote-helpers/test-bzr.sh b/t/remote-helpers/bzr.t\nsimilarity index 99%\nrename from contrib/remote-helpers/test-bzr.sh\nrename to t/remote-helpers/bzr.t\nindex 330b147..066e326 100755\n--- a/contrib/remote-helpers/test-bzr.sh\n+++ b/t/remote-helpers/bzr.t\n@@ -5,7 +5,7 @@\n \n test_description='Test remote-bzr'\n \n-test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/../../t\n+test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/..\n . \"$TEST_DIRECTORY\"/test-lib.sh\n \n if ! test_have_prereq PYTHON\ndiff --git a/contrib/remote-helpers/test-hg-bidi.sh b/t/remote-helpers/hg-bidi.t\nsimilarity index 98%\nrename from contrib/remote-helpers/test-hg-bidi.sh\nrename to t/remote-helpers/hg-bidi.t\nindex e963745..f6c51b0 100755\n--- a/contrib/remote-helpers/test-hg-bidi.sh\n+++ b/t/remote-helpers/hg-bidi.t\n@@ -8,7 +8,7 @@\n \n test_description='Test bidirectionality of remote-hg'\n \n-test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/../../t\n+test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/..\n . \"$TEST_DIRECTORY\"/test-lib.sh\n \n if ! test_have_prereq PYTHON\ndiff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/t/remote-helpers/hg-hg-git.t\nsimilarity index 99%\nrename from contrib/remote-helpers/test-hg-hg-git.sh\nrename to t/remote-helpers/hg-hg-git.t\nindex c6b4541..12e2d51 100755\n--- a/contrib/remote-helpers/test-hg-hg-git.sh\n+++ b/t/remote-helpers/hg-hg-git.t\n@@ -8,7 +8,7 @@\n \n test_description='Test remote-hg output compared to hg-git'\n \n-test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/../../t\n+test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/..\n . \"$TEST_DIRECTORY\"/test-lib.sh\n \n if ! test_have_prereq PYTHON\ndiff --git a/contrib/remote-helpers/test-hg.sh b/t/remote-helpers/hg.t\nsimilarity index 99%\nrename from contrib/remote-helpers/test-hg.sh\nrename to t/remote-helpers/hg.t\nindex 00b7dde..b4f26e7 100755\n--- a/contrib/remote-helpers/test-hg.sh\n+++ b/t/remote-helpers/hg.t\n@@ -8,7 +8,7 @@\n \n test_description='Test remote-hg'\n \n-test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/../../t\n+test -n \"$TEST_DIRECTORY\" || TEST_DIRECTORY=${0%/*}/..\n . \"$TEST_DIRECTORY\"/test-lib.sh\n \n if ! test_have_prereq PYTHON\n-- \n1.9.1+fc3.9.gc73078e\n"},{"id":"239211","messageId":"xmqqbnvua768.fsf@gitster.dls.corp.google.com","threadId":"36456","inReplyTo":"1398029971-1396-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/2] remote-helpers: graduate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-21T18:06:39Z","receivedAt":"2014-04-21T18:06:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> There doesn't seem to be any reason to keep these remote-helpers in the contrib\n> area.\n\nYay.\n\nI wouldn't phrase it \"doesn't seem to be any reason\", though.  The\ndecision to include is not done due to lack of negatives, but\nbecause adding them would be useful.\n\nAnd I think for these two, it holds true---people seem to have been\nexperimenting with them and getting good results with occasional bug\nreports that are addressed in a reasonable promptness.\n\nWill queue.\n\n\n> Felipe Contreras (2):\n>   remote-helpers: move out of contrib\n>   remote-helpers: move tests out of contrib\n>\n>  .gitignore                                                 |  2 ++\n>  Makefile                                                   |  2 ++\n>  contrib/remote-helpers/Makefile                            | 14 --------------\n>  contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py |  0\n>  contrib/remote-helpers/git-remote-hg => git-remote-hg.py   |  0\n>  t/Makefile                                                 |  8 +++++++-\n>  .../remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t   |  4 ++--\n>  .../test-hg-bidi.sh => t/remote-helpers/hg-bidi.t          |  4 ++--\n>  .../test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t      |  6 +++---\n>  contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t |  4 ++--\n>  10 files changed, 20 insertions(+), 24 deletions(-)\n>  delete mode 100644 contrib/remote-helpers/Makefile\n>  rename contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py (100%)\n>  rename contrib/remote-helpers/git-remote-hg => git-remote-hg.py (100%)\n>  rename contrib/remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t (98%)\n>  rename contrib/remote-helpers/test-hg-bidi.sh => t/remote-helpers/hg-bidi.t (97%)\n>  rename contrib/remote-helpers/test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t (98%)\n>  rename contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t (99%)\n"},{"id":"239213","messageId":"xmqq7g6ia5rr.fsf@gitster.dls.corp.google.com","threadId":"36456","inReplyTo":"1398029971-1396-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/2] remote-helpers: move tests out of contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-21T18:36:56Z","receivedAt":"2014-04-21T18:36:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> They should be tested by default.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  contrib/remote-helpers/Makefile                            | 14 --------------\n>  t/Makefile                                                 |  8 +++++++-\n>  .../remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t   |  2 +-\n>  .../test-hg-bidi.sh => t/remote-helpers/hg-bidi.t          |  2 +-\n>  .../test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t      |  2 +-\n>  contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t |  2 +-\n>  6 files changed, 11 insertions(+), 19 deletions(-)\n>  delete mode 100644 contrib/remote-helpers/Makefile\n>  rename contrib/remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t (99%)\n>  rename contrib/remote-helpers/test-hg-bidi.sh => t/remote-helpers/hg-bidi.t (98%)\n>  rename contrib/remote-helpers/test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t (99%)\n>  rename contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t (99%)\n>\n> diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile\n> deleted file mode 100644\n> index 239161d..0000000\n> --- a/contrib/remote-helpers/Makefile\n> +++ /dev/null\n> @@ -1,14 +0,0 @@\n> -TESTS := $(wildcard test*.sh)\n> -\n> -export T := $(addprefix $(CURDIR)/,$(TESTS))\n> -export MAKE := $(MAKE) -e\n> -export PATH := $(CURDIR):$(PATH)\n> -export TEST_LINT := test-lint-executable test-lint-shell-syntax\n> -\n> -test:\n> -\t$(MAKE) -C ../../t $@\n> -\n> -$(TESTS):\n> -\t$(MAKE) -C ../../t $(CURDIR)/$@\n> -\n> -.PHONY: $(TESTS)\n> diff --git a/t/Makefile b/t/Makefile\n> index 8fd1a72..818f4ed 100644\n> --- a/t/Makefile\n> +++ b/t/Makefile\n> @@ -14,6 +14,7 @@ RM ?= rm -f\n>  PROVE ?= prove\n>  DEFAULT_TEST_TARGET ?= test\n>  TEST_LINT ?= test-lint-duplicates test-lint-executable\n> +export TEST_DIRECTORY = $(CURDIR)\n>  \n>  ifdef TEST_OUTPUT_DIRECTORY\n>  TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results\n> @@ -29,6 +30,9 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\\'',$(TEST_RESULTS_DIRECTORY))\n>  T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n>  TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))\n>  TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))\n> +TREMOTE = $(wildcard remote-helpers/*.t)\n\nThis step needs a bit more work, I am afraid, to at least have these\nthree test scripts follow the same numbering scheme.  Especially given\nthat there were recent discussions on allowing a range of tests to\nbe run (or omitted) via notations like \"5000,5020,9800-9812\", not\ndoing so now will make it harder to implement such an enhancement.\n\nAlso, I noticed that, unlike say t9801 that shows this:\n\n    $ cd t && make T=t9801-*sh prove\n    rm -f -r 'test-results'\n    *** prove ***\n    t9801-git-p4-branch.sh .. skipped: skipping git p4 tests; no p4 or p4d\n    Files=1, Tests=0,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.00 ...\n    Result: NOTESTS\n\nthese are unnecessarily noisy when refusing to run:\n\n    $ cd t && make T=remote-helpers/bzr.t prove\n    *** prove ***\n    remote-helpers/bzr.t .. Traceback (most recent call last):\n      File \"<string>\", line 1, in <module>\n      File \"/usr/local/buildtools/current/sitecustomize/sitecu...\n        return real_import(name, globals, locals, fromlist, level)\n    ImportError: No module named bzrlib\n    remote-helpers/bzr.t .. skipped: skipping remote-bzr tests; bzr not available\n    Files=1, Tests=0,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.02 ...\n\nCan we squelch these expected import error messages?\n\nThanks.\n"},{"id":"239217","messageId":"5355690fbe8ad_32c4849310d1@nysa.notmuch","threadId":"36456","inReplyTo":"xmqq7g6ia5rr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] remote-helpers: move tests out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-21T18:53:03Z","receivedAt":"2014-04-21T18:53:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> > They should be tested by default.\n> >\n> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> > ---\n> >  contrib/remote-helpers/Makefile                            | 14 --------------\n> >  t/Makefile                                                 |  8 +++++++-\n> >  .../remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t   |  2 +-\n> >  .../test-hg-bidi.sh => t/remote-helpers/hg-bidi.t          |  2 +-\n> >  .../test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t      |  2 +-\n> >  contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t |  2 +-\n> >  6 files changed, 11 insertions(+), 19 deletions(-)\n> >  delete mode 100644 contrib/remote-helpers/Makefile\n> >  rename contrib/remote-helpers/test-bzr.sh => t/remote-helpers/bzr.t (99%)\n> >  rename contrib/remote-helpers/test-hg-bidi.sh => t/remote-helpers/hg-bidi.t (98%)\n> >  rename contrib/remote-helpers/test-hg-hg-git.sh => t/remote-helpers/hg-hg-git.t (99%)\n> >  rename contrib/remote-helpers/test-hg.sh => t/remote-helpers/hg.t (99%)\n> >\n> > diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile\n> > deleted file mode 100644\n> > index 239161d..0000000\n> > --- a/contrib/remote-helpers/Makefile\n> > +++ /dev/null\n> > @@ -1,14 +0,0 @@\n> > -TESTS := $(wildcard test*.sh)\n> > -\n> > -export T := $(addprefix $(CURDIR)/,$(TESTS))\n> > -export MAKE := $(MAKE) -e\n> > -export PATH := $(CURDIR):$(PATH)\n> > -export TEST_LINT := test-lint-executable test-lint-shell-syntax\n> > -\n> > -test:\n> > -\t$(MAKE) -C ../../t $@\n> > -\n> > -$(TESTS):\n> > -\t$(MAKE) -C ../../t $(CURDIR)/$@\n> > -\n> > -.PHONY: $(TESTS)\n> > diff --git a/t/Makefile b/t/Makefile\n> > index 8fd1a72..818f4ed 100644\n> > --- a/t/Makefile\n> > +++ b/t/Makefile\n> > @@ -14,6 +14,7 @@ RM ?= rm -f\n> >  PROVE ?= prove\n> >  DEFAULT_TEST_TARGET ?= test\n> >  TEST_LINT ?= test-lint-duplicates test-lint-executable\n> > +export TEST_DIRECTORY = $(CURDIR)\n> >  \n> >  ifdef TEST_OUTPUT_DIRECTORY\n> >  TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results\n> > @@ -29,6 +30,9 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\\'',$(TEST_RESULTS_DIRECTORY))\n> >  T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n> >  TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))\n> >  TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))\n> > +TREMOTE = $(wildcard remote-helpers/*.t)\n> \n> This step needs a bit more work, I am afraid, to at least have these\n> three test scripts follow the same numbering scheme.  Especially given\n> that there were recent discussions on allowing a range of tests to\n> be run (or omitted) via notations like \"5000,5020,9800-9812\", not\n> doing so now will make it harder to implement such an enhancement.\n\nI don't see how such an enhancement would be beneficial to these remote\nhelpers. For starters there aren't any number rages left for them, which is the\nreason why I created a new directory and handled them in a special way.\n\nThere might be other practical reasons to use this numbering scheme (although\nI'm sure the scripts could be fixed to not assume those), so I'm OK with having\nnumbers _for now_, but the range namespace should be different from t/t*.sh; it\nshould start from 0, since this is a different directory. That would probably\nrequire updating the scripts anyway, so I'm not sure what would be the benefit\nof this numbering scheme.\n\n> Also, I noticed that, unlike say t9801 that shows this:\n> \n>     $ cd t && make T=t9801-*sh prove\n>     rm -f -r 'test-results'\n>     *** prove ***\n>     t9801-git-p4-branch.sh .. skipped: skipping git p4 tests; no p4 or p4d\n>     Files=1, Tests=0,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.00 ...\n>     Result: NOTESTS\n> \n> these are unnecessarily noisy when refusing to run:\n> \n>     $ cd t && make T=remote-helpers/bzr.t prove\n>     *** prove ***\n>     remote-helpers/bzr.t .. Traceback (most recent call last):\n>       File \"<string>\", line 1, in <module>\n>       File \"/usr/local/buildtools/current/sitecustomize/sitecu...\n>         return real_import(name, globals, locals, fromlist, level)\n>     ImportError: No module named bzrlib\n>     remote-helpers/bzr.t .. skipped: skipping remote-bzr tests; bzr not available\n>     Files=1, Tests=0,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.02 ...\n> \n> Can we squelch these expected import error messages?\n\nSure:\n\nif ! \"$PYTHON_PATH\" -c 'import bzrlib' > /dev/null 2>&1\n\n-- \nFelipe Contreras\n"},{"id":"239222","messageId":"53557021ed75d_414c80b30836@nysa.notmuch","threadId":"36456","inReplyTo":"xmqqy4yy8p0l.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] remote-helpers: move tests out of contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-21T19:23:14Z","receivedAt":"2014-04-21T19:23:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> >> This step needs a bit more work, I am afraid, to at least have these\n> >> three test scripts follow the same numbering scheme.  Especially given\n> >> that there were recent discussions on allowing a range of tests to\n> >> be run (or omitted) via notations like \"5000,5020,9800-9812\", not\n> >> doing so now will make it harder to implement such an enhancement.\n> >\n> > I don't see how such an enhancement would be beneficial to these remote\n> > helpers.\n> \n> Why are remote helpers more special than other parts of the system?\n\nThey aren't.\n\n> Do they _have to_ be different and special?\n\nNo.\n \n> > For starters there aren't any number rages left for them,...\n> \n> Wouldn't \"5 - the pull and exporting\" be an obvious space for them,\n> especiall somewhere after 5800?\n\nNot _that_ obvious. If you are OK with having the hg-hg-git test (a test which\nrequires hg-git, a tool that is completely independent from git-remote-hg)\nthere, then fine.\n\n-- \nFelipe Contreras\n"},{"id":"239220","messageId":"xmqqy4yy8p0l.fsf@gitster.dls.corp.google.com","threadId":"36456","inReplyTo":"5355690fbe8ad_32c4849310d1@nysa.notmuch","subject":"Re: [PATCH 2/2] remote-helpers: move tests out of contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-21T19:24:10Z","receivedAt":"2014-04-21T19:24:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> This step needs a bit more work, I am afraid, to at least have these\n>> three test scripts follow the same numbering scheme.  Especially given\n>> that there were recent discussions on allowing a range of tests to\n>> be run (or omitted) via notations like \"5000,5020,9800-9812\", not\n>> doing so now will make it harder to implement such an enhancement.\n>\n> I don't see how such an enhancement would be beneficial to these remote\n> helpers.\n\nWhy are remote helpers more special than other parts of the system?\nDo they _have to_ be different and special?\n\n> For starters there aren't any number rages left for them,...\n\nWouldn't \"5 - the pull and exporting\" be an obvious space for them,\nespeciall somewhere after 5800?\n\n> There might be other practical reasons to use this numbering scheme (although\n> I'm sure the scripts could be fixed to not assume those),...\n\n;-)  Yes, as long as you can program it, anything is possible.\n\n>> Can we squelch these expected import error messages?\n>\n> Sure:\n\nThen I'd expect a reroll, at least to 2/2 (I do not think I saw\nanything iffy in 1/2).\n\nThanks.\n"}]}