[PATCH conductor] Strip whitespace from image id when importing images.
Matt Wagner
matt.wagner at redhat.com
Thu Aug 9 14:51:38 UTC 2012
On Thu, Aug 09, 2012 at 01:29:23PM +0200, jstransk at redhat.com wrote:
> From: Jiri Stransky <jstransk at redhat.com>
>
> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=819059
> ---
> src/app/controllers/images_controller.rb | 2 +-
> src/spec/controllers/images_controller_spec.rb | 49 ++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 1 deletions(-)
> create mode 100644 src/spec/controllers/images_controller_spec.rb
>
> diff --git a/src/app/controllers/images_controller.rb b/src/app/controllers/images_controller.rb
> index c3ca98c..00ae854 100644
> --- a/src/app/controllers/images_controller.rb
> +++ b/src/app/controllers/images_controller.rb
> @@ -194,7 +194,7 @@ class ImagesController < ApplicationController
>
> xml = "<image><name>#{params[:name]}</name></image>" unless params[:name].blank?
> begin
> - image = Image.import(account, params[:image_id], @environment, xml)
> + image = Image.import(account, params[:image_id].strip, @environment, xml)
> flash[:success] = t("images.import.image_imported")
> redirect_to image_url(image.id) and return
> rescue Exception => e
> diff --git a/src/spec/controllers/images_controller_spec.rb b/src/spec/controllers/images_controller_spec.rb
> new file mode 100644
> index 0000000..a684a0d
> --- /dev/null
> +++ b/src/spec/controllers/images_controller_spec.rb
> @@ -0,0 +1,49 @@
> +#
> +# Copyright 2012 Red Hat, Inc.
> +#
> +# Licensed under the Apache License, Version 2.0 (the "License");
> +# you may not use this file except in compliance with the License.
> +# You may obtain a copy of the License at
> +#
> +# http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +
> +require 'spec_helper'
> +
> +describe ImagesController do
> +
> + fixtures :all
> + before(:each) do
> + @admin_permission = FactoryGirl.create :admin_permission
> + @admin = @admin_permission.user
> + @provider_account = FactoryGirl.create :mock_provider_account
> + @pool_family = FactoryGirl.create :pool_family
> + end
> +
> + describe "#import" do
> +
> + it "strips whitespace off the image id provided by the user" do
> + ProviderAccount.stub(:find).and_return(@provider_account)
> + PoolFamily.stub(:find).and_return(@pool_family)
> + @image = mock('image')
> + @image.stub(:id).and_return('456def')
> +
> + Image.should_receive(:import).
> + with { |provider_account, image_id, pool_family, xml| image_id == 'Mock_mock_123abc' }.
> + and_return(@image)
> +
> + mock_warden(@admin)
> + post(:import, :image_id => ' Mock_mock_123abc ',
> + :name => 'imported',
> + :provider_account => '1',
> + :environment => 'some_environment')
> + end
> +
> + end
> +end
ACK, looks good! I have just pushed this to master.
It's been suggested that in controller tests, we should be using
mock_model[1] since we don't care about exercising actual model
functionality. However, none of our tests are currently doing this, so
this isn't a criticism of your patch -- it's good as-is! -- as much as a
note about something we as a team should consider switching to. :)
[1]
https://www.relishapp.com/rspec/rspec-rails/v/2-4/docs/mocks/mock-model
More information about the aeolus-devel
mailing list