The first patch adds the autogenerated sqlite3 files to .gitignore.
The second adds some more requirements to the Task model spec. They make the `rake spec` tests fail.
The third one makes the tests pass again.
And the last patch adds requirements for the Provider model. These do pass right away.
From: Tomas Sedovic tsedovic@redhat.com
After the Aggregator is set up, there are sqlite databases that are automatically generated from migrations. These are not to be edited directly.
They're added in .gitignore now. --- .gitignore | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/.gitignore b/.gitignore index 89945f2..54a235a 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,8 @@ ovirt-server.spec schema.rb log/ database.yml + +# databases automatically generated by Rails migrations +development.sqlite3 +production.sqlite3 +test.sqlite3
On Tue, Mar 23, 2010 at 11:49 AM, tsedovic@redhat.com wrote:
From: Tomas Sedovic tsedovic@redhat.com
After the Aggregator is set up, there are sqlite databases that are automatically generated from migrations. These are not to be edited directly.
They're added in .gitignore now.
ACK
From: Tomas Sedovic tsedovic@redhat.com
These requirements make sure that the records of task creation, start and end time are consistent.
The tests are failing now. --- src/spec/models/task_spec.rb | 34 ++++++++++++++++++++++++++++++++-- 1 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/src/spec/models/task_spec.rb b/src/spec/models/task_spec.rb index ffdb604..d392cb2 100644 --- a/src/spec/models/task_spec.rb +++ b/src/spec/models/task_spec.rb @@ -3,21 +3,28 @@ require 'spec_helper' describe Task do
before(:each) do + @valid_attributes = { :created_at => Time.now, + :time_started => Time.now + 3.minutes, + :time_ended => Time.now + 5.minutes, + :state => Task::STATE_FINISHED } @task = InstanceTask.new( {} ) end
+ it "should be valid with the test data" do + @task.attributes = @valid_attributes + @task.should be_valid + end + it "should begin in a queued state" do @task.state.should eql('queued') end
it "should be invalid with unknown type" do - @task.should be_valid @task.type = 'TotallyInvalidTask' @task.should_not be_valid end
it "should be invalid with unknown state" do - @task.should be_valid @task.state = 'BetYouDidNotExpectThisState' @task.should_not be_valid end @@ -31,4 +38,27 @@ describe Task do @task.type_label.should eql('Instance') end
+ it "should have 'created at' time set if it started" do + @task.attributes = @valid_attributes.except :created_at + @task.should_not be_valid + end + + it "should have 'started' time set if it ended" do + @task.attributes = @valid_attributes.except :time_started + @task.should_not be_valid + end + + it "should not be valid if it started before it was created" do + @task.attributes = @valid_attributes + @task.time_started = @task.created_at - 1.minute + @task.should_not be_valid + end + + it "should not be valid if it ended before it was started" do + @task.attributes = @valid_attributes + @task.time_ended = @task.time_started - 1.minute + @task.should_not be_valid + end + + end
On Tue, Mar 23, 2010 at 11:49 AM, tsedovic@redhat.com wrote:
From: Tomas Sedovic tsedovic@redhat.com
These requirements make sure that the records of task creation, start and end time are consistent.
The tests are failing now.
ACK
From: Tomas Sedovic tsedovic@redhat.com
Task used to validate correctly in situations where it had the "time_ended" attribute set and "time_started" was nil. And similarly for "time_started" vs."created_at". --- src/app/models/task.rb | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/app/models/task.rb b/src/app/models/task.rb index 5e907a0..6cb907e 100644 --- a/src/app/models/task.rb +++ b/src/app/models/task.rb @@ -93,6 +93,8 @@ class Task < ActiveRecord::Base end
def validate + errors.add("created_at", "Task started but does not have the creation time set") if time_started and created_at.nil? + errors.add("time_started", "Task ends but does not have the start time set") if time_ended and time_started.nil? errors.add("time_ended", "Tasks ends before it's started") unless time_ended.nil? or time_started.nil? or time_ended > time_started errors.add("time_started", "Tasks starts before it's created") unless time_started.nil? or created_at.nil? or time_started > created_at end
On Tue, Mar 23, 2010 at 11:49 AM, tsedovic@redhat.com wrote:
From: Tomas Sedovic tsedovic@redhat.com
Task used to validate correctly in situations where it had the "time_ended" attribute set and "time_started" was nil. And similarly for "time_started" vs."created_at".
ACK
From: Tomas Sedovic tsedovic@redhat.com
--- src/spec/models/provider_spec.rb | 57 ++++++++++++++++++++++++++++++++++--- 1 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/src/spec/models/provider_spec.rb b/src/spec/models/provider_spec.rb index c57eb7d..daca748 100644 --- a/src/spec/models/provider_spec.rb +++ b/src/spec/models/provider_spec.rb @@ -2,12 +2,59 @@ require 'spec_helper'
describe Provider do before(:each) do - @valid_attributes = { - } end
- it "should create a new instance given valid attributes" do - # pending - # Provider.create!(@valid_attributes) + it "should validate mock provider" do + provider = Factory(:mock_provider) + provider.should be_valid end + + it "should require a valid name" do + provider = Factory.create(:mock_provider) + [nil, ""].each do |invalid_value| + provider.name = invalid_value + provider.should_not be_valid + end + end + + it "should require a valid cloud_type" do + provider = Factory.create(:mock_provider) + [nil, ""].each do |invalid_value| + provider.cloud_type = invalid_value + provider.should_not be_valid + end + end + + it "should require a valid url" do + provider = Factory.create(:mock_provider) + [nil, ""].each do |invalid_value| + provider.url = invalid_value + provider.should_not be_valid + end + end + + it "should be able to connect to the specified framework" do + provider = Factory.create(:mock_provider) + deltacloud = provider.connect + provider.should be_valid + deltacloud.should_not be_nil + + provider.url = "http://totally.not/there" + deltacloud = provider.connect + provider.should have(1).error_on(:url) + provider.should_not be_valid + deltacloud.should be_nil + end + + + it "should require unique name" do + provider1 = Factory.create(:mock_provider) + provider2 = Factory.create(:mock_provider) + provider1.should be_valid + provider2.should be_valid + + provider2.name = provider1.name + provider2.should_not be_valid + end + end
This does not apply as it's own patch, it should go on top of the previous one in the series. I wanted to suggest using mock, but thought altering an existing set of tests would be easier than trying to explain.
I don't care about this being its own patch, so feel free to just git apply it on top of the previous one, then git commit --amend once everything is as desired.
The big thing is, we stub out connect, as DeltaCloud (client) is a collaborator, we should not require a core instance to be running just to make our tests pass. DeltaCloud should be tested with its own codebase (which it is).
One piece which should be tested that I have not added yet is for the connect method, there should be another mock for the logger. what we really want to test is if the logger receives the message that it should. If this turns out to be too problematic, it can be added later, but I think it would be a useful test. --- src/spec/models/provider_spec.rb | 54 +++++++++++++++++++------------------- 1 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/spec/models/provider_spec.rb b/src/spec/models/provider_spec.rb index daca748..64aec81 100644 --- a/src/spec/models/provider_spec.rb +++ b/src/spec/models/provider_spec.rb @@ -2,54 +2,43 @@ require 'spec_helper'
describe Provider do before(:each) do + @client = mock('DeltaCloud', :null_object => true) + Provider.stub!(:connect).and_return(@client) + @provider = Factory.create(:mock_provider) + end + + it "should return a client object" do + @provider.send(:valid_framework?).should be_true end
it "should validate mock provider" do - provider = Factory(:mock_provider) - provider.should be_valid + @provider.should be_valid end
it "should require a valid name" do - provider = Factory.create(:mock_provider) [nil, ""].each do |invalid_value| - provider.name = invalid_value - provider.should_not be_valid + @provider.name = invalid_value + @provider.should_not be_valid end end
it "should require a valid cloud_type" do - provider = Factory.create(:mock_provider) [nil, ""].each do |invalid_value| - provider.cloud_type = invalid_value - provider.should_not be_valid + @provider.cloud_type = invalid_value + @provider.should_not be_valid end end
it "should require a valid url" do - provider = Factory.create(:mock_provider) [nil, ""].each do |invalid_value| - provider.url = invalid_value - provider.should_not be_valid + @provider.url = invalid_value + @provider.should_not be_valid end end
- it "should be able to connect to the specified framework" do - provider = Factory.create(:mock_provider) - deltacloud = provider.connect - provider.should be_valid - deltacloud.should_not be_nil - - provider.url = "http://totally.not/there" - deltacloud = provider.connect - provider.should have(1).error_on(:url) - provider.should_not be_valid - deltacloud.should be_nil - end - - it "should require unique name" do - provider1 = Factory.create(:mock_provider) - provider2 = Factory.create(:mock_provider) + provider1 = Factory.create :mock_provider + provider2 = Factory.create :mock_provider provider1.should be_valid provider2.should be_valid
@@ -57,4 +46,15 @@ describe Provider do provider2.should_not be_valid end
+ it "should be able to connect to the specified framework" do + @provider.should be_valid + @provider.connect.should_not be_nil + + @provider.url = "http://totally.not/there" + @provider.stub(:connect).and_return(nil) + deltacloud = @provider.connect + @provider.should have(1).error_on(:url) + @provider.should_not be_valid + deltacloud.should be_nil + end end
deltacloud-devel@lists.fedorahosted.org