On Tue, 2011-06-14 at 15:55 +0100, mtaylor@redhat.com wrote:
From: Martyn Taylor mtaylor@redhat.com
.../aeolus-image/lib/config_parser.rb | 10 +++++++++- .../image_factory/aeolus-image/lib/list_command.rb | 5 +++++ .../aeolus-image/spec/list_command_spec.rb | 17 +++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletions(-) create mode 100644 services/image_factory/aeolus-image/spec/list_command_spec.rb
ok, this one almost works, but needs a couple minor fixes. First, the tests fail. I think I mentioned this in irc or something, but the test that checks for proper parsing of the command now fails, as the command cannot be executed as is in the test. My suggestion is to change that test to look for the expected failure, and add a new one that actually supplies the needed subcommand (or just calls delete, which is unimplemented, so should not error). The errors I get from 'rake spec' are: 1) 'Aeolus::Image::ListCommand should return a list of images' FAILED expected not: == nil, got: nil ./spec/list_command_spec.rb:11: ./spec/list_command_spec.rb:10:in `each' ./spec/list_command_spec.rb:10:
2) SystemExit in 'Aeolus::Image::ConfigParser should parse the specified command' exit ./spec/config_parser_spec.rb:8:
Finished in 26.533244 seconds
15 examples, 2 failures
Running the command directly with the following works fine after one minor fix: ruby -r 'rubygems' bin/aeolus-image.rb list --images
Couple other fixes listed inline
diff --git a/services/image_factory/aeolus-image/lib/config_parser.rb b/services/image_factory/aeolus-image/lib/config_parser.rb index 3ee8b1e..91b637a 100644 --- a/services/image_factory/aeolus-image/lib/config_parser.rb +++ b/services/image_factory/aeolus-image/lib/config_parser.rb @@ -1,3 +1,4 @@ +require 'rubygems'
rubygems should not be directly required by any of our classes, this will be added by rubygems itself once we package this. In the meantime, everything works as designed by calling: ruby -r 'rubygems' bin/aeolus-image <command> <options>
require 'optparse' require 'logger' require 'base_command' @@ -171,7 +172,14 @@ module Aeolus # For the list example above, that object would call a method 'images' based on the item # @options[:subcommand] being :images, so internally that class may do something like: # self.send(@options[:subcommand])
"Not implemented"
if !@options['subcommand'].nil?
The above check fails, it needs to be looking for @options[:subcommand]. Same comment I made before about switching if to unless here, as I think it is more readable, but that is just a personal preference, so you can leave it if you want.
list_command = ListCommand.new(@options)
list_command.send(@options[:subcommand])
else
# TODO: Pull out Print Usage into seporate method, and print
puts "Could not find subcommand for list, run `./aeolus-image --help` for usage instructions"
exit(1)
end end def build
diff --git a/services/image_factory/aeolus-image/lib/list_command.rb b/services/image_factory/aeolus-image/lib/list_command.rb index 35f6927..9939f74 100644 --- a/services/image_factory/aeolus-image/lib/list_command.rb +++ b/services/image_factory/aeolus-image/lib/list_command.rb @@ -4,6 +4,11 @@ module Aeolus def initialize(opts={}, logger=nil) super(opts, logger) end
def images
doc = Nokogiri::XML iwhd['/images'].get
doc.xpath("/objects/object/key").collect { |node| "uuid: " + node.text }
end endend
end diff --git a/services/image_factory/aeolus-image/spec/list_command_spec.rb b/services/image_factory/aeolus-image/spec/list_command_spec.rb new file mode 100644 index 0000000..01d3646 --- /dev/null +++ b/services/image_factory/aeolus-image/spec/list_command_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper'
+module Aeolus
- module Image
- describe ListCommand do
it "should return a list of images" do
regexp = Regexp.new('[uuid:\s][\w]{8}[-][\w]{4}[-][\w]{4}[-][\w]{4}[-][\w]{12}')
listc = ListCommand.new
images = listc.images
images.each do |image|
regexp.match(image.to_s).should_not == nil
end
end
- end
- end
+end \ No newline at end of file