[PATCH aeolus-image-rubygem 2/2] Tests for Aeolus::Image::Katello code

Martyn Taylor mtaylor at redhat.com
Thu May 31 14:46:09 UTC 2012


Matt,

It's difficult to review without any tests.

I understand that you decided to implement a full client since it was 
relatively straight forward and adding tests for the whole thing would 
likely take some time and stray away from your original goal.  So can 
you at least add spec tests for the parts of the API that you are trying 
to consume.  This will make it easy for reviewer to figure out what is 
going on / how to use the client and also make sure that ActiveResource 
and the API are compliant in each area.

Some other notes:

Be careful when lifting code from other projects.  The base.rb file in 
aeolus-image is bespoke to imagefactory and so turns off certificate 
verfication since factory did not support this at the time of writing 
the client.  I don't think this is necessary in Katello since they 
support certificates.  It's probably worth while looking over the rest 
of this class and remove anything that doesn't make sense in respect to 
conductor.

Lastly, as we discussed in IRC we should probably pull this out into a 
separate gem and contribute it back to Katello, its fine to live here 
for now, but I'dhope we can pull this out later.

I''l have to nack this for now until we have some proper tests.

Thanks mate

Martyn

On 05/24/2012 07:51 PM, Matt Wagner wrote:
> ---
>   spec/models/katello/base_spec.rb |   54 ++++++++++++++++++++++++++++++++++++++
>   spec/spec_helper.rb              |    2 +
>   2 files changed, 56 insertions(+), 0 deletions(-)
>   create mode 100644 spec/models/katello/base_spec.rb
>
> diff --git a/spec/models/katello/base_spec.rb b/spec/models/katello/base_spec.rb
> new file mode 100644
> index 0000000..1cf6da2
> --- /dev/null
> +++ b/spec/models/katello/base_spec.rb
> @@ -0,0 +1,54 @@
> +#
> +#   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'
> +
> +module Aeolus
> +  module Image
> +    module Katello
> +      describe Base do
> +        before(:all) do
> +          # We don't actually connect (hence the invalid port), but ARes seems to want
> +          # :site defined before you can call collection_path and related methods:
> +          Base.site = "http://localhost:1234/katello/api"
> +        end
> +        it "should default to JSON format" do
> +          Base.format.should equal(ActiveResource::Formats::JsonFormat)
> +        end
> +        it "should not use .json extension" do
> +          Base.collection_path.should_not match('\.json')
> +        end
> +
> +        describe "Derived classes" do
> +          before(:all) do
> +            class Foo<  Base; end
> +            class Bar<  Foo
> +              has_many :foos
> +            end
> +          end
> +
> +          it "should generate the appropriate URL" do
> +            Bar.collection_path.should match('\/bars$')
> +          end
> +
> +          it "should define valid associations for has_many" do
> +            Foo.should_receive(:find).with(:all, {:params=>{:bar_id=>1337}})
> +            Bar.new(:id =>  1337).foos
> +          end
> +        end
> +      end
> +    end
> +  end
> +end
> diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
> index 7761ead..2fbfced 100644
> --- a/spec/spec_helper.rb
> +++ b/spec/spec_helper.rb
> @@ -26,6 +26,8 @@ require 'warehouse/image_build'
>   require 'warehouse/target_image'
>   require 'warehouse/provider_image'
>   require 'warehouse/template'
> +# Katello
> +require 'katello/base'
>
>   RSpec.configure do |config|
>     config.extend VCR::RSpec::Macros




More information about the aeolus-devel mailing list