ACK, makes sense, tests pass, good to merge I don't have commit rights, so please commit yourself.
Just a thought from a guy who is trying to get used to rspec: The 'it' line in the test is pretty long and it contains a condition but there is just one assertion for the condition so IMHO is Ok. On Mon, Oct 29, 2012 at 05:03:56PM +0100, [email protected] wrote: > From: Jan Provaznik <[email protected]> > > For rhevm/vsphere an instance goes to 'stopped' state after creation so > conductor has to send explicit start request. To distinguish "stopped after > creation" state and common "stopped" state conductor checks if last > instance launch request was sent after last deployment launch request > (there can be multiple launch requests for deployment/instance because of > rolblack+retry process). > > In some cases it was possible that instance launch request was picked up by > delayed_job in same second when the deployment was created, so checking create > times by seconds was not precise enough. > --- > src/app/models/instance.rb | 2 +- > src/spec/models/instance_spec.rb | 18 ++++++++++++++++++ > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb > index 8e02002..2d27394 100644 > --- a/src/app/models/instance.rb > +++ b/src/app/models/instance.rb > @@ -525,7 +525,7 @@ class Instance < ActiveRecord::Base > # also make sure that the 'create' task was created after > # last deployment launch request - instance can be stopped > # since previous rollback+retry request > - last_task.created_at.to_i > last_launch_time.to_i && > + last_task.created_at.to_f > last_launch_time.to_f && > provider_account && > provider_account.provider.provider_type.goes_to_stop_after_creation? > end > diff --git a/src/spec/models/instance_spec.rb > b/src/spec/models/instance_spec.rb > index 3f9a6f5..26fea33 100644 > --- a/src/spec/models/instance_spec.rb > +++ b/src/spec/models/instance_spec.rb > @@ -379,4 +379,22 @@ describe Instance do > errors.should_not be_empty > errors.select {|e| e.include?("no Config Server available") }.should_not > be_empty > end > + > + describe ".stopped_after_creation?" do > + before(:each) do > + @instance = FactoryGirl.create(:instance, :pool_id => @pool.id, :state > => 'stopped') > + @deployment = FactoryGirl.create :deployment > + @deployment.instances << @instance > + end > + > + it "should be true if the deployment is pending and the provider doesn't > start an instance automatically" do > + @instance.provider_account.provider.provider_type. > + stub!(:goes_to_stop_after_creation?).and_return(true) > + @deployment.update_attribute(:state, Deployment::STATE_PENDING) > + @instance.tasks << InstanceTask.create!({:user => nil, > + :task_target => @instance, > + :action => > InstanceTask::ACTION_CREATE}) > + @instance.stopped_after_creation?.should be_true > + end > + end > end > -- > 1.7.11.4 > -- Martin Povolny <[email protected]> tel. +420777714458
