From 5aeee61e83927745010eed1a222e6eac5b28fa19 Mon Sep 17 00:00:00 2001 From: John Bender Date: Sun, 14 Mar 2010 22:34:48 -0700 Subject: [PATCH] action dependencies and unique requirement moved to actions/collection --- lib/vagrant.rb | 4 +- lib/vagrant/actions/base.rb | 14 +++ lib/vagrant/actions/collection.rb | 36 ++++++++ lib/vagrant/actions/runner.rb | 14 +-- test/test_helper.rb | 10 +++ test/vagrant/actions/collection_test.rb | 110 ++++++++++++++++++++++++ test/vagrant/actions/runner_test.rb | 18 ++-- 7 files changed, 187 insertions(+), 19 deletions(-) create mode 100644 lib/vagrant/actions/collection.rb create mode 100644 test/vagrant/actions/collection_test.rb diff --git a/lib/vagrant.rb b/lib/vagrant.rb index fafa85863..1b285604f 100644 --- a/lib/vagrant.rb +++ b/lib/vagrant.rb @@ -8,8 +8,8 @@ PROJECT_ROOT = File.join(libdir, '..') unless defined?(PROJECT_ROOT) end # The vagrant specific files which must be loaded prior to the rest -%w{vagrant/util vagrant/actions/base vagrant/downloaders/base vagrant/actions/runner - vagrant/config vagrant/provisioners/base vagrant/provisioners/chef}.each do |f| +%w{vagrant/util vagrant/actions/base vagrant/downloaders/base vagrant/actions/collection + vagrant/actions/runner vagrant/config vagrant/provisioners/base vagrant/provisioners/chef}.each do |f| require File.expand_path(f, libdir) end diff --git a/lib/vagrant/actions/base.rb b/lib/vagrant/actions/base.rb index ca18c368e..633a64a1e 100644 --- a/lib/vagrant/actions/base.rb +++ b/lib/vagrant/actions/base.rb @@ -82,6 +82,20 @@ module Vagrant # all your own exceptions, otherwise it'll mask the initially raised # exception. def rescue(exception); end + + # The following two methods are used for declaring action dependencies. + # For example, you require that the reload action be in place before + # a your new FooAction you might do the following + # + # def follows; [Reload] end + + # This method is called when the runner is determining the actions that + # must precede a given action. You would say "This action follows [Action1, Action2]" + def follows; [] end + + # This method is called when the runner is determining the actions that + # must follow a given action. You would say "This action precedes [Action3, Action4] + def precedes; [] end end # An exception which occured within an action. This should be used instead of diff --git a/lib/vagrant/actions/collection.rb b/lib/vagrant/actions/collection.rb new file mode 100644 index 000000000..805fe4203 --- /dev/null +++ b/lib/vagrant/actions/collection.rb @@ -0,0 +1,36 @@ +module Vagrant + module Actions + class Collection < Array + def dependencies! + each_with_index do |action, i| + action.follows.each do |klass| + unless self[0..i].klasses.include?(klass) + raise DependencyNotSatisfiedException.new("#{action.class} action must follow #{klass}") + end + end + + action.precedes.each do |klass| + unless self[i..length].klasses.include?(klass) + raise DependencyNotSatisfiedException.new("#{action.class} action must precede #{klass}") + end + end + end + end + + def duplicates? + klasses.uniq.size != size + end + + def duplicates! + raise DuplicateActionException.new if duplicates? + end + + def klasses + map { |o| o.class } + end + end + + class DuplicateActionException < Exception; end + class DependencyNotSatisfiedException < Exception; end + end +end diff --git a/lib/vagrant/actions/runner.rb b/lib/vagrant/actions/runner.rb index 920685cc3..a4f7901d6 100644 --- a/lib/vagrant/actions/runner.rb +++ b/lib/vagrant/actions/runner.rb @@ -47,7 +47,7 @@ module Vagrant # # @return [Array] def actions - @actions ||= [] + @actions ||= Actions::Collection.new end # Returns the first action instance which matches the given class. @@ -75,9 +75,9 @@ module Vagrant add_action(single_action, *args) end - # Raising it here might be too late and hard debug where the actions are comming from (meta actions) - raise DuplicateActionException.new if action_klasses.uniq.size < action_klasses.size - + actions.duplicates! + actions.dependencies! + # Call the prepare method on each once its # initialized, then call the execute! method begin @@ -127,12 +127,6 @@ module Vagrant end results end - - def action_klasses - actions.map { |a| a.class } - end end - - class DuplicateActionException < Exception; end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 07049ccfc..8ef0f667b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -76,6 +76,7 @@ class Test::Unit::TestCase vm = mock("vboxvm") mock_vm = mock("vm") action = action_klass.new(mock_vm, *args) + stub_default_action_dependecies(action) mock_vm.stubs(:vm).returns(vm) mock_vm.stubs(:vm=) @@ -86,6 +87,11 @@ class Test::Unit::TestCase [mock_vm, vm, action] end + def stub_default_action_dependecies(mock, klass=MockAction) + mock.stubs(:precedes).returns([]) + mock.stubs(:follows).returns([]) + end + # Sets up the mocks and stubs for a downloader def mock_downloader(downloader_klass) tempfile = mock("tempfile") @@ -94,3 +100,7 @@ class Test::Unit::TestCase [downloader_klass.new, tempfile] end end + +class MockAction; end +class MockActionOther; end + diff --git a/test/vagrant/actions/collection_test.rb b/test/vagrant/actions/collection_test.rb new file mode 100644 index 000000000..919804e9e --- /dev/null +++ b/test/vagrant/actions/collection_test.rb @@ -0,0 +1,110 @@ +require File.join(File.dirname(__FILE__), '..', '..', 'test_helper') + +class CollectionTest < Test::Unit::TestCase + context "checking uniqueness" do + setup do + @actions = Vagrant::Actions::Collection.new([1]) + end + + should "return true if there are duplicate classes in the collection" do + @actions << 1 + assert @actions.duplicates? + end + + should "return false it all the classes are unique" do + @actions << 1.0 << "foo" + assert !@actions.duplicates? + end + + should "raise an exception when there are duplicates" do + @actions << 1 + assert_raise Vagrant::Actions::DuplicateActionException do + @actions.duplicates! + end + end + + should "not raise an exception when there are no duplicates" do + @actions << 1.0 << "foo" + assert_nothing_raised do + @actions.duplicates! + end + end + end + + context "verifying dependencies" do + setup do + @mock_action = mock('action') + @mock_action.stubs(:class).returns(MockAction) + + @mock_action2 = mock('action2') + @mock_action2.stubs(:class).returns(MockActionOther) + # see test_helper + stub_default_action_dependecies(@mock_action) + stub_default_action_dependecies(@mock_action2) + end + + context "that come before an action" do + setup do + @mock_action.stubs(:follows).returns([MockActionOther]) + end + should "raise an exception if they are not met" do + assert_raise Vagrant::Actions::DependencyNotSatisfiedException do + collection.new([@mock_action]).dependencies! + end + end + + should "not raise an exception if they are met" do + assert_nothing_raised do + collection.new([@mock_action2, @mock_action]).dependencies! + end + end + end + + context "that follow an an action" do + setup do + @mock_action.stubs(:precedes).returns([MockActionOther]) + end + + should "raise an exception if they are not met" do + assert_raise Vagrant::Actions::DependencyNotSatisfiedException do + collection.new([@mock_action]).dependencies! + end + end + + should "not raise an exception if they are met" do + assert_nothing_raised do + collection.new([@mock_action, @mock_action2]).dependencies! + end + end + end + + context "that are before and after an action" do + setup do + @mock_action.stubs(:precedes).returns([MockActionOther]) + @mock_action.stubs(:follows).returns([MockActionOther]) + end + + should "raise an exception if they are met" do + assert_raise Vagrant::Actions::DependencyNotSatisfiedException do + collection.new([@mock_action2, @mock_action]).dependencies! + end + end + + should "not raise and exception if they are met" do + assert_nothing_raised do + collection.new([@mock_action2, @mock_action, @mock_action2]).dependencies! + end + end + end + end + + context "klasses" do + should "return a list of the collection element's classes" do + @action = mock('action') + assert_equal collection.new([@action]).klasses, [@action.class] + assert_equal collection.new([@action, 1.0, "foo"]).klasses, [@action.class, Float, String] + end + end + + def collection; Vagrant::Actions::Collection end +end diff --git a/test/vagrant/actions/runner_test.rb b/test/vagrant/actions/runner_test.rb index 5d63f970d..14c65dddc 100644 --- a/test/vagrant/actions/runner_test.rb +++ b/test/vagrant/actions/runner_test.rb @@ -6,6 +6,7 @@ class ActionRunnerTest < Test::Unit::TestCase action.stubs(:prepare) action.stubs(:execute!) action.stubs(:cleanup) + stub_default_action_dependecies(action) action end @@ -135,6 +136,7 @@ class ActionRunnerTest < Test::Unit::TestCase should "clear the actions and run a single action if given to execute!" do action = mock("action") run_action = mock("action_run") + stub_default_action_dependecies(run_action) run_class = mock("run_class") run_class.expects(:new).once.returns(run_action) @runner.actions << action @@ -148,7 +150,6 @@ class ActionRunnerTest < Test::Unit::TestCase end should "clear actions after running execute!" do - @runner.actions << mock_fake_action @runner.actions << mock_fake_action assert !@runner.actions.empty? # sanity @runner.execute! @@ -158,9 +159,10 @@ class ActionRunnerTest < Test::Unit::TestCase should "run #prepare on all actions, then #execute!" do action_seq = sequence("action_seq") actions = [] - 5.times do |i| + [MockAction, MockActionOther].each_with_index do |klass, i| action = mock("action#{i}") - + action.expects(:class).returns(klass) + stub_default_action_dependecies(action, klass) @runner.actions << action actions << action end @@ -176,10 +178,12 @@ class ActionRunnerTest < Test::Unit::TestCase context "exceptions" do setup do - @actions = [mock_fake_action, mock_fake_action] - @actions.each do |a| - a.stubs(:rescue) - @runner.actions << a + @actions = [MockAction, MockActionOther].map do |klass| + action = mock_fake_action + action.expects(:class).returns(klass) + action.stubs(:rescue) + @runner.actions << action + action end @exception = Exception.new