On Wed, Jun 19, 2013 at 03:50:01PM +0400, Ivan Zhakov wrote: > On Fri, Jun 14, 2013 at 6:40 PM, <s...@apache.org> wrote: > > Author: stsp > > Date: Fri Jun 14 14:40:17 2013 > > New Revision: 1493097 > > > > URL: http://svn.apache.org/r1493097 > > Log: > > Run tests with an exclusive lock on working copies. This should reduce test > > run time and also ensures that exclusive locking mode is tested. > > > > I've run ra_local and ra_serf tests with this change and got no failures. > > In any case, if there were any test failures with exlusive locking mode > > enabled, they'd most likely expose bugs in the test suite or Subversion > > itself. > > > I don't like this change actually: > 1. Running tests in different configuration than regular users are > using bad practice
I did think about this before making the change. Your argument can be turned around. If we never test the exclusive locking mode, how can we be sure that it works? And consider that, if a test passes with exclusive locking, it very likely passes with less restrictive locking. But the reverse is not true! Tests could fail in exclusive locking mode due to bugs in the tests or the code, and we would never see those failures until now. > 2. It also seems to broke svn benchmarks posted every week, because > now we get totally different numbers for operations. That's unfortunate. But what about things like server-side caching? Don't improvements in such areas have similar effects? I think having better test coverage and test speed is more important than keeping the benchmark results consistent over time. > Could you please make option to running test suite with exclusive > locking mode and leave it 'off' by default. Thanks! I could do that, yes. But it multiples the number of test configurations yet again, which I don't like. If we do this, I can switch my buildbot to use exclusive locks. And if the buildbot fails some day or we stop maintaining the bot, test coverage will get worse again because nobody tests exclusive mode anymore. So I'd rather have the default be 'on'.
