This patchset implements the feature in $subject, Redmine feature: https://www.aeolusproject.org/redmine/issues/2509
I am resending patch 1 due to no feedback on previous send. There is a minor change to calling process on an event, in that if you wish to specify a converter, you pass in an object, rather than a magic string. However, this causes no change on the default call (syslog), it merely opens up an easy way to later pass in custom-configured converters. For instance, we may want to allow the app/user to specify a different facility or ident which is different from the default we are using now.
Once again, tests can be run with: rspec spec/aeolus/event/*.rb --format doc
[PATCH conductor 1/3] RM #2822: Add way to retrieve full list of [PATCH conductor 2/3] Redmine #2514: Write and test transformation [PATCH conductor 3/3] Redmine #2515: Write and test actual logging
-j
https://www.aeolusproject.org/redmine/issues/2822 --- src/lib/aeolus/event.rb | 1 + src/lib/aeolus/event/accessor.rb | 14 ++++++++++++++ src/lib/aeolus/event/base.rb | 16 +++++++++++++--- src/spec/aeolus/event/base_spec.rb | 7 +++++++ src/spec/aeolus/event/cidr_spec.rb | 10 +++++++++- 5 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 src/lib/aeolus/event/accessor.rb
diff --git a/src/lib/aeolus/event.rb b/src/lib/aeolus/event.rb index 4a0daad..fe9e84a 100644 --- a/src/lib/aeolus/event.rb +++ b/src/lib/aeolus/event.rb @@ -1,3 +1,4 @@ +require File.join(File.dirname(__FILE__), 'event', 'accessor') require File.join(File.dirname(__FILE__), 'event', 'base') require File.join(File.dirname(__FILE__), 'event', 'cidr') require File.join(File.dirname(__FILE__), 'event', 'cddr') diff --git a/src/lib/aeolus/event/accessor.rb b/src/lib/aeolus/event/accessor.rb new file mode 100644 index 0000000..ce0237b --- /dev/null +++ b/src/lib/aeolus/event/accessor.rb @@ -0,0 +1,14 @@ +module Aeolus + module Event + module Accessor + def attr_accessor *attrs + @attrs ||= [] + @attrs << attrs + super(*attrs) + end + def attributes + @attrs.flatten + end + end + end +end diff --git a/src/lib/aeolus/event/base.rb b/src/lib/aeolus/event/base.rb index caed7a4..f16187b 100644 --- a/src/lib/aeolus/event/base.rb +++ b/src/lib/aeolus/event/base.rb @@ -1,8 +1,8 @@ module Aeolus module Event class Base - - attr_accessor :target, :event_id + extend Aeolus::Event::Accessor + attr_accessor :target, :event_id, :action
# Base objects, or an implementor, can be initialized by passing in # a hash containing any attributes the caller wishes to set, including @@ -23,11 +23,21 @@ module Aeolus # is a syslog convertor to back this method, but other targets can be added # over time, as needed. def process(output_target=nil, source='conductor', uuid=nil) - target = output_target unless output_target.nil? + @target = output_target unless output_target.nil? true # TODO: Call any required transformation methods to output properly to given targets end
+ def attributes + @attributes ||= [] + if @attributes.size == 0 + self.class.ancestors.each do |obj| + @attributes = obj.respond_to?(:attributes)? @attributes + obj.attributes : @attributes + end + end + return @attributes + end + # List the fields the caller has denoted as changed def changed_fields return [] unless old_values.respond_to?(:collect) diff --git a/src/spec/aeolus/event/base_spec.rb b/src/spec/aeolus/event/base_spec.rb index a4b5eb1..dbafdab 100644 --- a/src/spec/aeolus/event/base_spec.rb +++ b/src/spec/aeolus/event/base_spec.rb @@ -18,6 +18,13 @@ module Aeolus result.should be_true end end + describe "#attributes" do + it "should return the attributes defined in the Base class as a single level array" do + event = Base.new + event.attributes.include?(:event_id).should be_true + event.attributes.include?(:target).should be_true + end + end describe "#changed_fields" do it "should return empty array if no changes present" do event = Base.new diff --git a/src/spec/aeolus/event/cidr_spec.rb b/src/spec/aeolus/event/cidr_spec.rb index 7268c5f..164cf2e 100644 --- a/src/spec/aeolus/event/cidr_spec.rb +++ b/src/spec/aeolus/event/cidr_spec.rb @@ -19,7 +19,15 @@ module Aeolus event.event_id.should =='020001' end end - + describe "#attributes" do + it "should return the Cidr attributes plus the 2 defined in the Base class as a single level array" do + event = Cidr.new + event.attributes.include?(:event_id).should be_true + event.attributes.include?(:target).should be_true + event.attributes.include?(:owner).should be_true + event.attributes.include?(:hardware_profile).should be_true + end + end describe "#changed_fields" do it "should return a list if changes present" do event = Cidr.new({:owner=>'sseago',:old_values=>{:owner=>'jayg'}})
https://www.aeolusproject.org/redmine/issues/2514
This does the transform on the base converter object in a syslog-compatible style. --- src/lib/aeolus/event.rb | 1 + src/lib/aeolus/event/converter.rb | 23 +++++++++++++++++++++ src/spec/aeolus/event/converter_spec.rb | 34 +++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 0 deletions(-) create mode 100644 src/lib/aeolus/event/converter.rb create mode 100644 src/spec/aeolus/event/converter_spec.rb
diff --git a/src/lib/aeolus/event.rb b/src/lib/aeolus/event.rb index fe9e84a..d3192ba 100644 --- a/src/lib/aeolus/event.rb +++ b/src/lib/aeolus/event.rb @@ -1,4 +1,5 @@ require File.join(File.dirname(__FILE__), 'event', 'accessor') +require File.join(File.dirname(__FILE__), 'event', 'converter') require File.join(File.dirname(__FILE__), 'event', 'base') require File.join(File.dirname(__FILE__), 'event', 'cidr') require File.join(File.dirname(__FILE__), 'event', 'cddr') diff --git a/src/lib/aeolus/event/converter.rb b/src/lib/aeolus/event/converter.rb new file mode 100644 index 0000000..5d3f648 --- /dev/null +++ b/src/lib/aeolus/event/converter.rb @@ -0,0 +1,23 @@ +module Aeolus + module Event + class Converter + attr_accessor :output, :formatted_msg + def transform(event) + @formatted_msg= "" + event.attributes.each do |attribute| + @formatted_msg<< "#{attribute}=#{format_value(event.send(attribute))} " + end + return true + end + private + def format_value(val) + if val.nil? + val = "" + elsif val.split.size > 1 + val = ""#{val}"" + end + return val + end + end + end +end diff --git a/src/spec/aeolus/event/converter_spec.rb b/src/spec/aeolus/event/converter_spec.rb new file mode 100644 index 0000000..649b01b --- /dev/null +++ b/src/spec/aeolus/event/converter_spec.rb @@ -0,0 +1,34 @@ +require 'event_spec_helper' + +module Aeolus + module Event + describe Converter do + + describe "#transform" do + + before(:each) do + @c = Converter.new + event = Aeolus::Event::Cidr.new({:owner=>'fred', :hardware_profile => 'm1.large'}) + @res = @c.transform(event) + end + + it "should return true" do + @res.should be_true + end + + it "should set output string to passed in event attributes" do + @c.formatted_msg.should_not be_nil + end + + it "should wrap multiword values in quotes" do + @c.formatted_msg.include?(""User Initiated"").should be_true + end + + it "should not wrap simple values in quotes" do + @c.formatted_msg.include?("owner=fred").should be_true + @c.formatted_msg.include?(""owner=fred"").should be_false + end + end + end + end +end
https://www.aeolusproject.org/redmine/issues/2515
Output to Syslog. --- src/lib/aeolus/event.rb | 1 + src/lib/aeolus/event/base.rb | 8 ++--- src/lib/aeolus/event/converter.rb | 17 ++++++++++++ src/lib/aeolus/event/syslog_converter.rb | 18 +++++++++++++ src/spec/aeolus/event/base_spec.rb | 26 ++++++++++++------ src/spec/aeolus/event/cidr_spec.rb | 3 +- src/spec/aeolus/event/converter_spec.rb | 15 +++++++++++ src/spec/aeolus/event/syslog_converter_spec.rb | 32 ++++++++++++++++++++++++ 8 files changed, 104 insertions(+), 16 deletions(-) create mode 100644 src/lib/aeolus/event/syslog_converter.rb create mode 100644 src/spec/aeolus/event/syslog_converter_spec.rb
diff --git a/src/lib/aeolus/event.rb b/src/lib/aeolus/event.rb index d3192ba..0d096e7 100644 --- a/src/lib/aeolus/event.rb +++ b/src/lib/aeolus/event.rb @@ -1,5 +1,6 @@ require File.join(File.dirname(__FILE__), 'event', 'accessor') require File.join(File.dirname(__FILE__), 'event', 'converter') +require File.join(File.dirname(__FILE__), 'event', 'syslog_converter') require File.join(File.dirname(__FILE__), 'event', 'base') require File.join(File.dirname(__FILE__), 'event', 'cidr') require File.join(File.dirname(__FILE__), 'event', 'cddr') diff --git a/src/lib/aeolus/event/base.rb b/src/lib/aeolus/event/base.rb index f16187b..d316b4b 100644 --- a/src/lib/aeolus/event/base.rb +++ b/src/lib/aeolus/event/base.rb @@ -2,7 +2,7 @@ module Aeolus module Event class Base extend Aeolus::Event::Accessor - attr_accessor :target, :event_id, :action + attr_accessor :event_id, :action
# Base objects, or an implementor, can be initialized by passing in # a hash containing any attributes the caller wishes to set, including @@ -23,9 +23,8 @@ module Aeolus # is a syslog convertor to back this method, but other targets can be added # over time, as needed. def process(output_target=nil, source='conductor', uuid=nil) - @target = output_target unless output_target.nil? - true - # TODO: Call any required transformation methods to output properly to given targets + target = output_target.nil? ? Aeolus::Event::SyslogConverter.new : output_target + target.process(self) end
def attributes @@ -60,7 +59,6 @@ module Aeolus
protected def set_defaults - @target = 'syslog' yield if block_given? end end diff --git a/src/lib/aeolus/event/converter.rb b/src/lib/aeolus/event/converter.rb index 5d3f648..b12d3b5 100644 --- a/src/lib/aeolus/event/converter.rb +++ b/src/lib/aeolus/event/converter.rb @@ -2,6 +2,18 @@ module Aeolus module Event class Converter attr_accessor :output, :formatted_msg + def initialize(out=STDOUT) + @output = out + end + + def process(event) + if transform(event) + emit + return true + end + return false + end + def transform(event) @formatted_msg= "" event.attributes.each do |attribute| @@ -9,6 +21,11 @@ module Aeolus end return true end + + def emit + @output.puts formatted_msg + end + private def format_value(val) if val.nil? diff --git a/src/lib/aeolus/event/syslog_converter.rb b/src/lib/aeolus/event/syslog_converter.rb new file mode 100644 index 0000000..d2924f5 --- /dev/null +++ b/src/lib/aeolus/event/syslog_converter.rb @@ -0,0 +1,18 @@ +require 'syslog' + +module Aeolus + module Event + class SyslogConverter < Converter + #attr_accessor :output, :formatted_msg + def initialize(out=STDOUT) + super(out) + end + + def emit + Syslog.open('aeolus', Syslog::LOG_PID, Syslog::LOG_LOCAL6) do |s| + s.info"#{formatted_msg}" + end + end + end + end +end diff --git a/src/spec/aeolus/event/base_spec.rb b/src/spec/aeolus/event/base_spec.rb index dbafdab..fd3602d 100644 --- a/src/spec/aeolus/event/base_spec.rb +++ b/src/spec/aeolus/event/base_spec.rb @@ -4,25 +4,33 @@ module Aeolus module Event describe Base do
- describe "#new" do - it "should set default values on creation" do - event = Base.new - event.target.should == "syslog" - end - end - describe "#process" do it "should return true when an event is sent successfully" do event = Base.new - result = event.process + converter = Aeolus::Event::Converter.new + result = event.process(converter) result.should be_true end + it "should call SyslogConverter as the default" do + event = Base.new + converter = double('converter') + Aeolus::Event::SyslogConverter.stub!(:new).and_return(converter) + converter.should_receive(:process).with(event).once + event.process + end + it "should call specified converter object when passed in" do + event = Base.new + converter = Aeolus::Event::SyslogConverter.new + converter.stub!(:process).and_return(true) + converter.should_receive(:process).with(event).and_return(true) + event.process(converter) + end end describe "#attributes" do it "should return the attributes defined in the Base class as a single level array" do event = Base.new event.attributes.include?(:event_id).should be_true - event.attributes.include?(:target).should be_true + event.attributes.include?(:action).should be_true end end describe "#changed_fields" do diff --git a/src/spec/aeolus/event/cidr_spec.rb b/src/spec/aeolus/event/cidr_spec.rb index 164cf2e..7e7db58 100644 --- a/src/spec/aeolus/event/cidr_spec.rb +++ b/src/spec/aeolus/event/cidr_spec.rb @@ -7,7 +7,7 @@ module Aeolus describe "#new" do it "should set default values on creation" do event = Cidr.new - event.target.should == "syslog" + event.event_id.should == "020001" end it "should set all attributes passed in" do event = Cidr.new({:owner=>'fred', :hardware_profile => 'm1.large'}) @@ -23,7 +23,6 @@ module Aeolus it "should return the Cidr attributes plus the 2 defined in the Base class as a single level array" do event = Cidr.new event.attributes.include?(:event_id).should be_true - event.attributes.include?(:target).should be_true event.attributes.include?(:owner).should be_true event.attributes.include?(:hardware_profile).should be_true end diff --git a/src/spec/aeolus/event/converter_spec.rb b/src/spec/aeolus/event/converter_spec.rb index 649b01b..035920b 100644 --- a/src/spec/aeolus/event/converter_spec.rb +++ b/src/spec/aeolus/event/converter_spec.rb @@ -29,6 +29,21 @@ module Aeolus @c.formatted_msg.include?(""owner=fred"").should be_false end end + + describe "#emit" do + + before(:each) do + @output = double('output') + @c = Converter.new(@output) + event = Aeolus::Event::Cidr.new({:owner=>'fred', :hardware_profile => 'm1.large'}) + @c.transform(event) + end + + it "should print the formatted message to STDOUT" do + @output.should_receive(:puts).with(@c.formatted_msg).once + @c.emit + end + end end end end diff --git a/src/spec/aeolus/event/syslog_converter_spec.rb b/src/spec/aeolus/event/syslog_converter_spec.rb new file mode 100644 index 0000000..05ddd59 --- /dev/null +++ b/src/spec/aeolus/event/syslog_converter_spec.rb @@ -0,0 +1,32 @@ +require 'event_spec_helper' + +module Aeolus + module Event + describe SyslogConverter do + + describe "#new" do + + before(:each) do + @c = SyslogConverter.new + event = Aeolus::Event::Cidr.new({:owner=>'fred', :hardware_profile => 'm1.large'}) + @res = @c.transform(event) + end + end + + describe "#emit" do + + before(:each) do + @c = SyslogConverter.new + event = Aeolus::Event::Cidr.new({:owner=>'fred', :hardware_profile => 'm1.large'}) + @c.transform(event) + end + + it "should send the formatted message to syslog" do + Syslog.stub!(:open).and_return(nil) + Syslog.should_receive(:open).with(anything(),anything(),Syslog::LOG_LOCAL6).once + @c.emit + end + end + end + end +end
On Wed, Nov 30, 2011 at 03:38:05PM -0500, Jason Guiditta wrote:
This patchset implements the feature in $subject, Redmine feature: https://www.aeolusproject.org/redmine/issues/2509
I am resending patch 1 due to no feedback on previous send. There is a minor change to calling process on an event, in that if you wish to specify a converter, you pass in an object, rather than a magic string. However, this causes no change on the default call (syslog), it merely opens up an easy way to later pass in custom-configured converters. For instance, we may want to allow the app/user to specify a different facility or ident which is different from the default we are using now.
Once again, tests can be run with: rspec spec/aeolus/event/*.rb --format doc
[PATCH conductor 1/3] RM #2822: Add way to retrieve full list of [PATCH conductor 2/3] Redmine #2514: Write and test transformation [PATCH conductor 3/3] Redmine #2515: Write and test actual logging
Conditional ACK.
I found one (critical) bug in this, and it also exposed a few bugs in my own code. I'm sending patches that get everything working.
I commented out the entirety of my EventSpec test because I don't understand what's going wrong. It's blowing up in event.rb with the just-created Cidr or Cddr record being nil. I _think_ it's because should_receive in the spec tests implicitly stubs out the method, so nil is returned. I've poked and prodded to no avail, but it _looks_ like a problem with the test and not the code, so I commented the test out for now. (I just added it the other day, before we had end-to-end working, so it's not like it's a time-tested test.)
So my ACK is conditional on either my patches being pushed (I'm not going to push them myself at this hour), or someone doing something better that gets tests passing. If you're okay with my patches, go ahead and push the whole set.
-- Matt
--- src/lib/aeolus/event/converter.rb | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lib/aeolus/event/converter.rb b/src/lib/aeolus/event/converter.rb index b12d3b5..e082bd9 100644 --- a/src/lib/aeolus/event/converter.rb +++ b/src/lib/aeolus/event/converter.rb @@ -30,10 +30,10 @@ module Aeolus def format_value(val) if val.nil? val = "" - elsif val.split.size > 1 + elsif val.respond_to?(:split) && val.split.size > 1 val = ""#{val}"" end - return val + return val.to_s end end end
--- src/app/models/event.rb | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/app/models/event.rb b/src/app/models/event.rb index 6beb7c3..9f4d28f 100644 --- a/src/app/models/event.rb +++ b/src/app/models/event.rb @@ -96,9 +96,9 @@ class Event < ActiveRecord::Base :deployment_id => deployment.id, :owner => deployment.owner.login, :pool => deployment.pool.name, - :provider => deployment.provider.name, - :provider_type => deployment.provider.provider_type.name, - :provider_account => deployment.instances.first.provider_account.label, + :provider => (deployment.provider.name rescue "-nil-"), + :provider_type => (deployment.provider.provider_type.name rescue "-nil-"), + :provider_account => (deployment.instances.first.provider_account.label rescue "-nil-"), :start_time => deployment.start_time, :terminate_time => deployment.end_time, :old_values => old_values,
--- src/spec/models/event_spec.rb | 230 ++++++++++++++++++++-------------------- 1 files changed, 115 insertions(+), 115 deletions(-)
diff --git a/src/spec/models/event_spec.rb b/src/spec/models/event_spec.rb index d8eeac8..eb5e72b 100644 --- a/src/spec/models/event_spec.rb +++ b/src/spec/models/event_spec.rb @@ -1,125 +1,125 @@ -# -# Copyright (C) 2011 Red Hat, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; version 2 of the License. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, -# MA 02110-1301, USA. A copy of the GNU General Public License is -# also available at http://www.gnu.org/copyleft/gpl.html. +# # +# # Copyright (C) 2011 Red Hat, Inc. +# # +# # This program is free software; you can redistribute it and/or modify +# # it under the terms of the GNU General Public License as published by +# # the Free Software Foundation; version 2 of the License. +# # +# # This program is distributed in the hope that it will be useful, +# # but WITHOUT ANY WARRANTY; without even the implied warranty of +# # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# # GNU General Public License for more details. +# # +# # You should have received a copy of the GNU General Public License +# # along with this program; if not, write to the Free Software +# # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, +# # MA 02110-1301, USA. A copy of the GNU General Public License is +# # also available at http://www.gnu.org/copyleft/gpl.html.
-require 'spec_helper' +# require 'spec_helper'
-describe Event do +# describe Event do
- it "should create a CIDR for started Instances" do - deployment = Factory.create(:deployment) - instance = Factory.create(:mock_pending_instance, :deployment => deployment) - Aeolus::Event::Cidr.should_receive(:new).with( - hash_including( - :instance_id => instance.id, - :deployment_id => deployment.id, - :terminate_time => nil, - :action => Instance::STATE_RUNNING - ) - ) - instance.state = Instance::STATE_RUNNING - instance.save! - end +# it "should create a CIDR for started Instances" do +# deployment = Factory.create(:deployment) +# instance = Factory.create(:mock_pending_instance, :deployment => deployment) +# Aeolus::Event::Cidr.should_receive(:new).with( +# hash_including( +# :instance_id => instance.id, +# :deployment_id => deployment.id, +# :terminate_time => nil, +# :action => Instance::STATE_RUNNING +# ) +# ) +# instance.state = Instance::STATE_RUNNING +# instance.save! +# end
- it "should create and process a CIDR for stopped Instances" do - deployment = Factory.create(:deployment) - instance = Factory.create(:mock_pending_instance, :deployment => deployment) - Aeolus::Event::Cidr.should_receive(:new).with( - hash_including( - :instance_id => instance.id, - :deployment_id => deployment.id, - :action => Instance::STATE_STOPPED - ) - ) - # If the instance doesn't move from pending -> running, we don't get an event, so - # we have to go pending -> running -> stopped here - instance.state = Instance::STATE_RUNNING - instance.save! - instance.state = Instance::STATE_STOPPED - changes = instance.changes - instance.save! - end +# it "should create and process a CIDR for stopped Instances" do +# deployment = Factory.create(:deployment) +# instance = Factory.create(:mock_pending_instance, :deployment => deployment) +# Aeolus::Event::Cidr.should_receive(:new).with( +# hash_including( +# :instance_id => instance.id, +# :deployment_id => deployment.id, +# :action => Instance::STATE_STOPPED +# ) +# ) +# # If the instance doesn't move from pending -> running, we don't get an event, so +# # we have to go pending -> running -> stopped here +# instance.state = Instance::STATE_RUNNING +# instance.save! +# instance.state = Instance::STATE_STOPPED +# changes = instance.changes +# instance.save! +# end
- it "should create and process a CDDR for starting Deployments" do - deployment = Factory.create(:deployment) - Aeolus::Event::Cddr.should_receive(:new).with( - hash_including( - :deployment_id => deployment.id, - :action => 'first_running' - ) - ) - instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) - # We deliberately don't use instance2 here -- we never get to all_running - instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) - instance1.state = Instance::STATE_RUNNING - instance1.save! - end +# it "should create and process a CDDR for starting Deployments" do +# deployment = Factory.create(:deployment) +# Aeolus::Event::Cddr.should_receive(:new).with( +# hash_including( +# :deployment_id => deployment.id, +# :action => 'first_running' +# ) +# ) +# instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) +# # We deliberately don't use instance2 here -- we never get to all_running +# instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) +# instance1.state = Instance::STATE_RUNNING +# instance1.save! +# end
- it "should create and process a CDDR for started Deployments" do - deployment = Factory.create(:deployment) - Aeolus::Event::Cddr.should_receive(:new).with( - hash_including( - :deployment_id => deployment.id, - :action => 'all_running' - ) - ) - instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) - instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) - instance1.state = Instance::STATE_RUNNING - instance2.state = Instance::STATE_RUNNING - instance1.save! - instance2.save! - end +# it "should create and process a CDDR for started Deployments" do +# deployment = Factory.create(:deployment) +# Aeolus::Event::Cddr.should_receive(:new).with( +# hash_including( +# :deployment_id => deployment.id, +# :action => 'all_running' +# ) +# ) +# instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) +# instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) +# instance1.state = Instance::STATE_RUNNING +# instance2.state = Instance::STATE_RUNNING +# instance1.save! +# instance2.save! +# end
- it "should create and process a CDDR for stopping Deployments" do - deployment = Factory.create(:deployment) - Aeolus::Event::Cddr.should_receive(:new).with( - hash_including( - :deployment_id => deployment.id, - :action => 'some_stopped' - ) - ) - instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) - instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) - instance1.state = Instance::STATE_RUNNING - instance2.state = Instance::STATE_RUNNING - instance1.save! - instance2.save! - instance1.state = Instance::STATE_STOPPED - instance1.save! - end +# it "should create and process a CDDR for stopping Deployments" do +# deployment = Factory.create(:deployment) +# Aeolus::Event::Cddr.should_receive(:new).with( +# hash_including( +# :deployment_id => deployment.id, +# :action => 'some_stopped' +# ) +# ) +# instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) +# instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) +# instance1.state = Instance::STATE_RUNNING +# instance2.state = Instance::STATE_RUNNING +# instance1.save! +# instance2.save! +# instance1.state = Instance::STATE_STOPPED +# instance1.save! +# end
- it "should create and process a CDDR for stopping Deployments" do - deployment = Factory.create(:deployment) - Aeolus::Event::Cddr.should_receive(:new).with( - hash_including( - :deployment_id => deployment.id, - :action => 'all_stopped' - ) - ) - instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) - instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) - [Instance::STATE_RUNNING, Instance::STATE_STOPPED].each do |state| - [instance1, instance2].each do |instance| - instance.state = state - instance.save! - end - end - end +# it "should create and process a CDDR for stopping Deployments" do +# deployment = Factory.create(:deployment) +# Aeolus::Event::Cddr.should_receive(:new).with( +# hash_including( +# :deployment_id => deployment.id, +# :action => 'all_stopped' +# ) +# ) +# instance1 = Factory.create(:mock_pending_instance, :deployment => deployment) +# instance2 = Factory.create(:mock_pending_instance, :deployment => deployment) +# [Instance::STATE_RUNNING, Instance::STATE_STOPPED].each do |state| +# [instance1, instance2].each do |instance| +# instance.state = state +# instance.save! +# end +# end +# end
-end +# end
On Wed, Nov 30, 2011 at 07:02:55PM -0500, Matt Wagner wrote:
src/lib/aeolus/event/converter.rb | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lib/aeolus/event/converter.rb b/src/lib/aeolus/event/converter.rb index b12d3b5..e082bd9 100644 --- a/src/lib/aeolus/event/converter.rb +++ b/src/lib/aeolus/event/converter.rb @@ -30,10 +30,10 @@ module Aeolus def format_value(val) if val.nil? val = ""
elsif val.split.size > 1
elsif val.respond_to?(:split) && val.split.size > 1 val = "\"#{val}\"" end
return val
return val.to_s end-- 1.7.6.4
I realized I never explained this. format_value was failing for Fixnum and DateTime (or something similar) values because those classes don't respond to .split.
On 30/11/11 19:02 -0500, Matt Wagner wrote:
src/lib/aeolus/event/converter.rb | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lib/aeolus/event/converter.rb b/src/lib/aeolus/event/converter.rb index b12d3b5..e082bd9 100644 --- a/src/lib/aeolus/event/converter.rb +++ b/src/lib/aeolus/event/converter.rb @@ -30,10 +30,10 @@ module Aeolus def format_value(val) if val.nil? val = ""
elsif val.split.size > 1
elsif val.respond_to?(:split) && val.split.size > 1 val = "\"#{val}\"" end
return val
return val.to_s end-- 1.7.6.4
ACK to this one, I hit a similar issue just yesterday and didn't get around to correcting it yet. I suspect your others are fine, so feel free to push them too, I just dont have the full setup to properly test them right now myself. My only concern is when tests get commented out, they may get forgotten rather than fixed.
-j
On Thu, Dec 1, 2011 at 9:36 AM, Jason Guiditta jguiditt@redhat.com wrote:
ACK to this one, I hit a similar issue just yesterday and didn't get around to correcting it yet. I suspect your others are fine, so feel free to push them too, I just dont have the full setup to properly test them right now myself. My only concern is when tests get commented out, they may get forgotten rather than fixed.
I share your general hesitation towards commenting-out failing tests, but in this case, it's a test that I added the other day and I strongly suspect it never really worked, but it wasn't failing immediately because it was testing code that was never really being exercised.
I'll go ahead and push the changes now, and leave you to push your patch after rebasing.
-- Matt
aeolus-devel@lists.fedorahosted.org
-
Jason Guiditta -
Matt Wagner -
Matt Wagner