This has the same issue as the previous one, lots of whitespace errors, I wont paste the list this time. Overall, this looks good - tests pass, and you have covered a goodly number of paths through the app. Couple comments inline, though they can be in a later patch (since there is a bunch of rework for naming already).
On Wed, Apr 7, 2010 at 2:29 PM, mtaylor <[email protected]> wrote: > --- > src/features/portal_pool.feature | 79 > ++++++++++++++++++++ > src/features/step_definitions/portal_pool_steps.rb | 42 ++++++++++ > src/features/support/env.rb | 8 ++ > src/features/support/paths.rb | 15 +++- > src/spec/factories/hardware_profile.rb | 7 ++ > src/spec/factories/provider.rb | 4 +- > src/spec/factories/realm.rb | 1 + > 7 files changed, 153 insertions(+), 3 deletions(-) > create mode 100644 src/features/portal_pool.feature > create mode 100644 src/features/step_definitions/portal_pool_steps.rb > > diff --git a/src/features/portal_pool.feature > b/src/features/portal_pool.feature > new file mode 100644 > index 0000000..2011fab > --- /dev/null > +++ b/src/features/portal_pool.feature > @@ -0,0 +1,79 @@ > +Feature: Manage Portal Pools > + In order to manage my cloud infrastructure > + As a user > + I want to manage my portal pools > + > + Scenario: Create a new Pool > + Given I am on the homepage > + And I am an authorised user > + And I am logged in > I notice these previous two lines appear in every scenario, so it may clean things up a bit to move them into a share Background. + And there is not a portal pool named "mockpool" > + When I follow "Add a pool" > + Then I should be on the new portal pool page > + And I should see "Create a new Pool" > + When I fill in "portal_pool_name" with "mockpool" > + And I press "Save" > + Then I should be on the show portal pool page > + And I should see "Pool added" > + And I should see "mockpool" > + And I should have a portal pool named "mockpool" > + > + @focus > + Scenario: View Portal Pool's Hardware Profiles > + Given I am an authorised user > + And I own a portal pool named "mockpool" > + And the Portal Pool has the following Hardware Profiles: > + | name | memory | storage | architecture | > + | m1-small | 1.7 | 160.0 | i386 | > + | m1-large | 7.5 | 850.0 | x86_64 | > + | m1-xlarge | 15.0 | 1690.0 | x86_64 | > + And I am logged in > + And I am on the homepage > + When I follow "mockpool" > + Then I should be on the show portal pool page > + When I follow "Hardware Profiles" > + Then I should see "m1-small" > + And I should see "m1-large" > + And I should see "m1-xlarge" > + And I should see "1.7" > + And I should see "7.5" > + And I should see "15.0" > + And I should see "160.0" > + And I should see "850.0" > + And I should see "1690.0" > + And I should see "i386" > + And I should see "x86_64" > + And I should see "x86_64" > + > The above is a good candidate to go into a Scenario Outline, I think. Not sure if this would need pickle brought in to read the table easier, but the idea is that all these 'And I should see' lines can go into a table (like you do for listing the Hardware Profiles above. This will make the test less repetitious and easier to read. + @focus > + Scenario: View Portal Pool's Realms > + Given I am an authorised user > + And I own a portal pool named "mockpool" > + And the Portal Pool has the following Realms named "Europe, > United States" > + And I am logged in > + And I am on the account page > + When I follow "mockpool" > + Then I should be on the show portal pool page > + When I follow "Realms" > + Then I should see "Europe" > + And I should see "United States" > + > + Scenario: View Portal Pool's Images > + Given I am an authorised user > + And I am logged in > + And I own a portal pool named "mockpool" > + And the Portal Pool has the following Images: > + | name | architecture | > + | Fedora 10 | x86_64 | > + | Fedora 10 | i386 | > + | JBoss | i386 | > + And I am on the account page > + When I follow "mockpool" > + Then I should be on the show portal pool page > + When I follow "View Images" > + Then I should see "Fedora 10" > + And I should see "x86_64" > + And I should see "Fedora 10" > + And I should see "i386" > + And I should see "JBoss" > + And I should see "i386" > Same as above, this would be cleaner/easier to read in a table. \ No newline at end of file > diff --git a/src/features/step_definitions/portal_pool_steps.rb > b/src/features/step_definitions/portal_pool_steps.rb > new file mode 100644 > index 0000000..4e20a7a > --- /dev/null > +++ b/src/features/step_definitions/portal_pool_steps.rb > @@ -0,0 +1,42 @@ > +Given /^I am an authorised user$/ do > + @admin_permission = Factory :admin_permission > + @user = @admin_permission.user > +end > + > +Given /^I own a portal pool named "([^\"]*)"$/ do |name| > + @portal_pool = Factory(:portal_pool, :name => name) > + @user.owned_pools << @portal_pool > +end > + > +Given /^the Portal Pool has the following Hardware Profiles:$/ do |table| > + table.hashes.each do |hash| > + @portal_pool.hardware_profiles << Factory(:hardware_profile_auto, > hash) > + end > +end > + > +Given /^the Portal Pool has the following Realms named "([^\"]*)"$/ do > |names| > + @cloud_account = Factory :cloud_account > + @provider = @cloud_account.provider > + > + names.split(", ").each do |name| > + @portal_pool.realms << Factory(:realm, :name => name, :provider => > @provider) > + end > + @portal_pool.cloud_accounts << @cloud_account > + > +end > + > +Given /^the Portal Pool has the following Images:$/ do |table| > + table.hashes.each do |hash| > + hash["portal_pool"] = @portal_pool > + @portal_pool.images << Factory(:image, hash) > + end > +end > + > +Given /^there is not a portal pool named "([^\"]*)"$/ do |name| > + PortalPool.find_by_name(name).should be_nil > +end > + > +Then /^I should have a portal pool named "([^\"]*)"$/ do |name| > + PortalPool.find_by_name(name).should_not be_nil > + PortalPool.find_by_name(name).permissions.size.should == 1 > +end > \ No newline at end of file > diff --git a/src/features/support/env.rb b/src/features/support/env.rb > index 341854f..2d6e9a1 100644 > --- a/src/features/support/env.rb > +++ b/src/features/support/env.rb > @@ -21,6 +21,14 @@ Webrat.configure do |config| > config.open_error_files = false # Set to true if you want error pages to > pop up in the browser > end > > +Provider.class_eval do > + alias_method :original_validate, :validate > + def validate > + if !nil_or_empty(url) > + # errors.add("url", "must be a valid provider url") unless > valid_framework? > + end > + end > +end > > This is an interesting take on how to unhook the features from needing core running. Working with Tomas, we went the route of stubbing out the connect method. Now I am thinking perhaps we should just stub out DeltaCloud.new to return a mock object - I think that will cover all classes that need it. However, this is fine for now, just give it some thought, we should get all these tests using the same type of structure (features and specs) > # If you set this to false, any error raised from within your app will > bubble > # up to your step definition and out to cucumber unless you catch it > somewhere > diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb > index 7c16ade..5fc4127 100644 > --- a/src/features/support/paths.rb > Above comments aside, this is a great first step. Please resend series to list once you have rebased and made the few changes I requested - just to have someone else run the tests and make sure everything still seems ok. so, 'quasi-ACK'? -j
_______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
