search

puppetlabs / ruby-hocon

A Ruby port of the Typesafe Config library.
Apache License 2.0
34 stars 30 forks source link

Hocon::Impl::TokenType.name causes "wrong number of arguments (0 for 1)" with specinfra gem #75

Closed domcleal closed 7 years ago

domcleal commented 8 years ago

The hocon gem appears to be causing problems when used in Puppet beaker tests as the Hocon::Impl::TokenType.name method overrides the regular Class#name (well, Module#name) method.

The new method in https://github.com/puppetlabs/ruby-hocon/blob/0.9.5/lib/hocon/impl/token_type.rb#L24 requires an argument and doesn't return the class name.

This causes failures such as:

  5) postgresql::server::recovery should manage recovery File "/tmp/recovery.conf" should contain /restore_command = 'restore_command'/
     Failure/Error: it { is_expected.to contain /restore_command = 'restore_command'/ }
     ArgumentError:
       wrong number of arguments (0 for 1)

     # ./vendor/bundle/ruby/2.1.0/gems/hocon-0.9.5/lib/hocon/impl/token_type.rb:24:in `name'
     # ./vendor/bundle/ruby/2.1.0/gems/specinfra-2.59.1/lib/specinfra/ext/class.rb:5:in `block in subclasses'
     # ./vendor/bundle/ruby/2.1.0/gems/specinfra-2.59.1/lib/specinfra/ext/class.rb:4:in `each_object'
     # ./vendor/bundle/ruby/2.1.0/gems/specinfra-2.59.1/lib/specinfra/ext/class.rb:4:in `subclasses'
     # ./vendor/bundle/ruby/2.1.0/gems/beaker-rspec-5.4.0/lib/beaker-rspec/helpers/serverspec.rb:86:in `detect_os'

(https://travis-ci.org/puppetlabs/puppetlabs-postgresql/jobs/138923171)

as the specinfra gem tries to iterate over every class and calls .name: https://github.com/mizzy/specinfra/commit/8d686d505dc9df6338e70b98ebadf4bec2bacd6d.

It's probably best if the class method on TokenType is renamed to prevent overriding the regular .name method.

domcleal commented 8 years ago

https://github.com/mizzy/specinfra/pull/558 & https://github.com/puppetlabs/beaker-answers/pull/22 have some more info, as it seems master/1.x doesn't trigger the bug with specinfra.

I think it still exists though, and the change in 3783f3053481f46f9113528aab19b4c060490184 (present in 1.x) has masked it by only lazily loading the code. It might be best to fix it still.

cprice404 commented 8 years ago

@domcleal at a glance, I agree with your assessment; we shouldn't be shadowing methods on the ruby Class/Method objects. Will need to audit the hocon code to look for places where our version of the name method is being called and fix them to call the new one.

To get this addressed more quickly we should probably file a Jira ticket in the HC project on the Puppet Jira system. I'll try to do that later today if no one else beats me to it.

cprice404 commented 8 years ago

Filed https://tickets.puppetlabs.com/browse/HC-80 to track this in Jira.

cprice404 commented 8 years ago

@kevpl @lamawithonel @domcleal if any of you were able to validate the fix in #77 that would be great.

cprice404 commented 8 years ago

Released 1.1.0 today, which should fix this. Feel free to open another ticket if it doesn't.