Re: [PATCH 2/6] t5538: fix default http port
On Mon, Feb 10, 2014 at 10:23:53AM -0800, Junio C Hamano wrote: > Jeff King writes: > > > Here it is as a Real Patch™. I just based it on master, so it can > > replace your 5537/5538 fix in your series. > > Thanks, looks good. Will put this at the bottom and rebuild the > nd/http-fetch-shallow-fix series on top. Thanks. It might be worth squashing in the patch below (or sticking it on top), to cover git-daemon as well. AFAICT, that leaves SVN_HTTPD_PORT, which we do not set by default at all, and the p4 tests, which already do their own similar magic. The "set GIT_HTTPD_TESTS by default" magic should probably go in a separate series, so I'll roll them by themselves. -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 2/6] t5538: fix default http port
On Mon, Feb 10, 2014 at 02:15:24PM -0500, Jeff King wrote:
> Thanks. It might be worth squashing in the patch below (or sticking it
> on top), to cover git-daemon as well.
Patch would probably be easier to read if I actually included it...
-- >8 --
Subject: [PATCH] tests: auto-set git-daemon port
A recent commit taught lib-httpd to always start apache on
the same port as the numbered tests. Let's do the same for
the git-daemon tests.
Signed-off-by: Jeff King
---
t/lib-git-daemon.sh | 2 +-
t/t5570-git-daemon.sh | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index 394b06b..1f22de2 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -22,7 +22,7 @@ then
test_done
fi
-LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-'8121'}
+LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-${this_test#t}}
GIT_DAEMON_PID=
GIT_DAEMON_DOCUMENT_ROOT_PATH="$PWD"/repo
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index e061468..6b16379 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -3,7 +3,6 @@
test_description='test fetching over git protocol'
. ./test-lib.sh
-LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-5570}
. "$TEST_DIRECTORY"/lib-git-daemon.sh
start_git_daemon
--
1.8.5.2.500.g8060133
--
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 2/6] t5538: fix default http port
Jeff King writes: > Here it is as a Real Patch™. I just based it on master, so it can > replace your 5537/5538 fix in your series. Thanks, looks good. Will put this at the bottom and rebuild the nd/http-fetch-shallow-fix series on top. > 1. Is there anybody who has apache installed who would _not_ want to > bring it up for the test? > > 2. Is there anybody for whom the failure mode of bringing up apache > would be unpleasant (e.g., if it hangs the tests or something)? > > For (1), we could perhaps have a GIT_NO_TEST_HTTPD to avoid it. > > For (2), I suspect we may need to make our error handling more robust, > but getting people to run it is the first step to figuring out what the > problems are. > > If we go this route, we should probably do the same for > GIT_TEST_GIT_DAEMON in t5570 (and for that matter, we should probably do > the same for the port numbers). All good points. -- 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 2/6] t5538: fix default http port
On Sat, Feb 08, 2014 at 02:36:11PM +0700, Duy Nguyen wrote:
> > Thinking on this more, I wonder if we should just do something like
> > this:
> [...]
> Yes!
Here it is as a Real Patch™. I just based it on master, so it can
replace your 5537/5538 fix in your series.
> Next stop, attempt to start httpd at start_httpd regardless of
> GIT_TEST_HTTPD. If successful, test_set_prereq HTTPD or something that
> the following tests can use. If GIT_TEST_HTTPD is set and start_httpd
> fails, error out, not set prereq.
I'd be fine with that, as I always run them anyway. The potential
downsides would be:
1. Is there anybody who has apache installed who would _not_ want to
bring it up for the test?
2. Is there anybody for whom the failure mode of bringing up apache
would be unpleasant (e.g., if it hangs the tests or something)?
For (1), we could perhaps have a GIT_NO_TEST_HTTPD to avoid it.
For (2), I suspect we may need to make our error handling more robust,
but getting people to run it is the first step to figuring out what the
problems are.
If we go this route, we should probably do the same for
GIT_TEST_GIT_DAEMON in t5570 (and for that matter, we should probably do
the same for the port numbers).
-- >8 --
Subject: [PATCH] tests: auto-set LIB_HTTPD_PORT from test name
We set the default apache port for each of the httpd tests
to the 4-digit test number of the test script. We want these
to remain unique so that the tests do not conflict with each
other when run in parallel.
Instead of doing it manually in each test script, let's just
set it from the test name at run time. This is simpler, and
is one less thing to be updated when test scripts are
renamed (e.g., when being re-rolled or when conflicting
after being merged with another topic).
Incidentally, this fixes a case where t5537 and t5538 used
the same port number (5537), and could conflict with each
other when run in parallel.
Signed-off-by: Jeff King
---
t/lib-httpd.sh | 2 +-
t/t5537-fetch-shallow.sh | 1 -
t/t5538-push-shallow.sh | 1 -
t/t5540-http-push.sh | 1 -
t/t5541-http-push.sh | 1 -
t/t5550-http-fetch.sh| 1 -
t/t5551-http-fetch.sh| 1 -
t/t5561-http-backend.sh | 1 -
8 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index bfdff2a..b43162e 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -64,7 +64,7 @@ case $(uname) in
esac
LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'8111'}
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-${this_test#t}}
TEST_PATH="$TEST_DIRECTORY"/lib-httpd
HTTPD_ROOT_PATH="$PWD"/httpd
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index b0fa738..adf215a 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -178,7 +178,6 @@ if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
test_done
fi
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5537'}
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
index 0a6e40f..8e54ac5 100755
--- a/t/t5538-push-shallow.sh
+++ b/t/t5538-push-shallow.sh
@@ -126,7 +126,6 @@ if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
test_done
fi
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5537'}
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 5b0198c..8d7b3c5 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -16,7 +16,6 @@ then
fi
LIB_HTTPD_DAV=t
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5540'}
. "$TEST_DIRECTORY"/lib-httpd.sh
ROOT_PATH="$PWD"
start_httpd
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index bfd241e..73af16f 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -12,7 +12,6 @@ if test -n "$NO_CURL"; then
fi
ROOT_PATH="$PWD"
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5541'}
. "$TEST_DIRECTORY"/lib-httpd.sh
. "$TEST_DIRECTORY"/lib-terminal.sh
start_httpd
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 8392624..1a3a2b6 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -8,7 +8,6 @@ if test -n "$NO_CURL"; then
test_done
fi
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5550'}
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index a124efe..e07eaf3 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -8,7 +8,6 @@ if test -n "$NO_CURL"; then
test_done
fi
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5551'}
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh
index b5d7fbc..d23fb02 100755
--- a/t/t5561-http-backend.sh
+++ b/t/t5561-http-backend.sh
@@ -8,7 +8,6 @@ if test -n "$NO_CURL"; then
test_done
fi
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5561'}
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to major
Re: [PATCH 2/6] t5538: fix default http port
On Sat, Feb 8, 2014 at 6:47 AM, Jeff King wrote:
> Thinking on this more, I wonder if we should just do something like
> this:
>
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index bfdff2a..c82b4ee 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -64,7 +64,9 @@ case $(uname) in
> esac
>
> LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}
> -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'8111'}
> +if test -z "$LIB_HTTPD_PORT"; then
> + LIB_HTTPD_PORT=${this_test#t}
> +fi
>
> TEST_PATH="$TEST_DIRECTORY"/lib-httpd
> HTTPD_ROOT_PATH="$PWD"/httpd
>
> and drop the manual LIB_HTTPD_PORT setting in each of the test scripts.
> That would prevent this type of error in the future, and people can
> still override if they want to.
>
> Any test scripts that are not numbered would need to set it, but we
> should not have any of those (and if we did, the failure mode would be
> OK, as Apache would barf on the bogus port number).
Yes! Next stop, attempt to start httpd at start_httpd regardless of
GIT_TEST_HTTPD. If successful, test_set_prereq HTTPD or something that
the following tests can use. If GIT_TEST_HTTPD is set and start_httpd
fails, error out, not set prereq.
--
Duy
--
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 2/6] t5538: fix default http port
On Thu, Feb 06, 2014 at 02:35:33PM -0500, Jeff King wrote:
> On Thu, Feb 06, 2014 at 10:10:35PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
> > Originally I had t5537 use port 5536 and 5538 use port 5537(!). Then
> > Jeff found my fault so I changed port in t5537 from 5536 to 5537 in
> > 3b32a7c (t5537: fix incorrect expectation in test case 10 -
> > 2014-01-08). Which makes it worse because now both t5537 and t5538
> > both use the same port. Fix it.
> [...]
Thinking on this more, I wonder if we should just do something like
this:
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index bfdff2a..c82b4ee 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -64,7 +64,9 @@ case $(uname) in
esac
LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'8111'}
+if test -z "$LIB_HTTPD_PORT"; then
+ LIB_HTTPD_PORT=${this_test#t}
+fi
TEST_PATH="$TEST_DIRECTORY"/lib-httpd
HTTPD_ROOT_PATH="$PWD"/httpd
and drop the manual LIB_HTTPD_PORT setting in each of the test scripts.
That would prevent this type of error in the future, and people can
still override if they want to.
Any test scripts that are not numbered would need to set it, but we
should not have any of those (and if we did, the failure mode would be
OK, as Apache would barf on the bogus port number).
-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 2/6] t5538: fix default http port
On Thu, Feb 06, 2014 at 10:10:35PM +0700, Nguyễn Thái Ngọc Duy wrote: > Originally I had t5537 use port 5536 and 5538 use port 5537(!). Then > Jeff found my fault so I changed port in t5537 from 5536 to 5537 in > 3b32a7c (t5537: fix incorrect expectation in test case 10 - > 2014-01-08). Which makes it worse because now both t5537 and t5538 > both use the same port. Fix it. Yikes. I'm surprised we didn't notice this before, as it would probably barf intermittently when running the tests in parallel. Perhaps it was the cause of some intermittent failures that have gone undiagnosed. Patch looks obviously correct. -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
