On 17.03.2018 13:57, Philip Martin wrote:
> Julian Foad <julianf...@apache.org> writes:
>
>> But does that mean this code isn't being tested by any of our manual
>> or buildbot testing in the last 6 months?
> Yes, I think so.
>
> For svnserve the code only gets enabled when svnserve has block-read
> enabled and svnserveautocheck doesn't do that. By invoking svnserve
> manually it is possible to enable it and lots of tests fail with the
> zlib error:
>
> subversion/svnserve/svnserve -Tdr subversion/tests/cmdline --block-read 1
> make check BASE_URL=svn://localhost
>
> For http the code only gets enabled when mod_dav_svn has block read
> enabled and davautocheck doesn't do that. Patching davautocheck.sh
>
> Index: ../src/subversion/tests/cmdline/davautocheck.sh
> ===================================================================
> --- ../src/subversion/tests/cmdline/davautocheck.sh (revision 1827068)
> +++ ../src/subversion/tests/cmdline/davautocheck.sh (working copy)
> @@ -548,6 +548,7 @@
> Require valid-user
> SVNAdvertiseV2Protocol ${ADVERTISE_V2_PROTOCOL}
> SVNCacheRevProps ${CACHE_REVPROPS_SETTING}
> + SVNBlockRead on
> ${SVN_PATH_AUTHZ_LINE}
> </Location>
>
> causes lots of tests to fail with the zlib error.
>
> There doesn't seen to be a way to enable the block-read code for svn
> file:// access but it could be enabled for svnadmin by making the cache
> greater than 64MB. The regression tests use the default 16MB so
> patching main.py:
>
> Index: ../src/subversion/tests/cmdline/svntest/actions.py
> ===================================================================
> --- ../src/subversion/tests/cmdline/svntest/actions.py (revision
> 1827068)
> +++ ../src/subversion/tests/cmdline/svntest/actions.py (working copy)
> @@ -378,7 +378,7 @@
>
> def run_and_verify_dump(repo_dir, deltas=False):
> "Runs 'svnadmin dump' and reports any errors, returning the dump content."
> - args = ()
> + args = ('-M65')
> if deltas:
> args += ('--deltas',)
> exit_code, output, errput = run_and_verify_svnadmin(
>
> causes tests to fail with the zlib error.
>
>> Extending our testing is one option. I would much prefer to apply the
>> principle of testable design: "if this code only runs sometimes,
>> change it so it runs always". Can we do that?
> There are a lot of tuning options for caching. Do we want to add
> mechanisms to enable them individually in the testsuite? Do we want to
> change the defaults in our binaries? Do we want to eliminate the tuning
> options?
As long as we have tuning options, we should test them. It's not that
hard to add buildslave configurations.
As for eliminating them: it would be nice if the server could auto-tune
these parameters, but that is probably not a worthwhile goal to spend
our efforts at (and that would make testing behaviour with different
options harder).
-- Brane
