core: only delete state if not created on initialize only

/cc @sethvargo

lib/vagrant/machine.rb

@@ -135,6 +135,12 @@ module Vagrant
         @logger.debug("Eager loading WinRM communicator to avoid GH-3390")
         communicate
       end
+
+      # If the ID is the special not created ID, then set our ID to
+      # nil so that we destroy all our data.
+      if state.id == MachineState::NOT_CREATED_ID
+        self.id = nil
+      end
     end
 
     # This calls an action on the provider. The provider may or may not
@@ -472,12 +478,6 @@ module Vagrant
       result = @provider.state
       raise Errors::MachineStateInvalid if !result.is_a?(MachineState)
 
-      # If the ID is the special not created ID, then set our ID to
-      # nil so that we destroy all our data.
-      if result.id == MachineState::NOT_CREATED_ID
-        self.id = nil
-      end
-
       # Update our state cache if we have a UUID and an entry in the
       # master index.
       uuid = index_uuid

test/unit/support/dummy_provider.rb

@@ -22,8 +22,13 @@ module VagrantTests
     end
 
     def state
-      state_id = :running
-      state_id = state_file.read.to_sym if state_file.file?
+      if !state_file.file?
+        new_state = @machine.id
+        new_state = Vagrant::MachineState::NOT_CREATED_ID if !new_state
+        self.state = new_state
+      end
+
+      state_id = state_file.read.to_sym
       Vagrant::MachineState.new(state_id, state_id.to_s, state_id.to_s)
     end
 

test/unit/vagrant/environment_test.rb

@@ -253,7 +253,7 @@ describe Vagrant::Environment do
   describe "#machine" do
     # A helper to register a provider for use in tests.
     def register_provider(name, config_class=nil, options=nil)
-      provider_cls = Class.new(Vagrant.plugin("2", :provider))
+      provider_cls = Class.new(VagrantTests::DummyProvider)
 
       register_plugin("2") do |p|
         p.provider(name, options) { provider_cls }

test/unit/vagrant/machine_test.rb

@@ -7,11 +7,7 @@ describe Vagrant::Machine do
   include_context "unit"
 
   let(:name)     { "foo" }
-  let(:provider) do
-    double("provider").tap do |obj|
-      obj.stub(_initialize: nil)
-    end
-  end
+  let(:provider) { new_provider_mock }
   let(:provider_cls) do
     obj = double("provider_cls")
     obj.stub(new: provider)
@@ -46,6 +42,15 @@ describe Vagrant::Machine do
 
   subject { instance }
 
+  def new_provider_mock
+    double("provider").tap do |obj|
+      obj.stub(_initialize: nil)
+      obj.stub(machine_id_changed: nil)
+      allow(obj).to receive(:state).and_return(Vagrant::MachineState.new(
+        :created, "", ""))
+    end
+  end
+
   # Returns a new instance with the test data
   def new_instance
     described_class.new(name, provider_name, provider_cls, provider_config,
@@ -54,6 +59,17 @@ describe Vagrant::Machine do
   end
 
   describe "initialization" do
+    it "should set the ID to nil if the state is not created" do
+      subject.id = "foo"
+
+      allow(provider).to receive(:state).and_return(Vagrant::MachineState.new(
+        Vagrant::MachineState::NOT_CREATED_ID, "short", "long"))
+
+      subject = new_instance
+      expect(subject.state.id).to eq(Vagrant::MachineState::NOT_CREATED_ID)
+      expect(subject.id).to be_nil
+    end
+
     describe "communicator loading" do
       it "doesn't eager load SSH" do
         config.vm.communicator = :ssh
@@ -86,8 +102,7 @@ describe Vagrant::Machine do
         received_machine = nil
 
         if !instance
-          instance = double("instance")
-          instance.stub(_initialize: nil)
+          instance = new_provider_mock
         end
 
         provider_cls = double("provider_cls")
@@ -166,7 +181,7 @@ describe Vagrant::Machine do
       end
 
       it "should initialize the capabilities" do
-        instance = double("instance")
+        instance = new_provider_mock
         expect(instance).to receive(:_initialize).with { |p, m|
           expect(p).to eq(provider_name)
           expect(m.name).to eq(name)
@@ -410,6 +425,10 @@ describe Vagrant::Machine do
     end
 
     it "is set one when setting an ID" do
+      # Stub the message we want
+      allow(provider).to receive(:state).and_return(Vagrant::MachineState.new(
+        :preparing, "preparing", "preparing"))
+
       # Setup the box information
       box = double("box")
       box.stub(name: "foo")
@@ -657,7 +676,7 @@ describe Vagrant::Machine do
     it "should query state from the provider" do
       state = Vagrant::MachineState.new(:id, "short", "long")
 
-      expect(provider).to receive(:state).and_return(state)
+      allow(provider).to receive(:state).and_return(state)
       expect(instance.state.id).to eq(:id)
     end
 
@@ -681,18 +700,6 @@ describe Vagrant::Machine do
       expect(entry.state).to eq("short")
       env.machine_index.release(entry)
     end
-
-    it "should set the ID to nil if the state is not created" do
-      state = Vagrant::MachineState.new(
-       Vagrant::MachineState::NOT_CREATED_ID, "short", "long")
-
-      allow(provider).to receive(:machine_id_changed)
-      subject.id = "foo"
-
-      expect(provider).to receive(:state).and_return(state)
-      expect(subject.state.id).to eq(Vagrant::MachineState::NOT_CREATED_ID)
-      expect(subject.id).to be_nil
-    end
   end
 
   describe "#with_ui" do

test/unit/vagrant/vagrantfile_test.rb

@@ -26,7 +26,7 @@ describe Vagrant::Vagrantfile do
 
   # A helper to register a provider for use in tests.
   def register_provider(name, config_class=nil, options=nil)
-    provider_cls = Class.new(Vagrant.plugin("2", :provider)) do
+    provider_cls = Class.new(VagrantTests::DummyProvider) do
       if options && options[:unusable]
         def self.usable?(raise_error=false)
           raise Vagrant::Errors::VagrantError if raise_error