Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test

From: Todd Zullinger <tmz@pobox.com>
Date: Dec 1, 2017, 03:40 UTC
Message-ID: <20171201034047.GC3693@zaya.teonanacatl.net>
In-Reply-To: <20171201030727.GH20640@aiede.mtv.corp.google.com>

Hi Jonathan,

Jonathan Nieder wrote:

Show 15 quoted lines

> 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.

Show 16 quoted lines

>> 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)