DataStore tests and slight modifications

lib/vagrant/data_store.rb

@@ -1,3 +1,5 @@
+require 'pathname'
+
 require 'log4r'
 
 module Vagrant
@@ -21,19 +23,16 @@ module Vagrant
       @logger    = Log4r::Logger.new("vagrant::datastore")
       @logger.info("Created: #{file_path}")
 
-      @file_path = file_path
-      return if !file_path
+      @file_path = Pathname.new(file_path)
 
-      raise Errors::DotfileIsDirectory if File.directory?(file_path)
+      if @file_path.exist?
+        raise Errors::DotfileIsDirectory if @file_path.directory?
 
-      if File.exist?(file_path)
-        File.open(file_path, "r") do |f|
-          begin
-            merge!(JSON.parse(f.read))
-          rescue JSON::ParserError
-            # Ignore if the data is invalid in the file.
-            @logger.error("Data store contained invalid JSON. Ignoring.")
-          end
+        begin
+          merge!(JSON.parse(@file_path.read))
+        rescue JSON::ParserError
+          # Ignore if the data is invalid in the file.
+          @logger.error("Data store contained invalid JSON. Ignoring.")
         end
       end
     end
@@ -48,10 +47,14 @@ module Vagrant
       if empty?
         # Delete the file since an empty data store is not useful
         @logger.info("Deleting data store since we're empty: #{@file_path}")
-        File.delete(@file_path) if File.file?(@file_path)
+        @file_path.delete if @file_path.exist?
       else
         @logger.info("Committing data to data store: #{@file_path}")
-        File.open(@file_path, "w") { |f| f.write(to_json) }
+
+        @file_path.open("w") do |f|
+          f.write(to_json)
+          f.fsync
+        end
       end
     end
 

test/unit/vagrant/data_store_test.rb

@@ -0,0 +1,67 @@
+require File.expand_path("../../base", __FILE__)
+
+require 'pathname'
+
+describe Vagrant::DataStore do
+  include_context "unit"
+
+  let(:db_file) do
+    # We create a tempfile and force an explicit close/unlink
+    # but save the path so that we can re-use it multiple times
+    temp = Tempfile.new("vagrant")
+    result = Pathname.new(temp.path)
+    temp.close
+    temp.unlink
+
+    result
+  end
+
+  let(:instance)    { described_class.new(db_file) }
+
+  it "initializes a new DB file" do
+    instance[:data] = true
+    instance.commit
+    instance[:data].should == true
+
+    test = described_class.new(db_file)
+    test[:data].should == true
+  end
+
+  it "initializes empty if the file contains invalid data" do
+    db_file.open("w+") { |f| f.write("NOPE!") }
+    described_class.new(db_file).should be_empty
+  end
+
+  it "initializes empty if the file doesn't exist" do
+    described_class.new("NOPENOPENOPENOPENPEPEPEPE").should be_empty
+  end
+
+  it "raises an error if the path given is a directory" do
+    db_file.delete if db_file.exist?
+    db_file.mkdir
+
+    expect { described_class.new(db_file) }.
+      to raise_error(Vagrant::Errors::DotfileIsDirectory)
+  end
+
+  it "cleans nil and empties when committing" do
+    instance[:data] = { :bar => nil }
+    instance[:another] = {}
+    instance.commit
+
+    # The instance is now empty because the data was nil
+    instance.should be_empty
+  end
+
+  it "deletes the data file if the store is empty when saving" do
+    instance[:data] = true
+    instance.commit
+
+    another = described_class.new(db_file)
+    another[:data] = nil
+    another.commit
+
+    # The file should no longer exist
+    db_file.should_not be_exist
+  end
+end

test/unit_legacy/vagrant/data_store_test.rb

@@ -1,85 +0,0 @@
-require "fileutils"
-require "test_helper"
-
-class DataStoreTest < Test::Unit::TestCase
-  setup do
-    @klass = Vagrant::DataStore
-    @initial_data = { "foo" => "bar" }
-    @db_file = File.join(tmp_path, "data_store_test")
-    File.open(@db_file, "w") { |f| f.write(@initial_data.to_json) }
-
-    @instance = @klass.new(@db_file)
-  end
-
-  teardown do
-    File.delete(@db_file) if File.exist?(@db_file)
-  end
-
-  should "raise an exception if the db file is a directory" do
-    file = tmp_path.join("data_store_folder_test")
-    FileUtils.mkdir_p(file)
-    assert_raises (Vagrant::Errors::DotfileIsDirectory) {
-      @klass.new(file)
-    }
-  end
-
-  should "initialize just fine if the db file contains invalid data" do
-    file = tmp_path.join("data_store_empty_test")
-    File.open(file, "w") { |f| f.write("") }
-
-    instance = @klass.new(file)
-    assert instance.length == 0
-  end
-
-  should "be a hash with indifferent access" do
-    assert @instance.is_a?(Vagrant::Util::HashWithIndifferentAccess)
-  end
-
-  should "just be an empty hash if file doesn't exist" do
-    assert @klass.new("NEvERNENVENRNE").empty?
-  end
-
-  should "read the data" do
-    assert_equal @initial_data["foo"], @instance[:foo]
-  end
-
-  should "read the data by stringifying keys" do
-    @instance[:bar] = { "baz" => "yay" }
-    @instance.commit
-    @instance = @klass.new(@db_file)
-    assert_equal "yay", @instance[:bar]["baz"]
-  end
-
-  should "write the data, but not save it right away" do
-    @instance[:foo] = "changed"
-    assert_equal "changed", @instance[:foo]
-    assert_equal @initial_data["foo"], @klass.new(@db_file)["foo"]
-  end
-
-  should "write the data if commit is called" do
-    @instance[:foo] = "changed"
-    @instance.commit
-
-    assert_equal "changed", @klass.new(@db_file)[:foo]
-  end
-
-  should "delete the data store file if the hash is empty" do
-    @instance[:foo] = :bar
-    @instance.commit
-    assert File.exist?(@db_file)
-
-    @instance.clear
-    assert @instance.empty?
-    @instance.commit
-    assert !File.exist?(@db_file)
-  end
-
-  should "clean nil and empties if commit is called" do
-    @instance[:foo] = { :bar => nil }
-    @instance[:bar] = {}
-    @instance.commit
-
-    assert !@instance.has_key?(:foo)
-    assert !@instance.has_key?(:bar)
-  end
-end