Re: Should this test pass?

Hi Vlad,

On 1/10/07, Vlad Georgescu <vgeorgescu@gmail.com> wrote:
> Joe Swatosh wrote:
> > Hi Brane,
> >
> > On 1/9/07, Branko Čibej <brane@xbc.nu> wrote:
> >> Joe Swatosh wrote:
> >>> Hi All,
> >>> As I've been trying to get the Ruby bindings tests to pass on windows,
> >>> I ran into a failing test. If I changed from BDB to FSFS the Ruby
> >>> version of the test passes.
> >>>
> >>> I rewrote it as a C test, just to make sure that Ruby wasn't involved
> >>> at all and sure enough the test below fails, but if the fs_type is
> >>> changed to fsfs it passes.
> >>>
> >>> Perhaps I've screwed up the build somehow? Screwed up somehow that
> >>> wasn't detected by the existing tests?
> >> You don't say how the test fails, but I'm pretty sure you can't delete a
> >> BDB filesystem while it's still open.
> >>
> >> -- Brane
> >>
> >
> > I realized this morning that this was a pretty boneheaded report.
> > Let's see if I can do better.
> >
> > With this patch:
> > Index: svn-trunk-wc/subversion/tests/libsvn_fs/fs-test.c
> > ===================================================================
> > --- svn-trunk-wc/subversion/tests/libsvn_fs/fs-test.c (revision 22939)
> > +++ svn-trunk-wc/subversion/tests/libsvn_fs/fs-test.c (working copy)
> > @@ -4501,6 +4501,28 @@
> > }
> >
> >
> > +static svn_error_t *
> > +test_delete_fs(const char **msg,
> > + svn_boolean_t msg_only,
> > + svn_test_opts_t *opts,
> > + apr_pool_t *pool)
> > +{
> > + svn_fs_t *fs;
> > + const char *name = "test-fs-deletion";
> > + apr_hash_t *fs_config = apr_hash_make(pool);
> > + apr_hash_set(fs_config, SVN_FS_CONFIG_BDB_TXN_NOSYNC,
> > + APR_HASH_KEY_STRING, "1");
> > + apr_hash_set(fs_config, SVN_FS_CONFIG_FS_TYPE,
> > + APR_HASH_KEY_STRING,
> > + opts->fs_type);
> > + *msg = "create a file system then destroy it";
> > + if (msg_only)
> > + return SVN_NO_ERROR;
> > + SVN_ERR(svn_fs_create(&fs, name, fs_config, pool));
> > + SVN_ERR(svn_fs_delete_fs(name, pool));
>
> You can't delete the filesystem path while the filesystem is still open,
> because BDB only closes its databases at pool cleanup time. It seems the
> Ruby bindings aren't following these rules, but I don't know enough
> about SWIG or Ruby to help solve this problem.
>
> On a side note, Joe's patch revealed that we're leaking an error in the
> BDB code somewhere, which I'm currently tracking down.
>
> --
> Vlad
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>

So the original test should *NOT* pass, but this test should:

<code>
static svn_error_t *
test_delete_fs(const char **msg,
                    svn_boolean_t msg_only,
                    svn_test_opts_t *opts,
                    apr_pool_t *pool)
{
  svn_fs_t *fs;
  const char *name = "test-fs-deletion";
  apr_pool_t *private_pool = svn_pool_create(NULL);
  apr_hash_t *fs_config = apr_hash_make(private_pool);
  apr_hash_set(fs_config, SVN_FS_CONFIG_BDB_TXN_NOSYNC,
               APR_HASH_KEY_STRING, "1");
  apr_hash_set(fs_config, SVN_FS_CONFIG_FS_TYPE,
               APR_HASH_KEY_STRING,
               opts->fs_type);
  *msg = "create a file system then destroy it";

  if (msg_only)
    return SVN_NO_ERROR;

  SVN_ERR(svn_fs_create(&fs, name, fs_config, private_pool));
  svn_pool_destroy(private_pool);

  private_pool = svn_pool_create(NULL);
  SVN_ERR(svn_fs_delete_fs(name, private_pool));
  svn_pool_destroy(private_pool);

  return SVN_NO_ERROR;
}

</code>

and does. Thanks to both you and Brane for getting me pointed in the
right direction. I'll have to think about this some. I'm not sure of
the impacts on the Ruby bindings. Its interesting that the original
Ruby bindings tests pass on kou's platform.

Thanks again,

--
Joe Swatosh