You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Fuhrmann <st...@wandisco.com> on 2013/08/20 11:05:28 UTC
Re: svn commit: r1515378 - /subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
On Mon, Aug 19, 2013 at 2:02 PM, Daniel Shahaf <da...@elego.de> wrote:
> danielsh@apache.org wrote on Mon, Aug 19, 2013 at 11:55:04 -0000:
> > Author: danielsh
> > Date: Mon Aug 19 11:55:04 2013
> > New Revision: 1515378
> >
> > URL: http://svn.apache.org/r1515378
> > Log:
> > FS C tests: remove backend-dependent early out from tests that don't
> depend on
> > backend implementation details.
> >
> > * subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > (create_packed_filesystem): Only munge the format file for FSFS.
> > (read_packed_fs, commit_packed_fs, get_set_revprop_packed_fs,
> > get_set_large_revprop_packed_fs, get_set_huge_revprop_packed_fs,
> > recover_fully_packed, file_hint_at_shard_boundary, test_info,
> > pack_shard_size_one, get_set_multiple_huge_revprops_packed_fs):
> > As above.
> >
> > Modified:
> > subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> >
> > Modified:
> subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c?rev=1515378&r1=1515377&r2=1515378&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> (original)
> > +++ subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c Mon
> Aug 19 11:55:04 2013
> > @@ -154,11 +154,16 @@ create_packed_filesystem(const char *dir
> >
> > subpool = svn_pool_create(pool);
> >
> > - /* Rewrite the format file */
> > - SVN_ERR(svn_io_read_version_file(&version,
> > - svn_dirent_join(dir, "format",
> subpool),
> > - subpool));
> > - SVN_ERR(write_format(dir, version, shard_size, subpool));
> > + /* Rewrite the format file. (The rest of this function is
> backend-agnostic,
> > + so we just avoid adding the FSFS-specific format information if we
> run on
> > + some other backend.) */
> > + if ((strcmp(opts->fs_type, "fsfs") == 0))
> > + {
> > + SVN_ERR(svn_io_read_version_file(&version,
> > + svn_dirent_join(dir, "format",
> subpool),
> > + subpool));
> > + SVN_ERR(write_format(dir, version, shard_size, subpool));
> > + }
> >
> > /* Reopen the filesystem */
> > SVN_ERR(svn_fs_open(&fs, dir, NULL, subpool));
> > @@ -398,11 +403,6 @@ read_packed_fs(const svn_test_opts_t *op
> > svn_stringbuf_t *rstring;
> > svn_revnum_t i;
> >
> > - /* Bail (with success) on known-untestable scenarios */
> > - if ((strcmp(opts->fs_type, "fsfs") != 0)
> > - || (opts->server_minor_version && (opts->server_minor_version <
> 6)))
> > - return SVN_NO_ERROR;
> > -
>
> Stefan,
>
> With this change, the delta between fs-fs-pack-test.c and
> fs-x-pack-test.c is actually pretty minimal. Can you look into merging
> them into one file?
>
Hi Daniel,
I will review & probably apply the patch soon-ish.
However, FSX packing is going to be different from
FSFS in the future (multi-stage packing vs. shard
packing). Testing invariants can certainly be shared.
I guess those should be moved to e.g. FS tests.
-- Stefan^2.
Re: svn commit: r1515378 -
/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Tue, Aug 20, 2013 at 11:05:28 +0200:
> On Mon, Aug 19, 2013 at 2:02 PM, Daniel Shahaf <da...@elego.de> wrote:
>
> > danielsh@apache.org wrote on Mon, Aug 19, 2013 at 11:55:04 -0000:
> > > Author: danielsh
> > > Date: Mon Aug 19 11:55:04 2013
> > > New Revision: 1515378
> > >
> > > URL: http://svn.apache.org/r1515378
> > > Log:
> > > FS C tests: remove backend-dependent early out from tests that don't
> > depend on
> > > backend implementation details.
> > >
> > > * subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > > (create_packed_filesystem): Only munge the format file for FSFS.
> > > (read_packed_fs, commit_packed_fs, get_set_revprop_packed_fs,
> > > get_set_large_revprop_packed_fs, get_set_huge_revprop_packed_fs,
> > > recover_fully_packed, file_hint_at_shard_boundary, test_info,
> > > pack_shard_size_one, get_set_multiple_huge_revprops_packed_fs):
> > > As above.
> > >
> > > Modified:
> > > subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > >
> > > Modified:
> > subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c?rev=1515378&r1=1515377&r2=1515378&view=diff
> > >
> > ==============================================================================
> > > --- subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > (original)
> > > +++ subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c Mon
> > Aug 19 11:55:04 2013
> > > @@ -154,11 +154,16 @@ create_packed_filesystem(const char *dir
> > >
> > > subpool = svn_pool_create(pool);
> > >
> > > - /* Rewrite the format file */
> > > - SVN_ERR(svn_io_read_version_file(&version,
> > > - svn_dirent_join(dir, "format",
> > subpool),
> > > - subpool));
> > > - SVN_ERR(write_format(dir, version, shard_size, subpool));
> > > + /* Rewrite the format file. (The rest of this function is
> > backend-agnostic,
> > > + so we just avoid adding the FSFS-specific format information if we
> > run on
> > > + some other backend.) */
> > > + if ((strcmp(opts->fs_type, "fsfs") == 0))
> > > + {
> > > + SVN_ERR(svn_io_read_version_file(&version,
> > > + svn_dirent_join(dir, "format",
> > subpool),
> > > + subpool));
> > > + SVN_ERR(write_format(dir, version, shard_size, subpool));
> > > + }
> > >
> > > /* Reopen the filesystem */
> > > SVN_ERR(svn_fs_open(&fs, dir, NULL, subpool));
> > > @@ -398,11 +403,6 @@ read_packed_fs(const svn_test_opts_t *op
> > > svn_stringbuf_t *rstring;
> > > svn_revnum_t i;
> > >
> > > - /* Bail (with success) on known-untestable scenarios */
> > > - if ((strcmp(opts->fs_type, "fsfs") != 0)
> > > - || (opts->server_minor_version && (opts->server_minor_version <
> > 6)))
> > > - return SVN_NO_ERROR;
> > > -
> >
> > Stefan,
> >
> > With this change, the delta between fs-fs-pack-test.c and
> > fs-x-pack-test.c is actually pretty minimal. Can you look into merging
> > them into one file?
> >
>
> Hi Daniel,
>
> I will review & probably apply the patch soon-ish.
>
> However, FSX packing is going to be different from
> FSFS in the future (multi-stage packing vs. shard
> packing). Testing invariants can certainly be shared.
Right. Any tests not stat()ing specific files under the db/ directory,
for example, can and should run on all backends.
> I guess those should be moved to e.g. FS tests.
>
Yeah, they could live in tests/libsvn_fs/fs-pack-test.c which would be
run for all backends (including BDB) and individual test functions may
restrict themselves to a whitelisted list of backends. If
tests/libsvn_fs_fs/*-test.c are excluded from being run on BDB, we
should fix that.
Cheers,
Daniel
> -- Stefan^2.
Re: svn commit: r1515378 -
/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Tue, Aug 20, 2013 at 11:05:28 +0200:
> On Mon, Aug 19, 2013 at 2:02 PM, Daniel Shahaf <da...@elego.de> wrote:
>
> > danielsh@apache.org wrote on Mon, Aug 19, 2013 at 11:55:04 -0000:
> > > Author: danielsh
> > > Date: Mon Aug 19 11:55:04 2013
> > > New Revision: 1515378
> > >
> > > URL: http://svn.apache.org/r1515378
> > > Log:
> > > FS C tests: remove backend-dependent early out from tests that don't
> > depend on
> > > backend implementation details.
> > >
> > > * subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > > (create_packed_filesystem): Only munge the format file for FSFS.
> > > (read_packed_fs, commit_packed_fs, get_set_revprop_packed_fs,
> > > get_set_large_revprop_packed_fs, get_set_huge_revprop_packed_fs,
> > > recover_fully_packed, file_hint_at_shard_boundary, test_info,
> > > pack_shard_size_one, get_set_multiple_huge_revprops_packed_fs):
> > > As above.
> > >
> > > Modified:
> > > subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > >
> > > Modified:
> > subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c?rev=1515378&r1=1515377&r2=1515378&view=diff
> > >
> > ==============================================================================
> > > --- subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
> > (original)
> > > +++ subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c Mon
> > Aug 19 11:55:04 2013
> > > @@ -154,11 +154,16 @@ create_packed_filesystem(const char *dir
> > >
> > > subpool = svn_pool_create(pool);
> > >
> > > - /* Rewrite the format file */
> > > - SVN_ERR(svn_io_read_version_file(&version,
> > > - svn_dirent_join(dir, "format",
> > subpool),
> > > - subpool));
> > > - SVN_ERR(write_format(dir, version, shard_size, subpool));
> > > + /* Rewrite the format file. (The rest of this function is
> > backend-agnostic,
> > > + so we just avoid adding the FSFS-specific format information if we
> > run on
> > > + some other backend.) */
> > > + if ((strcmp(opts->fs_type, "fsfs") == 0))
> > > + {
> > > + SVN_ERR(svn_io_read_version_file(&version,
> > > + svn_dirent_join(dir, "format",
> > subpool),
> > > + subpool));
> > > + SVN_ERR(write_format(dir, version, shard_size, subpool));
> > > + }
> > >
> > > /* Reopen the filesystem */
> > > SVN_ERR(svn_fs_open(&fs, dir, NULL, subpool));
> > > @@ -398,11 +403,6 @@ read_packed_fs(const svn_test_opts_t *op
> > > svn_stringbuf_t *rstring;
> > > svn_revnum_t i;
> > >
> > > - /* Bail (with success) on known-untestable scenarios */
> > > - if ((strcmp(opts->fs_type, "fsfs") != 0)
> > > - || (opts->server_minor_version && (opts->server_minor_version <
> > 6)))
> > > - return SVN_NO_ERROR;
> > > -
> >
> > Stefan,
> >
> > With this change, the delta between fs-fs-pack-test.c and
> > fs-x-pack-test.c is actually pretty minimal. Can you look into merging
> > them into one file?
> >
>
> Hi Daniel,
>
> I will review & probably apply the patch soon-ish.
>
> However, FSX packing is going to be different from
> FSFS in the future (multi-stage packing vs. shard
> packing). Testing invariants can certainly be shared.
Right. Any tests not stat()ing specific files under the db/ directory,
for example, can and should run on all backends.
> I guess those should be moved to e.g. FS tests.
>
Yeah, they could live in tests/libsvn_fs/fs-pack-test.c which would be
run for all backends (including BDB) and individual test functions may
restrict themselves to a whitelisted list of backends. If
tests/libsvn_fs_fs/*-test.c are excluded from being run on BDB, we
should fix that.
Cheers,
Daniel
> -- Stefan^2.