[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