[Ovirt-devel] [PATCH server] Fixes broken unit tests for bonding classes and the host browser.

[Mohammed Morsi]

Darryl L. Pierce wrote:
> Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
> ---
>  src/app/models/bonding.rb          |    6 ++++++
>  src/app/models/bonding_type.rb     |    4 ++++
>  src/test/unit/bonding_test.rb      |   10 +++++-----
>  src/test/unit/bonding_type_test.rb |    2 +-
>  4 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
> index 941e2cd..4b6e8a5 100644
> --- a/src/app/models/bonding.rb
> +++ b/src/app/models/bonding.rb
> @@ -33,6 +33,12 @@ class Bonding < ActiveRecord::Base
>    validates_presence_of :name,
>      :message => 'A name is required.'
>  
> +  validates_presence_of :host_id,
> +    :message => 'A host must be specified.'
> +
> +  validates_presence_of :bonding_type_id,
> +    :message => 'A bonding type must be specified.'
> +
>    validates_presence_of :interface_name,
>      :message => 'An interface name is required.'
>  
> diff --git a/src/app/models/bonding_type.rb b/src/app/models/bonding_type.rb
> index e0d2193..f844df1 100644
> --- a/src/app/models/bonding_type.rb
> +++ b/src/app/models/bonding_type.rb
> @@ -22,5 +22,9 @@
>  #
>  class BondingType < ActiveRecord::Base
>    validates_presence_of :label
> +  validates_uniqueness_of :label,
> +    :message => 'Label must be unique'
>    validates_presence_of :mode
> +  validates_uniqueness_of :mode,
> +    :message => 'Mode must be unique'
>  end
> diff --git a/src/test/unit/bonding_test.rb b/src/test/unit/bonding_test.rb
> index 4bdb079..a2cdef8 100644
> --- a/src/test/unit/bonding_test.rb
> +++ b/src/test/unit/bonding_test.rb
> @@ -28,10 +28,10 @@ class BondingTest < ActiveSupport::TestCase
>  
>    def setup
>      @bonding = Bonding.new(
> -      :name           => 'Bonding1',
> -      :interface_name => 'bond0',
> -      :type_id        => bonding_types(:failover_bonding_type),
> -      :host_id        => hosts(:mailservers_managed_node))
> +      :name            => 'Bonding1',
> +      :interface_name  => 'bond0',
> +      :bonding_type_id => bonding_types(:failover_bonding_type),
> +      :host_id         => hosts(:mailservers_managed_node))
>    end
>  
>    # Ensures that the name is required.
> @@ -53,7 +53,7 @@ class BondingTest < ActiveSupport::TestCase
>    # Ensures that the bonding type is required.
>    #
>    def test_valid_fails_without_type
> -    @bonding.type_id = nil
> +    @bonding.bonding_type_id = nil
>  
>      flunk 'Bondings have to have a valid type.' if @bonding.valid?
>    end
> diff --git a/src/test/unit/bonding_type_test.rb b/src/test/unit/bonding_type_test.rb
> index 5f2ccb2..c900002 100644
> --- a/src/test/unit/bonding_type_test.rb
> +++ b/src/test/unit/bonding_type_test.rb
> @@ -29,6 +29,6 @@ class BondingTypeTest < ActiveSupport::TestCase
>    def test_modes_must_be_unique
>      @bonding_type.mode = BondingType.find(:first).mode
>  
> -    assert @bonding_type.save == false, 'A bonding type with a duplicate mode should not save.'
> +    flunk "Modes must be unique" if @bonding_type.valid?
>    end
>  end
>   
ACK as this patch seems to take care of the bonding problems. As
mentioned I'm seeing two last broken test cases, both seeming to stem
from the same problem (they are attached). It seems that in
host_browser.rb, specifically write_host_info line 305 doesn't succeed
in retrieving BootType from proto in those two broken cases. I've
verified that the contents of the BootType table is null there where as
other tables have data (crudely tested by putting a long sleep
immediately before the line in question and then running the psql client
manually and selecting records ). I tried debugging it for a while
yesterday but was unable to come up with a solution, perhaps we can
figure out something together. (a theory I had was that some deletion is
cascading and knocking out the BootType records). Perhaps if everything
is working locally for you, there is still a bit of code in your local
repo that you haven't sent out yet? I'm wondering because I see  you
mention host_browser in the subject line but don't see the actual
changes to any host browser module in your attached patch.

  Thanks alot,
     Mo

  1) Failure:
test_write_host_info_with_duplicate_nic(HostBrowserIdentifyTest)
    [./test/unit/host_browser_identify_test.rb:226:in
`test_write_host_info_with_duplicate_nic'
    
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`__send__'
    
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`run']:
Exception raised:
Class: <RuntimeError>
Message: <"Called id for nil, which would mistakenly be 4 -- if you
really wanted the id of nil, use object_id">
---Backtrace---
./test/unit/../../host-browser/host-browser.rb:317:in `write_host_info'
./test/unit/../../host-browser/host-browser.rb:308:in `collect'
./test/unit/../../host-browser/host-browser.rb:308:in `write_host_info'
./test/unit/host_browser_identify_test.rb:226:in
`test_write_host_info_with_duplicate_nic'
./test/unit/host_browser_identify_test.rb:226:in
`test_write_host_info_with_duplicate_nic'
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`__send__'
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`run'
---------------

  2) Failure:
test_write_host_info_with_missing_uuid(HostBrowserIdentifyTest)
    [./test/unit/host_browser_identify_test.rb:167:in
`test_write_host_info_with_missing_uuid'
    
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`__send__'
    
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`run']:
Exception raised:
Class: <RuntimeError>
Message: <"Called id for nil, which would mistakenly be 4 -- if you
really wanted the id of nil, use object_id">
---Backtrace---
./test/unit/../../host-browser/host-browser.rb:317:in `write_host_info'
./test/unit/../../host-browser/host-browser.rb:308:in `collect'
./test/unit/../../host-browser/host-browser.rb:308:in `write_host_info'
./test/unit/host_browser_identify_test.rb:167:in
`test_write_host_info_with_missing_uuid'
./test/unit/host_browser_identify_test.rb:167:in
`test_write_host_info_with_missing_uuid'
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`__send__'
/usr/lib/ruby/gems/1.8/gems/activesupport-2.1.1/lib/active_support/testing/setup_and_teardown.rb:33:in
`run'
---------------

38 tests, 47 assertions, 2 failures, 0 errors