[PATCH aggregator] Fixing broken users controller spec

This patch is bringing the unit tests build back to green state.
I refactored users_controller_spec.rb to be used without factories.
This patch relies on DatabaseClener patch posted also today.

From: Ladislav Martincik

- This patch expects DatabaseCleaner to be part of RSpec config
---
 src/app/controllers/users_controller.rb       |    2 +-
 src/app/models/user.rb                        |    4 +-
 src/spec/controllers/users_controller_spec.rb |  101 ++++++++++--------------
 src/spec/factories/base_permission_object.rb  |    3 +
 src/spec/factories/metadata_object.rb         |    5 +
 src/spec/factories/permission.rb              |   10 +-
 6 files changed, 58 insertions(+), 67 deletions(-)
 create mode 100644 src/spec/factories/base_permission_object.rb

diff --git a/src/app/controllers/users_controller.rb b/src/app/controllers/users_controller.rb
index 9a6c1db..9896c98 100644
--- a/src/app/controllers/users_controller.rb
+++ b/src/app/controllers/users_controller.rb
@@ -77,7 +77,7 @@ class UsersController < ApplicationController
     @user = params[:user][:id] ? User.find(params[:user][:id]) : @current_user
     if params[:commit] == "Save"
       if @user
-        has_users_modify= BasePermissionObject.general_permission_scope.can_modify_users(@current_user)
+        has_users_modify = BasePermissionObject.general_permission_scope.can_modify_users(@current_user)
         if @user != @current_user
           if !has_users_modify
             flash[:notice] = "Invalid Permission to perform this operation"
diff --git a/src/app/models/user.rb b/src/app/models/user.rb
index aed6fe6..0332095 100644
--- a/src/app/models/user.rb
+++ b/src/app/models/user.rb
@@ -28,8 +28,8 @@ class User < ActiveRecord::Base
   belongs_to :quota, :autosave => true
   accepts_nested_attributes_for :quota

-  validates_length_of   :first_name, :maximum => 255
-  validates_length_of   :last_name,  :maximum => 255
+  validates_length_of :first_name, :maximum => 255, :allow_blank => true
+  validates_length_of :last_name,  :maximum => 255, :allow_blank => true

# authlogic's password confirmation doesn't fire up when we fill in the
diff --git a/src/spec/controllers/users_controller_spec.rb b/src/spec/controllers/users_controller_spec.rb
index 92435f4..eedb0b3 100644
--- a/src/spec/controllers/users_controller_spec.rb
+++ b/src/spec/controllers/users_controller_spec.rb
@@ -1,112 +1,95 @@
 require 'spec_helper'

describe UsersController do
-  fixtures :all
+
   before(:each) do
+    Factory(:base_permission_object)
     @tuser = Factory :tuser
     @admin_permission = Factory :admin_permission
+    @admin_permission.role.privileges << Privilege.new(:name => 'user_modify')
     @admin = @admin_permission.user
+    Factory.create(:default_quota_metadata)
+    Factory.create(:default_role_metadata)
+    Factory.create(:default_pool_metadata)
     activate_authlogic
-
-    @allow_self_service_logins = Factory(:metadata_object, :key => "allow_self_service_logins", :value => "true")
-
-    @default_quota = Factory(:unlimited_quota)
-    @self_service_default_quota = Factory(:metadata_object, :key => "self_service_default_quota",
-                                                            :value => @default_quota,
-                                                            :object_type => "Quota")
-
-    @default_pool = Factory(:pool, :name => "default_pool")
-    @self_service_default_quota = Factory(:metadata_object, :key => "self_service_default_pool",
-                                                          :value => @default_pool,
-                                                          :object_type => "Pool")
-
-    @default_role = Role.find(:first, :conditions => ['name = ?', 'Instance Creator and User'])
-    @self_service_default_quota = Factory(:metadata_object, :key => "self_service_default_role",
-                                                          :value => @default_role,
-                                                          :object_type => "Role")
   end

-  it "should call new method" do
-    route_for(:controller => 'user_sessions', :action => 'new').should == 'login'
+  it "allows user to get to registration form for new user" do
     get :new
-    @current_user.should == nil
-    UserSession.find.should == nil
     response.should be_success
   end

describe "#create" do
-    before(:each) do
-
-    end
-
     context "user enters valid input" do
-      it "should create user" do
-        lambda {
-          post :create, :user => { :login => "tuser2", :email => "[e-mail]",
-                                   :password => "testpass",
-                                   :password_confirmation => "testpass" }
-        }.should change{ User.count }
-        user = User.find(:first, :conditions => ['login = ?', "tuser2"])
+      it "creates user" do
+        lambda do
+          post :create, :user => {
+            :login => "tuser2", :email => "[e-mail]",
+            :password => "testpass",
+            :password_confirmation => "testpass" }
+        end.should change(User, :count).by(1)
+
         response.should redirect_to(dashboard_url)
       end

it "fails to create pool" do
-        lambda {
+        lambda do
           post :create, :user => {}
-        }.should_not change{ User.count }
-        p = Pool.find_by_name("tuser2")
-        p.should be_nil
+        end.should_not change(User, :count)
+
         returned_user = assigns[:user]
         returned_user.errors.empty?.should be_false
         returned_user.should have(2).errors_on(:login)
         returned_user.should have(2).errors_on(:email)
         returned_user.should have(1).error_on(:password)
         returned_user.should have(1).error_on(:password_confirmation)
-        #assigns[:user].errors.find_all {|attr,msg|
-        #  ["login", "email", "password",  "password_confirmation"].
-        #  include?(attr).should be_true
-        #}
-        response.should  render_template('new')
+
+        response.should render_template('new')
       end
     end
   end

-  it "should allow an admin to create user" do
+  it "allows an admin to create user" do
     UserSession.create(@admin)
-    lambda {
-      post :create, :user => { :login => "tuser3", :email => "[e-mail]",
-                               :password => "testpass",
-                               :password_confirmation => "testpass" }
-    }.should change{ User.count }
-    user = User.find(:first, :conditions => ['login = ?', "tuser3"])
+    lambda do
+      post :create, :user => {
+        :login => "tuser3", :email => "[e-mail]",
+        :password => "testpass",
+        :password_confirmation => "testpass" }
+    end.should change(User, :count)
+
     response.should redirect_to(users_url)
   end

it "should not allow a regular user to create user" do
     UserSession.create(@tuser)
-    lambda {
-      post :create, :user => { :login => "tuser4", :email => "[e-mail]",
-                               :password => "testpass",
-                               :password_confirmation => "testpass" }
-    }.should_not change{ User.count }
+    lambda do
+      post :create, :user => {
+        :login => "tuser4", :email => "[e-mail]",
+        :password => "testpass",
+        :password_confirmation => "testpass" }
+    end.should_not change(User, :count)
   end

-  it "should show user" do
+  it "provides show view for user" do
     UserSession.create(@tuser)
     get :show
+
     response.should be_success
   end

-  it "should get edit" do
+  it "provides edit view for user" do
     UserSession.create(@tuser)
     get :edit, :id => @tuser.id
+
     response.should be_success
   end

-  test "should update user" do
+  it "updates user with new data" do
     UserSession.create(@tuser)
-    put :update, :id => @tuser.id, :user => { }, :save =>'true'
-    response.should redirect_to(users_path)
+    put :update, :id => @tuser.id, :user => {}, :commit => 'Save'
+
+    response.should redirect_to(dashboard_path)
   end

# checks whether proper error template is rendered when an exception raises
diff --git a/src/spec/factories/base_permission_object.rb b/src/spec/factories/base_permission_object.rb
new file mode 100644
index 0000000..ccc2bf1
--- /dev/null
+++ b/src/spec/factories/base_permission_object.rb
@@ -0,0 +1,3 @@
+Factory.define :base_permission_object do |o|
+  o.name BasePermissionObject::GENERAL_PERMISSION_SCOPE
+end
diff --git a/src/spec/factories/metadata_object.rb b/src/spec/factories/metadata_object.rb
index 5735896..9ac8021 100644
--- a/src/spec/factories/metadata_object.rb
+++ b/src/spec/factories/metadata_object.rb
@@ -4,6 +4,11 @@ Factory.define :metadata_object do |o|
   o.object_type nil
 end

+Factory.define :default_logins_metadata, :parent => :metadata_object do |o|
+   o.key 'allow_self_service_logins'
+   o.value 'true'
+end
+
 Factory.define :default_zone_metadata, :parent => :metadata_object do |o|
    o.key 'default_zone'
    o.value {Factory.create(:zone).id}
diff --git a/src/spec/factories/permission.rb b/src/spec/factories/permission.rb
index 8b9fd69..9ccd3e7 100644
--- a/src/spec/factories/permission.rb
+++ b/src/spec/factories/permission.rb
@@ -3,25 +3,25 @@ Factory.define :permission do |p|
 end

Factory.define :admin_permission, :parent => :permission do |p|
-  p.role { |r| Role.find(:first, :conditions => ['name = ?', 'Administrator']) }
+  p.role { |r| Role.first(:conditions => ['name = ?', 'Administrator']) || Factory(:role, :name => 'Administrator') }
   p.permission_object { |r| BasePermissionObject.general_permission_scope }
   p.user { |u| u.association(:admin_user) }
 end

Factory.define :provider_admin_permission, :parent => :permission do |p|
-  p.role { |r| Role.find(:first, :conditions => ['name = ?', 'Provider Administrator']) }
+  p.role { |r| Role.first(:conditions => ['name = ?', 'Provider Administrator']) || Factory(:role, :name => 'Provider Administrator') }
   p.permission_object { |r| r.association(:mock_provider) }
   p.user { |u| u.association(:provider_admin_user) }
 end

Factory.define :pool_creator_permission, :parent => :permission do |p|
-  p.role { |r| Role.find(:first, :conditions => ['name = ?', 'Pool Creator']) }
+  p.role { |r| Role.first(:conditions => ['name = ?', 'Pool Creator']) || Factory(:role, :name => 'Pool Creator') }
   p.permission_object { |r| r.association(:mock_provider) }
   p.user { |u| u.association(:pool_creator) }
 end

Factory.define :instance_creator_and_user_permission, :parent => :permission do |p|
-  p.role { |r| Role.find(:first, :conditions => ['name = ?', 'Instance Creator and User']) }
+  p.role { |r| Role.first(:conditions => ['name = ?', 'Instance Creator and User']) || Factory(:role, :name => 'Instance Creator and User') }
   p.permission_object { |r| r.association(:pool) }
   p.user { |u| u.association(:user) }
-end
\ No newline at end of file
+end

[Jan Provazník]

On 11/11/2010 04:45 PM, [e-mail] wrote:
> From: Ladislav Martincik
> This patch expects DatabaseCleaner to be part of RSpec config
>
> src/app/controllers/users_controller.rb       |    2 +-
>   src/app/models/user.rb                        |    4 +-
>   src/spec/controllers/users_controller_spec.rb |  101 ++++++++++--------------
>   src/spec/factories/base_permission_object.rb  |    3 +
>   src/spec/factories/metadata_object.rb         |    5 +
>   src/spec/factories/permission.rb              |   10 +-
>   6 files changed, 58 insertions(+), 67 deletions(-)
>   create mode 100644 src/spec/factories/base_permission_object.rb
>
> diff --git a/src/app/controllers/users_controller.rb b/src/app/controllers/users_controller.rb
> index 9a6c1db..9896c98 100644
> --- a/src/app/controllers/users_controller.rb
> +++ b/src/app/controllers/users_controller.rb
> @@ -77,7 +77,7 @@ class UsersController<  ApplicationController
>       @user = params[:user][:id] ? User.find(params[:user][:id]) : @current_user
>       if params[:commit] == "Save"
>         if @user
>    has_users_modify= BasePermissionObject.general_permission_scope.can_modify_users(@current_user)
>    has_users_modify = BasePermissionObject.general_permission_scope.can_modify_users(@current_user)
>     if @user != @current_user
>       if !has_users_modify
>         flash[:notice] = "Invalid Permission to perform this operation"
>
> diff --git a/src/app/models/user.rb b/src/app/models/user.rb
> index aed6fe6..0332095 100644
> --- a/src/app/models/user.rb
> +++ b/src/app/models/user.rb
> @@ -28,8 +28,8 @@ class User<  ActiveRecord::Base
>     belongs_to :quota, :autosave =>  true
>     accepts_nested_attributes_for :quota
> validates_length_of   :first_name, :maximum =>  255
> validates_length_of   :last_name,  :maximum =>  255
>
> validates_length_of :first_name, :maximum =>  255, :allow_blank =>  true
>
> validates_length_of :last_name,  :maximum =>  255, :allow_blank =>  true
>
> # authlogic's password confirmation doesn't fire up when we fill in the
>
> diff --git a/src/spec/controllers/users_controller_spec.rb b/src/spec/controllers/users_controller_spec.rb
> index 92435f4..eedb0b3 100644
> --- a/src/spec/controllers/users_controller_spec.rb
> +++ b/src/spec/controllers/users_controller_spec.rb
> @@ -1,112 +1,95 @@
>   require 'spec_helper'
>
> describe UsersController do
> fixtures :all
> before(:each) do
> Factory(:base_permission_object)
>  @tuser = Factory :tuser
>  @admin_permission = Factory :admin_permission
> @admin_permission.role.privileges<<  Privilege.new(:name =>  'user_modify')
>  @admin = @admin_permission.user
> Factory.create(:default_quota_metadata)
> Factory.create(:default_role_metadata)
> Factory.create(:default_pool_metadata)
>  activate_authlogic
>
> @allow_self_service_logins = Factory(:metadata_object, :key =>  "allow_self_service_logins", :value =>  "true")
>
> @default_quota = Factory(:unlimited_quota)
>
> @self_service_default_quota = Factory(:metadata_object, :key =>  "self_service_default_quota",
>                                                        :value =>  @default_quota,
>                                                        :object_type =>  "Quota")
>
> @default_pool = Factory(:pool, :name =>  "default_pool")
>
> @self_service_default_quota = Factory(:metadata_object, :key =>  "self_service_default_pool",
>                                                      :value =>  @default_pool,
>                                                      :object_type =>  "Pool")
>
> @default_role = Role.find(:first, :conditions =>  ['name = ?', 'Instance Creator and User'])
>
> @self_service_default_quota = Factory(:metadata_object, :key =>  "self_service_default_role",
>                                                      :value =>  @default_role,
>                                                      :object_type =>  "Role")
>
> end
>
> it "should call new method" do
>
> route_for(:controller =>  'user_sessions', :action =>  'new').should == 'login'
> it "allows user to get to registration form for new user" do
>    get :new
>
> @current_user.should == nil
>
> UserSession.find.should == nil
>  response.should be_success
> end
>
> describe "#create" do
>
> before(:each) do
>
> end
> context "user enters valid input" do
>  it "should create user" do
>    lambda {
>      post :create, :user =>  { :login =>  "tuser2", :email =>  "[e-mail]",
>                               :password =>  "testpass",
>                               :password_confirmation =>  "testpass" }
>    }.should change{ User.count }
>    user = User.find(:first, :conditions =>  ['login = ?', "tuser2"])
>  it "creates user" do
>    lambda do
>      post :create, :user =>  {
>        :login =>  "tuser2", :email =>  "[e-mail]",
>        :password =>  "testpass",
>        :password_confirmation =>  "testpass" }
>    end.should change(User, :count).by(1)
>     response.should redirect_to(dashboard_url)
>   end
>
>   it "fails to create pool" do
>    lambda {
>    lambda do
>       post :create, :user =>  {}
>    }.should_not change{ User.count }
>    p = Pool.find_by_name("tuser2")
>    p.should be_nil
>    end.should_not change(User, :count)
>     returned_user = assigns[:user]
>     returned_user.errors.empty?.should be_false
>     returned_user.should have(2).errors_on(:login)
>     returned_user.should have(2).errors_on(:email)
>     returned_user.should have(1).error_on(:password)
>     returned_user.should have(1).error_on(:password_confirmation)
>    #assigns[:user].errors.find_all {|attr,msg|
>    #  ["login", "email", "password",  "password_confirmation"].
>    #  include?(attr).should be_true
>    #}
>    response.should  render_template('new')
>    response.should render_template('new')
>   end
> end
> end
> it "should allow an admin to create user" do
> it "allows an admin to create user" do
>    UserSession.create(@admin)
> lambda {
>  post :create, :user =>  { :login =>  "tuser3", :email =>  "[e-mail]",
>                           :password =>  "testpass",
>                           :password_confirmation =>  "testpass" }
> }.should change{ User.count }
> user = User.find(:first, :conditions =>  ['login = ?', "tuser3"])
>
> lambda do
>  post :create, :user =>  {
>    :login =>  "tuser3", :email =>  "[e-mail]",
>    :password =>  "testpass",
>    :password_confirmation =>  "testpass" }
>
> end.should change(User, :count)
> response.should redirect_to(users_url)
>
> end
>
> it "should not allow a regular user to create user" do
>     UserSession.create(@tuser)
> lambda {
>  post :create, :user =>  { :login =>  "tuser4", :email =>  "[e-mail]",
>                           :password =>  "testpass",
>                           :password_confirmation =>  "testpass" }
> }.should_not change{ User.count }
> lambda do
>  post :create, :user =>  {
>    :login =>  "tuser4", :email =>  "[e-mail]",
>    :password =>  "testpass",
>    :password_confirmation =>  "testpass" }
> end.should_not change(User, :count)
> end
> it "should show user" do
> it "provides show view for user" do
>    UserSession.create(@tuser)
>    get :show
> response.should be_success
> end
> it "should get edit" do
> it "provides edit view for user" do
>    UserSession.create(@tuser)
>    get :edit, :id =>  @tuser.id
> response.should be_success
> end
> test "should update user" do
> it "updates user with new data" do
>    UserSession.create(@tuser)
> put :update, :id =>  @tuser.id, :user =>  { }, :save =>'true'
> response.should redirect_to(users_path)
>
> put :update, :id =>  @tuser.id, :user =>  {}, :commit =>  'Save'
>
> response.should redirect_to(dashboard_path)
> end
>
> # checks whether proper error template is rendered when an exception raises
>
> diff --git a/src/spec/factories/base_permission_object.rb b/src/spec/factories/base_permission_object.rb
> new file mode 100644
> index 0000000..ccc2bf1
> --- /dev/null
> +++ b/src/spec/factories/base_permission_object.rb
> @@ -0,0 +1,3 @@
> +Factory.define :base_permission_object do |o|
> o.name BasePermissionObject::GENERAL_PERMISSION_SCOPE
>
> +end
> diff --git a/src/spec/factories/metadata_object.rb b/src/spec/factories/metadata_object.rb
> index 5735896..9ac8021 100644
> --- a/src/spec/factories/metadata_object.rb
> +++ b/src/spec/factories/metadata_object.rb
> @@ -4,6 +4,11 @@ Factory.define :metadata_object do |o|
>     o.object_type nil
>   end
>
> +Factory.define :default_logins_metadata, :parent =>  :metadata_object do |o|
> o.key 'allow_self_service_logins'
> o.value 'true'
>
> +end
> Factory.define :default_zone_metadata, :parent =>  :metadata_object do |o|
>    o.key 'default_zone'
>    o.value {Factory.create(:zone).id}
>
> diff --git a/src/spec/factories/permission.rb b/src/spec/factories/permission.rb
> index 8b9fd69..9ccd3e7 100644
> --- a/src/spec/factories/permission.rb
> +++ b/src/spec/factories/permission.rb
> @@ -3,25 +3,25 @@ Factory.define :permission do |p|
>   end
>
> Factory.define :admin_permission, :parent =>  :permission do |p|
> p.role { |r| Role.find(:first, :conditions =>  ['name = ?', 'Administrator']) }
>
> p.role { |r| Role.first(:conditions =>  ['name = ?', 'Administrator']) || Factory(:role, :name =>  'Administrator') }
>  p.permission_object { |r| BasePermissionObject.general_permission_scope }
>  p.user { |u| u.association(:admin_user) }
> end
>
> Factory.define :provider_admin_permission, :parent =>  :permission do |p|
> p.role { |r| Role.find(:first, :conditions =>  ['name = ?', 'Provider Administrator']) }
>
> p.role { |r| Role.first(:conditions =>  ['name = ?', 'Provider Administrator']) || Factory(:role, :name =>  'Provider Administrator') }
>  p.permission_object { |r| r.association(:mock_provider) }
>  p.user { |u| u.association(:provider_admin_user) }
> end
>
> Factory.define :pool_creator_permission, :parent =>  :permission do |p|
> p.role { |r| Role.find(:first, :conditions =>  ['name = ?', 'Pool Creator']) }
>
> p.role { |r| Role.first(:conditions =>  ['name = ?', 'Pool Creator']) || Factory(:role, :name =>  'Pool Creator') }
>  p.permission_object { |r| r.association(:mock_provider) }
>  p.user { |u| u.association(:pool_creator) }
> end
>
> Factory.define :instance_creator_and_user_permission, :parent =>  :permission do |p|
> p.role { |r| Role.find(:first, :conditions =>  ['name = ?', 'Instance Creator and User']) }
> p.role { |r| Role.first(:conditions =>  ['name = ?', 'Instance Creator and User']) || Factory(:role, :name =>  'Instance Creator and User') }
>  p.permission_object { |r| r.association(:pool) }
>  p.user { |u| u.association(:user) }
>
> -end
> \ No newline at end of file
> +end

ACK

[Ladislav Martincik]

On Thu, Nov 11, 2010 at 4:45 PM,  wrote:
> This patch is bringing the unit tests build back to green state.
> I refactored users_controller_spec.rb to be used without factories.

Of course I mean without fixtures. ;-)
> This patch relies on DatabaseClener patch posted also today.
> _______________________________________________
> deltacloud-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/deltacloud-devel