You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2007/02/05 12:15:01 UTC
[PATCH]Make svnserve auto test instance to listen on random port
Hi All,
Find the attached patch and log.
With regards
Kamesh Jayachandran
Re: [PATCH]Make svnserve auto test instance to listen on random port
Posted by Daniel Rall <dl...@collab.net>.
On Tue, 06 Feb 2007, Kamesh Jayachandran wrote:
> Daniel Rall wrote:
> >On Mon, 05 Feb 2007, Kamesh Jayachandran wrote:
...
> >>Run the test svnserve instance on a random port so that it won't
> >>collide with existing instance of standard svnserve.
> >>
> > ^^^
> >I'd remove the word "standard" from the log message.
>
> done.
...
> >I'm in favor of this concept, but have some comments about the
> >implementation.
> >
> >
> >>Index: subversion/tests/cmdline/svnserveautocheck.sh
> >>===================================================================
> >>--- subversion/tests/cmdline/svnserveautocheck.sh (revision 23345)
> >>+++ subversion/tests/cmdline/svnserveautocheck.sh (working copy)
> >>@@ -60,13 +60,20 @@
> >>
> >> rm -f $SVNSERVE_PID
> >>
> >>+SVNSERVE_PORT=$(($RANDOM+1024))
> >>
> >
> >$RANDOM should works with /bin/bash, which is apparently already being
> >demanded by this script.
> >
> >
> >>+while netstat -an|grep $SVNSERVE_PORT|grep "LISTEN";
> >>
> >
> >Will netstat always be in the user's path? On my Fedora Core 6 box,
> >it's in /bin, which is fine. How about on Solaris, OS X, etc.?
> >
> >
>
> Yes in Solaris it is at /bin. In MacOS it is in /usr/sbin which seemed
> to be in the default PATH.
>
> >As written, you shouldn't need the semi-colon at the end of the line.
> >However, I'd rather see the "do" at the end of this line, in which
> >case you'd retain the semi-colon.
> >
> >
> done.
>
> >>+do
> >>+SVNSERVE_PORT=$(($RANDOM+1024))
> >>
> >
> >Need leading indentation.
> >
> done
I've committed this part of the change today (r23372). I'm still a
little concerned that this loop might never exit on some systems, but
it WORKSFORME, so we'll cross that bridge if we ever reach it.
> >>+done
> >>+
> >> $SERVER_CMD -d -r $ABS_BUILDDIR/subversion/tests/cmdline \
> >> --listen-host 127.0.0.1 \
> >>+ --listen-port $SVNSERVE_PORT \
> >> --pid-file $SVNSERVE_PID &
> >>
> >>-BASE_URL="svn://localhost"
> >>+BASE_URL="svn://localhost:$SVNSERVE_PORT"
> >>
> >>-make svncheck
> >>+make check BASE_URL=$BASE_URL
> >
> >Changing the make target doesn't seem unreasonable to me, but if done,
> >should be a separate commit. I believe that doing so would also
> >obviate the need for setting a BASE_URL environment variable in
> >svnserveautocheck.sh.
>
> Do you mean changing the 'svncheck' target?
>
> +make check BASE_URL=$BASE_URL is more like that of davautocheck.
Yeah, that's what I meant. I committed this part of the change
earlier this week.
Re: [PATCH]Make svnserve auto test instance to listen on random port
Posted by Kamesh Jayachandran <ka...@collab.net>.
Daniel Rall wrote:
> On Mon, 05 Feb 2007, Kamesh Jayachandran wrote:
>
>
>> [[[
>>
>> Run the test svnserve instance on a random port so that it won't
>> collide with existing instance of standard svnserve.
>>
> ^^^
> I'd remove the word "standard" from the log message.
>
done.
>
>> * subversion/tests/cmdline/svnserveautocheck.sh
>> Run 'svnserve' on a random free to LISTEN port.
>>
>> Patch by: kameshj
>> ]]]
>>
>
> I'm in favor of this concept, but have some comments about the
> implementation.
>
>
>> Index: subversion/tests/cmdline/svnserveautocheck.sh
>> ===================================================================
>> --- subversion/tests/cmdline/svnserveautocheck.sh (revision 23345)
>> +++ subversion/tests/cmdline/svnserveautocheck.sh (working copy)
>> @@ -60,13 +60,20 @@
>>
>> rm -f $SVNSERVE_PID
>>
>> +SVNSERVE_PORT=$(($RANDOM+1024))
>>
>
> $RANDOM should works with /bin/bash, which is apparently already being
> demanded by this script.
>
>
>> +while netstat -an|grep $SVNSERVE_PORT|grep "LISTEN";
>>
>
> Will netstat always be in the user's path? On my Fedora Core 6 box,
> it's in /bin, which is fine. How about on Solaris, OS X, etc.?
>
>
Yes in Solaris it is at /bin. In MacOS it is in /usr/sbin which seemed
to be in the default PATH.
> As written, you shouldn't need the semi-colon at the end of the line.
> However, I'd rather see the "do" at the end of this line, in which
> case you'd retain the semi-colon.
>
>
done.
>> +do
>> +SVNSERVE_PORT=$(($RANDOM+1024))
>>
>
> Need leading indentation.
>
done
>
>> +done
>> +
>> $SERVER_CMD -d -r $ABS_BUILDDIR/subversion/tests/cmdline \
>> --listen-host 127.0.0.1 \
>> + --listen-port $SVNSERVE_PORT \
>> --pid-file $SVNSERVE_PID &
>>
>> -BASE_URL="svn://localhost"
>> +BASE_URL="svn://localhost:$SVNSERVE_PORT"
>>
>> -make svncheck
>> +make check BASE_URL=$BASE_URL
>>
>
> Changing the make target doesn't seem unreasonable to me, but if done,
> should be a separate commit. I believe that doing so would also
> obviate the need for setting a BASE_URL environment variable in
> svnserveautocheck.sh.
>
>
Do you mean changing the 'svncheck' target?
+make check BASE_URL=$BASE_URL is more like that of davautocheck.
With regards
Kamesh Jayachandran
Re: [PATCH]Make svnserve auto test instance to listen on random port
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 05 Feb 2007, Kamesh Jayachandran wrote:
> [[[
>
> Run the test svnserve instance on a random port so that it won't
> collide with existing instance of standard svnserve.
^^^
I'd remove the word "standard" from the log message.
>
> * subversion/tests/cmdline/svnserveautocheck.sh
> Run 'svnserve' on a random free to LISTEN port.
>
> Patch by: kameshj
> ]]]
I'm in favor of this concept, but have some comments about the
implementation.
> Index: subversion/tests/cmdline/svnserveautocheck.sh
> ===================================================================
> --- subversion/tests/cmdline/svnserveautocheck.sh (revision 23345)
> +++ subversion/tests/cmdline/svnserveautocheck.sh (working copy)
> @@ -60,13 +60,20 @@
>
> rm -f $SVNSERVE_PID
>
> +SVNSERVE_PORT=$(($RANDOM+1024))
$RANDOM should works with /bin/bash, which is apparently already being
demanded by this script.
> +while netstat -an|grep $SVNSERVE_PORT|grep "LISTEN";
Will netstat always be in the user's path? On my Fedora Core 6 box,
it's in /bin, which is fine. How about on Solaris, OS X, etc.?
As written, you shouldn't need the semi-colon at the end of the line.
However, I'd rather see the "do" at the end of this line, in which
case you'd retain the semi-colon.
> +do
> +SVNSERVE_PORT=$(($RANDOM+1024))
Need leading indentation.
> +done
> +
> $SERVER_CMD -d -r $ABS_BUILDDIR/subversion/tests/cmdline \
> --listen-host 127.0.0.1 \
> + --listen-port $SVNSERVE_PORT \
> --pid-file $SVNSERVE_PID &
>
> -BASE_URL="svn://localhost"
> +BASE_URL="svn://localhost:$SVNSERVE_PORT"
>
> -make svncheck
> +make check BASE_URL=$BASE_URL
Changing the make target doesn't seem unreasonable to me, but if done,
should be a separate commit. I believe that doing so would also
obviate the need for setting a BASE_URL environment variable in
svnserveautocheck.sh.
> r=$?
>
> really_cleanup