git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
@ 2017-12-01  2:32 Todd Zullinger
  2017-12-01  2:32 ` [PATCH 1/2] t/lib-git-svn: whitespace cleanup Todd Zullinger
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  2:32 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

These tests are not run by default nor are they enabled in travis-ci.  I
don't know how much testing they get in user or other packager builds.

I've been slowly increasing the test suite usage in fedora builds.  I
ran into this while testing locally with parallel make test.  The
official fedora builds don't run in parallel (yet), as even before I ran
into this issue, builds on the fedora builders randomly failed too
often.  I'm hoping to eventually enable parallel tests by default
though, since it's so much faster.

I'm not sure if there's any objection to changing the variable needed to
enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE.  The way
SVNSERVE_PORT is set in this patch should allow the port to be set
explicitly, in case someone requires that -- and they understand that it
can fail if running parallel tests, of course.  Whether that's a
feature or a bug, I'm not sure. :)

The indentation of lib-git-svn.sh didn't use tabs consistently, in only
a few places, so I cleaned that up first.  I can drop that change if
it's unwanted.

Todd Zullinger (2):
  t/lib-git-svn: whitespace cleanup
  t/lib-git-svn.sh: improve svnserve tests with parallel make test

 t/lib-git-svn.sh | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

-- 
2.15.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/2] t/lib-git-svn: whitespace cleanup
  2017-12-01  2:32 [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
@ 2017-12-01  2:32 ` Todd Zullinger
  2017-12-01  3:04   ` Jonathan Nieder
  2017-12-01  2:32 ` [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  2:32 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 t/lib-git-svn.sh | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 688313ed5c..84366b2624 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -17,8 +17,8 @@ SVN_TREE=$GIT_SVN_DIR/svn-tree
 svn >/dev/null 2>&1
 if test $? -ne 1
 then
-    skip_all='skipping git svn tests, svn not found'
-    test_done
+	skip_all='skipping git svn tests, svn not found'
+	test_done
 fi
 
 svnrepo=$PWD/svnrepo
@@ -110,18 +110,18 @@ EOF
 }
 
 require_svnserve () {
-    if test -z "$SVNSERVE_PORT"
-    then
-	skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
-        test_done
-    fi
+	if test -z "$SVNSERVE_PORT"
+	then
+		skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
+		test_done
+	fi
 }
 
 start_svnserve () {
-    svnserve --listen-port $SVNSERVE_PORT \
-             --root "$rawsvnrepo" \
-             --listen-once \
-             --listen-host 127.0.0.1 &
+	svnserve --listen-port $SVNSERVE_PORT \
+		 --root "$rawsvnrepo" \
+		 --listen-once \
+		 --listen-host 127.0.0.1 &
 }
 
 prepare_a_utf8_locale () {
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  2:32 [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
  2017-12-01  2:32 ` [PATCH 1/2] t/lib-git-svn: whitespace cleanup Todd Zullinger
@ 2017-12-01  2:32 ` Todd Zullinger
  2017-12-01  3:02   ` Jonathan Nieder
  2017-12-01  2:56 ` [PATCH 0/2] " Eric Wong
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  2:32 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

Previously, setting SVNSERVE_PORT enabled several tests which require a
local svnserve daemon to be run (in t9113 & t9126).  The tests share the
setup of the local svnserve via `start_svnserve()`.  The function uses
the svnserve option `--listen-once` which causes svnserve to accept one
connection on the port, serve it, and exit.  When running the tests in
parallel this fails if one test tries to start svnserve while the other
is still running.

Use the test number as the svnserve port (similar to httpd tests) to
avoid port conflicts.  Set GIT_TEST_SVNSERVE to any value other than
'false' or 'auto' to enable these tests.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 t/lib-git-svn.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 84366b2624..4c1f81f167 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -110,14 +110,16 @@ EOF
 }
 
 require_svnserve () {
-	if test -z "$SVNSERVE_PORT"
+	test_tristate GIT_TEST_SVNSERVE
+	if ! test "$GIT_TEST_SVNSERVE" = true
 	then
-		skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
+		skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to enable)'
 		test_done
 	fi
 }
 
 start_svnserve () {
+	SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
 	svnserve --listen-port $SVNSERVE_PORT \
 		 --root "$rawsvnrepo" \
 		 --listen-once \
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  2:32 [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
  2017-12-01  2:32 ` [PATCH 1/2] t/lib-git-svn: whitespace cleanup Todd Zullinger
  2017-12-01  2:32 ` [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
@ 2017-12-01  2:56 ` Eric Wong
  2017-12-01  3:40   ` Todd Zullinger
  2017-12-01  3:07 ` Jonathan Nieder
  2017-12-14 18:19 ` Todd Zullinger
  4 siblings, 1 reply; 20+ messages in thread
From: Eric Wong @ 2017-12-01  2:56 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: git

Todd Zullinger <tmz@pobox.com> wrote:
> These tests are not run by default nor are they enabled in travis-ci.  I
> don't know how much testing they get in user or other packager builds.
> 
> I've been slowly increasing the test suite usage in fedora builds.  I
> ran into this while testing locally with parallel make test.  The
> official fedora builds don't run in parallel (yet), as even before I ran
> into this issue, builds on the fedora builders randomly failed too
> often.  I'm hoping to eventually enable parallel tests by default
> though, since it's so much faster.

Cool.

> I'm not sure if there's any objection to changing the variable needed to
> enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE.  The way
> SVNSERVE_PORT is set in this patch should allow the port to be set
> explicitly, in case someone requires that -- and they understand that it
> can fail if running parallel tests, of course.  Whether that's a
> feature or a bug, I'm not sure. :)

I'm fine with this for now.  Since svnserve (and git-daemon)
both support inetd behavior, I think we can eventually have a
test helper which binds random ports and pretends to be an
inetd, letting the test run without any special setup.

It would let multiple test instances run in parallel, even.

> The indentation of lib-git-svn.sh didn't use tabs consistently, in only
> a few places, so I cleaned that up first.  I can drop that change if
> it's unwanted.

Fine by me.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  2:32 ` [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
@ 2017-12-01  3:02   ` Jonathan Nieder
  2017-12-01  3:45     ` Todd Zullinger
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Nieder @ 2017-12-01  3:02 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: git, Eric Wong

Hi,

Todd Zullinger wrote:

> Previously, setting SVNSERVE_PORT enabled several tests which require a
> local svnserve daemon to be run (in t9113 & t9126).  The tests share the
> setup of the local svnserve via `start_svnserve()`.  The function uses
> the svnserve option `--listen-once` which causes svnserve to accept one
> connection on the port, serve it, and exit.  When running the tests in
> parallel this fails if one test tries to start svnserve while the other
> is still running.

I had trouble reading this because I didn't know what previous time it
was referring to.  Is it about how the option currently behaves?

(Git's commit messages tend to use the present tense to describe the
behavior before the patch, like a bug report, and the imperative to
describe the change the patch proposes to make, like an impolite bug
report. :))

> Use the test number as the svnserve port (similar to httpd tests) to
> avoid port conflicts.  Set GIT_TEST_SVNSERVE to any value other than
> 'false' or 'auto' to enable these tests.

This uses imperative in two ways and also ended up confusing me.  The
second one is a direction to me, not Git, right?  How about:

	Use the test number instead of $SVNSERVE_PORT as the svnserve
	port (similar to httpd tests) to avoid port conflicts.
	Developers can set GIT_TEST_SVNSERVE to any value other than
	'false' or 'auto' to enable these tests.
>
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
>  t/lib-git-svn.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

The patch looks good.  Thanks.

> diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
> index 84366b2624..4c1f81f167 100644
> --- a/t/lib-git-svn.sh
> +++ b/t/lib-git-svn.sh
> @@ -110,14 +110,16 @@ EOF
>  }
>  
>  require_svnserve () {
> -	if test -z "$SVNSERVE_PORT"
> +	test_tristate GIT_TEST_SVNSERVE
> +	if ! test "$GIT_TEST_SVNSERVE" = true
>  	then
> -		skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
> +		skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to enable)'
>  		test_done
>  	fi
>  }
>  
>  start_svnserve () {
> +	SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
>  	svnserve --listen-port $SVNSERVE_PORT \
>  		 --root "$rawsvnrepo" \
>  		 --listen-once \
> -- 
> 2.15.1
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] t/lib-git-svn: whitespace cleanup
  2017-12-01  2:32 ` [PATCH 1/2] t/lib-git-svn: whitespace cleanup Todd Zullinger
@ 2017-12-01  3:04   ` Jonathan Nieder
  2017-12-01  3:42     ` Todd Zullinger
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Nieder @ 2017-12-01  3:04 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: git, Eric Wong

Todd Zullinger wrote:

> Subject: t/lib-git-svn: whitespace cleanup
>
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
>  t/lib-git-svn.sh | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.

nit: it would have been a tiny bit easier to review if the commit
message mentioned that this is only changing the indentation from an
inconsistent space/tab mixture to tabs and isn't making any other
changes.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  2:32 [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
                   ` (2 preceding siblings ...)
  2017-12-01  2:56 ` [PATCH 0/2] " Eric Wong
@ 2017-12-01  3:07 ` Jonathan Nieder
  2017-12-01  3:40   ` Todd Zullinger
  2017-12-14 18:19 ` Todd Zullinger
  4 siblings, 1 reply; 20+ messages in thread
From: Jonathan Nieder @ 2017-12-01  3:07 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: git, Eric Wong

Todd Zullinger wrote:

> These tests are not run by default nor are they enabled in travis-ci.  I
> don't know how much testing they get in user or other packager builds.
>
> I've been slowly increasing the test suite usage in fedora builds.  I
> ran into this while testing locally with parallel make test.  The
> official fedora builds don't run in parallel (yet), as even before I ran
> into this issue, builds on the fedora builders randomly failed too
> often.  I'm hoping to eventually enable parallel tests by default
> though, since it's so much faster.

This background could go in the commit message for patch 2, but it
also speaks for itself as an obviously good change so I could go
either way.

> I'm not sure if there's any objection to changing the variable needed to
> enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE.  The way
> SVNSERVE_PORT is set in this patch should allow the port to be set
> explicitly, in case someone requires that -- and they understand that it
> can fail if running parallel tests, of course.  Whether that's a
> feature or a bug, I'm not sure. :)

micronit: can this just say something like

	Patch 2 is the important one --- see that one for rationale.

	Patch 1 is an optional preparatory style cleanup.

next time?  That way, you get an automatic guarantee that all the
important information is available in "git log" output to people who
need it later.

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  2:56 ` [PATCH 0/2] " Eric Wong
@ 2017-12-01  3:40   ` Todd Zullinger
  0 siblings, 0 replies; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  3:40 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Hi Eric,

Eric Wong wrote:
> I'm fine with this for now.  Since svnserve (and git-daemon)
> both support inetd behavior, I think we can eventually have a
> test helper which binds random ports and pretends to be an
> inetd, letting the test run without any special setup.
>
> It would let multiple test instances run in parallel, even.

Indeed, that would be a nice general improvement. :)

Thanks,

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Suppose I were a member of Congress, and suppose I were an idiot. But,
I repeat myself.
    -- Mark Twain


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  3:07 ` Jonathan Nieder
@ 2017-12-01  3:40   ` Todd Zullinger
  0 siblings, 0 replies; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  3:40 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Eric Wong

Hi Jonathan,

Jonathan Nieder wrote:
> Todd Zullinger wrote:
>
>> These tests are not run by default nor are they enabled in travis-ci.  I
>> don't know how much testing they get in user or other packager builds.
>>
>> I've been slowly increasing the test suite usage in fedora builds.  I
>> ran into this while testing locally with parallel make test.  The
>> official fedora builds don't run in parallel (yet), as even before I ran
>> into this issue, builds on the fedora builders randomly failed too
>> often.  I'm hoping to eventually enable parallel tests by default
>> though, since it's so much faster.
>
> This background could go in the commit message for patch 2, but it
> also speaks for itself as an obviously good change so I could go
> either way.

Heh.  If there's something in there in particular that seems useful, I
can certainly add it.  I'm not sure what parts of this text would be
beneficial to someone down the line though.

I usually err on the 'too much information' side of commit messages.
I'm happy that it's much harder to do that here.  I'd rather have to
skim a long message than wonder about the motivation for a change.

>> I'm not sure if there's any objection to changing the variable needed to
>> enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE.  The way
>> SVNSERVE_PORT is set in this patch should allow the port to be set
>> explicitly, in case someone requires that -- and they understand that it
>> can fail if running parallel tests, of course.  Whether that's a
>> feature or a bug, I'm not sure. :)
>
> micronit: can this just say something like
>
> 	Patch 2 is the important one --- see that one for rationale.
>
> 	Patch 1 is an optional preparatory style cleanup.
>
> next time?  That way, you get an automatic guarantee that all the
> important information is available in "git log" output to people who
> need it later.

Yeah, I'll try to remember that.  I started this without the
whitespace cleanup and as I was writing in the single patch
description that I didn't think the whitespace cleanup was warranted,
I talked myself into doing it as the prep patch. :)

Thanks,

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Tradition: Just because you've always done it that way doesn't mean
it's not incredibly stupid.
    -- Demotivators (www.despair.com)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] t/lib-git-svn: whitespace cleanup
  2017-12-01  3:04   ` Jonathan Nieder
@ 2017-12-01  3:42     ` Todd Zullinger
  2017-12-01  3:56       ` Jonathan Nieder
  0 siblings, 1 reply; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  3:42 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Eric Wong

Hi Jonathan,

Jonathan Nieder wrote:
> nit: it would have been a tiny bit easier to review if the commit
> message mentioned that this is only changing the indentation from an
> inconsistent space/tab mixture to tabs and isn't making any other
> changes.

If only you saw how many times I typed a subject and changed it
before settling on the terse version...

How about:

    t/lib-git-svn: cleanup inconsistent tab/space usage

?

Thanks,

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
How am I supposed to hallucinate with all these swirling colors
distracting me?
    -- Lisa Simpson


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  3:02   ` Jonathan Nieder
@ 2017-12-01  3:45     ` Todd Zullinger
  2017-12-01  3:53       ` Jonathan Nieder
  0 siblings, 1 reply; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  3:45 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Eric Wong

Hi Jonathan,

Jonathan Nieder wrote:
> Todd Zullinger wrote:
>
>> Previously, setting SVNSERVE_PORT enabled several tests which require a
>> local svnserve daemon to be run (in t9113 & t9126).  The tests share the
>> setup of the local svnserve via `start_svnserve()`.  The function uses
>> the svnserve option `--listen-once` which causes svnserve to accept one
>> connection on the port, serve it, and exit.  When running the tests in
>> parallel this fails if one test tries to start svnserve while the other
>> is still running.
>
> I had trouble reading this because I didn't know what previous time it
> was referring to.  Is it about how the option currently behaves?
>
> (Git's commit messages tend to use the present tense to describe the
> behavior before the patch, like a bug report, and the imperative to
> describe the change the patch proposes to make, like an impolite bug
> report. :))

This is what I get for skipping grammar classes to go hiking in my
youth.  But I'm sure I'd do it all again, if given the chance. ;)

>> Use the test number as the svnserve port (similar to httpd tests) to
>> avoid port conflicts.  Set GIT_TEST_SVNSERVE to any value other than
>> 'false' or 'auto' to enable these tests.
>
> This uses imperative in two ways and also ended up confusing me.  The
> second one is a direction to me, not Git, right?  How about:
>
> 	Use the test number instead of $SVNSERVE_PORT as the svnserve
> 	port (similar to httpd tests) to avoid port conflicts.
> 	Developers can set GIT_TEST_SVNSERVE to any value other than
> 	'false' or 'auto' to enable these tests.

Much better, thank you.  How about this for the full commit message:

    t/lib-git-svn.sh: improve svnserve tests with parallel make test

    Setting SVNSERVE_PORT enables several tests which require a local
    svnserve daemon to be run (in t9113 & t9126).  The tests share setup of
    the local svnserve via `start_svnserve()`.  The function uses svnserve's
    `--listen-once` option, which causes svnserve to accept one connection
    on the port, serve it, and exit.  When running the tests in parallel
    this fails if one test tries to start svnserve while the other is still
    running.

    Use the test number as the svnserve port (similar to httpd tests) to
    avoid port conflicts.  Developers can set GIT_TEST_SVNSERVE to any value
    other than 'false' or 'auto' to enable these tests.

?

Thanks,

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Curiosity killed the cat, but for awhile I was a suspect.
    -- Steven Wright


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  3:45     ` Todd Zullinger
@ 2017-12-01  3:53       ` Jonathan Nieder
  2017-12-01  4:11         ` Todd Zullinger
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Nieder @ 2017-12-01  3:53 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: git, Eric Wong

Todd Zullinger wrote:

> Much better, thank you.  How about this for the full commit message:
>
>    t/lib-git-svn.sh: improve svnserve tests with parallel make test
>
>    Setting SVNSERVE_PORT enables several tests which require a local
>    svnserve daemon to be run (in t9113 & t9126).  The tests share setup of
>    the local svnserve via `start_svnserve()`.  The function uses svnserve's
>    `--listen-once` option, which causes svnserve to accept one connection
>    on the port, serve it, and exit.  When running the tests in parallel
>    this fails if one test tries to start svnserve while the other is still
>    running.
>
>    Use the test number as the svnserve port (similar to httpd tests) to
>    avoid port conflicts.  Developers can set GIT_TEST_SVNSERVE to any value
>    other than 'false' or 'auto' to enable these tests.
>
> ?

Yep, with this description it is
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks for putting up with my nits. :)

Jonathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] t/lib-git-svn: whitespace cleanup
  2017-12-01  3:42     ` Todd Zullinger
@ 2017-12-01  3:56       ` Jonathan Nieder
  0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Nieder @ 2017-12-01  3:56 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: git, Eric Wong

Todd Zullinger wrote:
> Jonathan Nieder wrote:

>> nit: it would have been a tiny bit easier to review if the commit
>> message mentioned that this is only changing the indentation from an
>> inconsistent space/tab mixture to tabs and isn't making any other
>> changes.
>
> If only you saw how many times I typed a subject and changed it
> before settling on the terse version...

Heh.  No worries, it was a really small nit.

> How about:
>
>    t/lib-git-svn: cleanup inconsistent tab/space usage
>
> ?

Sure, looks good.

Thanks again,
Jonathan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  3:53       ` Jonathan Nieder
@ 2017-12-01  4:11         ` Todd Zullinger
  2017-12-01 15:56           ` [PATCH v2 1/2] t/lib-git-svn: cleanup inconsistent tab/space usage Todd Zullinger
       [not found]           ` <20171201153241.27071-1-tmz@pobox.com>
  0 siblings, 2 replies; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01  4:11 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Eric Wong

Jonathan Nieder wrote:
> Yep, with this description it is 
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Thanks for putting up with my nits. :)

Thank you for taking the time and looking at the details. :)

I'll send a v2 with the changes in the morning, in case there are any 
other comments (but mostly because it's late and time for a swim).

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It is impossible to enjoy idling thoroughly unless one has plenty of
work to do.
    -- Jerome K. Jerome


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/2] t/lib-git-svn: cleanup inconsistent tab/space usage
  2017-12-01  4:11         ` Todd Zullinger
@ 2017-12-01 15:56           ` Todd Zullinger
       [not found]           ` <20171201153241.27071-1-tmz@pobox.com>
  1 sibling, 0 replies; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01 15:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong, Jonathan Nieder

Acked-by: Eric Wong <e@80x24.org>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 t/lib-git-svn.sh | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 688313ed5c..84366b2624 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -17,8 +17,8 @@ SVN_TREE=$GIT_SVN_DIR/svn-tree
 svn >/dev/null 2>&1
 if test $? -ne 1
 then
-    skip_all='skipping git svn tests, svn not found'
-    test_done
+	skip_all='skipping git svn tests, svn not found'
+	test_done
 fi
 
 svnrepo=$PWD/svnrepo
@@ -110,18 +110,18 @@ EOF
 }
 
 require_svnserve () {
-    if test -z "$SVNSERVE_PORT"
-    then
-	skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
-        test_done
-    fi
+	if test -z "$SVNSERVE_PORT"
+	then
+		skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
+		test_done
+	fi
 }
 
 start_svnserve () {
-    svnserve --listen-port $SVNSERVE_PORT \
-             --root "$rawsvnrepo" \
-             --listen-once \
-             --listen-host 127.0.0.1 &
+	svnserve --listen-port $SVNSERVE_PORT \
+		 --root "$rawsvnrepo" \
+		 --listen-once \
+		 --listen-host 127.0.0.1 &
 }
 
 prepare_a_utf8_locale () {
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
       [not found]           ` <20171201153241.27071-1-tmz@pobox.com>
@ 2017-12-01 15:56             ` Todd Zullinger
  2017-12-01 20:06               ` Jonathan Nieder
  0 siblings, 1 reply; 20+ messages in thread
From: Todd Zullinger @ 2017-12-01 15:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong, Jonathan Nieder

Setting SVNSERVE_PORT enables several tests which require a local
svnserve daemon to be run (in t9113 & t9126).  The tests share setup of
the local svnserve via `start_svnserve()`.  The function uses svnserve's
`--listen-once` option, which causes svnserve to accept one connection
on the port, serve it, and exit.  When running the tests in parallel
this fails if one test tries to start svnserve while the other is still
running.

Use the test number as the svnserve port (similar to httpd tests) to
avoid port conflicts.  Developers can set GIT_TEST_SVNSERVE to any value
other than 'false' or 'auto' to enable these tests.

Acked-by: Eric Wong <e@80x24.org>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 t/lib-git-svn.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 84366b2624..4c1f81f167 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -110,14 +110,16 @@ EOF
 }
 
 require_svnserve () {
-	if test -z "$SVNSERVE_PORT"
+	test_tristate GIT_TEST_SVNSERVE
+	if ! test "$GIT_TEST_SVNSERVE" = true
 	then
-		skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
+		skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to enable)'
 		test_done
 	fi
 }
 
 start_svnserve () {
+	SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
 	svnserve --listen-port $SVNSERVE_PORT \
 		 --root "$rawsvnrepo" \
 		 --listen-once \
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01 15:56             ` [PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
@ 2017-12-01 20:06               ` Jonathan Nieder
  0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Nieder @ 2017-12-01 20:06 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: Junio C Hamano, git, Eric Wong

Todd Zullinger wrote:

> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
>  t/lib-git-svn.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

This and the previous one are indeed still
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-01  2:32 [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
                   ` (3 preceding siblings ...)
  2017-12-01  3:07 ` Jonathan Nieder
@ 2017-12-14 18:19 ` Todd Zullinger
  2017-12-14 18:45   ` Junio C Hamano
  4 siblings, 1 reply; 20+ messages in thread
From: Todd Zullinger @ 2017-12-14 18:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong, Jonathan Nieder

Hi Junio,

I wanted to check if this minor patch series had slipped
under your radar.  If it's waiting patiently in your queue
for other topics to advance, that's fine, of course. :)

Finished patches: <20171201155653.29553-1-tmz@pobox.com> and
<20171201155653.29553-2-tmz@pobox.com>.

Thanks,

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Anyone who is capable of getting themselves made President should on
no account be allowed to do the job.
    -- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-14 18:19 ` Todd Zullinger
@ 2017-12-14 18:45   ` Junio C Hamano
  2017-12-14 20:35     ` Todd Zullinger
  0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2017-12-14 18:45 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: git, Eric Wong, Jonathan Nieder

Todd Zullinger <tmz@pobox.com> writes:

> I wanted to check if this minor patch series had slipped
> under your radar.

Totally.  Queued.

As they come with Ack by the area maintainer, I'll fast-track them
down to 'master' (other topics typically cook at least for a week in
'next').

Thanks for pinging.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
  2017-12-14 18:45   ` Junio C Hamano
@ 2017-12-14 20:35     ` Todd Zullinger
  0 siblings, 0 replies; 20+ messages in thread
From: Todd Zullinger @ 2017-12-14 20:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong, Jonathan Nieder

Junio C Hamano wrote:
> Todd Zullinger <tmz@pobox.com> writes:
>> I wanted to check if this minor patch series had slipped
>> under your radar.
> 
> Totally.  Queued.
> 
> As they come with Ack by the area maintainer, I'll fast-track them
> down to 'master' (other topics typically cook at least for a week in
> 'next').
> 
> Thanks for pinging.

Thank you, as always.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
There is considerable overlap between the intelligence of the smartest
bears and the dumbest tourists.
  -- Park ranger yro.slashdot.org/comments.pl?sid=191810&cid=15757347


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-12-14 20:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01  2:32 [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
2017-12-01  2:32 ` [PATCH 1/2] t/lib-git-svn: whitespace cleanup Todd Zullinger
2017-12-01  3:04   ` Jonathan Nieder
2017-12-01  3:42     ` Todd Zullinger
2017-12-01  3:56       ` Jonathan Nieder
2017-12-01  2:32 ` [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
2017-12-01  3:02   ` Jonathan Nieder
2017-12-01  3:45     ` Todd Zullinger
2017-12-01  3:53       ` Jonathan Nieder
2017-12-01  4:11         ` Todd Zullinger
2017-12-01 15:56           ` [PATCH v2 1/2] t/lib-git-svn: cleanup inconsistent tab/space usage Todd Zullinger
     [not found]           ` <20171201153241.27071-1-tmz@pobox.com>
2017-12-01 15:56             ` [PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Todd Zullinger
2017-12-01 20:06               ` Jonathan Nieder
2017-12-01  2:56 ` [PATCH 0/2] " Eric Wong
2017-12-01  3:40   ` Todd Zullinger
2017-12-01  3:07 ` Jonathan Nieder
2017-12-01  3:40   ` Todd Zullinger
2017-12-14 18:19 ` Todd Zullinger
2017-12-14 18:45   ` Junio C Hamano
2017-12-14 20:35     ` Todd Zullinger

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).