Stefan Fuhrmann wrote on Tue, Aug 20, 2013 at 11:05:28 +0200: > On Mon, Aug 19, 2013 at 2:02 PM, Daniel Shahaf <danie...@elego.de> wrote: > > > danie...@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.