Ruby binding test of fs failing on windows

Hi kou,

in http://thread.gmane.org/gmane.comp.version-control.subversion.devel/84157/focus=84157
Brane, Vlad and I conclude that this (taken from test_fs.rb) is
probably not a good test:

  def test_create
    path = File.join(@tmp_path, "fs")
    fs_type = Svn::Fs::TYPE_BDB
    config = {Svn::Fs::CONFIG_FS_TYPE => fs_type}

    assert(!File.exist?(path))
    fs = Svn::Fs::FileSystem.create(path, config)
    assert(File.exist?(path))
    assert_equal(fs_type, Svn::Fs.type(path))
    fs.set_warning_func do |err|
      p err
      abort
    end
    assert_equal(path, fs.path)
    Svn::Fs::FileSystem.delete(path)
    assert(!File.exist?(path))
  end

even though it is passing on your platform. The issue is the
filesystem being open when we try to delete it. I deleted the test
from test_fs.rb locally, but I've since been thinking about it, and I
wanted to bounce an idea off of you about a couple small interface
changes (not sure if the implementation changes would be small or
not).

Interface changes:
- Add a close method to Svn::Fs (that would release the new pool
member added to Svn::Fs which would close the filesystem).
- Add a block form to Svn::Fs.open, Svn::Fs.new, and Svn::Fs.create
(with the obvious implementations).

Implementation changes:
Here I'm kind of fuzzy. I think that we'd have to add a pool member
to Svn::Fs (initialized as a "subpool" of the class variable pool?)
and pass it into Svn::Ext::Fs.create or Svn::Ext::Fs.open, which would
mean modifying their signatures and implementations to use it instead
of the global.

We could then modify the test to look like

  def test_create
    path = File.join(@tmp_path, "fs")
    fs_type = Svn::Fs::TYPE_BDB
    config = {Svn::Fs::CONFIG_FS_TYPE => fs_type}

    assert(!File.exist?(path))
    fs = Svn::Fs::FileSystem.create(path, config)
    assert(File.exist?(path))
    assert_equal(fs_type, Svn::Fs.type(path))
    fs.set_warning_func do |err|
      p err
      abort
    end
    assert_equal(path, fs.path)
    fs.close
    Svn::Fs::FileSystem.delete(path)
    assert(!File.exist?(path))
  end

or

  def test_create
    path = File.join(@tmp_path, "fs")
    fs_type = Svn::Fs::TYPE_BDB
    config = {Svn::Fs::CONFIG_FS_TYPE => fs_type}

    assert(!File.exist?(path))
    fs = Svn::Fs::FileSystem.create(path, config) {
      assert(File.exist?(path))
      assert_equal(fs_type, Svn::Fs.type(path))
      fs.set_warning_func do |err|
        p err
        abort
      end
      assert_equal(path, fs.path)
    end
    Svn::Fs::FileSystem.delete(path)
    assert(!File.exist?(path))
  end

=============

There are similar issues with a test in test_repos.rb.

Many questions:
Should we just delete the test since it is a poor usage example?
Should we modify Svn::Fs as above? Or some other way?
Am I on the right track here at all?

I suppose the block forms could come latter, if you even think this is
an direction worth pursuing.

--
Joe Swatosh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org