Re: svn commit: r1667524 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

[Evgeny Kotkov <ev...@visualsvn.com>]

Stefan Fuhrmann <st...@apache.org> writes:

>  static svn_error_t *
> -test_fsfs_config_opts(const svn_test_opts_t *opts,
> -                      apr_pool_t *pool)
> +test_create_with_config_opts(const svn_test_opts_t *opts,
> +                             apr_pool_t *pool)
>  {
>    apr_hash_t *fs_config;
>    svn_fs_t *fs;
> @@ -6722,9 +6722,9 @@ test_fsfs_config_opts(const svn_test_opt
>    const svn_fs_fsfs_info_t *fsfs_info;
>
>    /* Bail (with SKIP) on known-untestable scenarios */
> -  if (strcmp(opts->fs_type, "fsfs") != 0)
> +  if (strcmp(opts->fs_type, SVN_FS_TYPE_BDB) == 0)
>      return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL,
> -                            "this will test FSFS repositories only");
> +                            "this will not test BDB repositories");

Is there a reason to run this test with --fs-type=fsx?  I think we'd be just
wasting time here, because the test itself doesn't rely on opts->fs_type,
and everything it does is FSFS-specific.

When creating file systems in this test, we specify SVN_FS_TYPE_FSFS and
FSFS-specific config options; when checking what we've got, we downcast the
information structure to an FSFS-specific svn_fs_fsfs_info_t.  I think that the
test was designed to run only with --fs-type=fsfs.

Am I missing something?


Regards,
Evgeny Kotkov

Re: svn commit: r1667524 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

[Stefan Fuhrmann <st...@wandisco.com>]

On Thu, Mar 19, 2015 at 10:34 AM, Evgeny Kotkov <evgeny.kotkov@visualsvn.com
> wrote:

> Stefan Fuhrmann <st...@apache.org> writes:
>
> >  static svn_error_t *
> > -test_fsfs_config_opts(const svn_test_opts_t *opts,
> > -                      apr_pool_t *pool)
> > +test_create_with_config_opts(const svn_test_opts_t *opts,
> > +                             apr_pool_t *pool)
> >  {
> >    apr_hash_t *fs_config;
> >    svn_fs_t *fs;
> > @@ -6722,9 +6722,9 @@ test_fsfs_config_opts(const svn_test_opt
> >    const svn_fs_fsfs_info_t *fsfs_info;
> >
> >    /* Bail (with SKIP) on known-untestable scenarios */
> > -  if (strcmp(opts->fs_type, "fsfs") != 0)
> > +  if (strcmp(opts->fs_type, SVN_FS_TYPE_BDB) == 0)
> >      return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL,
> > -                            "this will test FSFS repositories only");
> > +                            "this will not test BDB repositories");
>
> Is there a reason to run this test with --fs-type=fsx?  I think we'd be
> just
> wasting time here, because the test itself doesn't rely on opts->fs_type,
> and everything it does is FSFS-specific.
>
> When creating file systems in this test, we specify SVN_FS_TYPE_FSFS and
> FSFS-specific config options; when checking what we've got, we downcast the
> information structure to an FSFS-specific svn_fs_fsfs_info_t.  I think
> that the
> test was designed to run only with --fs-type=fsfs.
>

You are right, I missed that it is hard-coded to use FSFS.
Reverted to FSFS-only execution in r1668117.

I simply wanted to remove as many fs type restrictions
as possible, i.e. maximize coverage, w/o making the test
fail.

Am I missing something?
>

Tests that are truly backend-specific should go into the
respective libsvn_fs_* folder. There is also a bit of test
duplication between FSFS and FSX there that should be
moved to FS. When I find time & energy, I might sort
that out for 1.10. Patches are welcome.

-- Stefan^2.