[katello-devel] A couple of recent changes in the codebase

[Petr Chalupa]

On 01.02.13 13:59, Dmitri Dolguikh wrote:
> On 01/02/13 12:49 PM, Petr Chalupa wrote:
>>
>>
>> On 29.01.13 23:28, Dmitri Dolguikh wrote:
>>> Hey all,
>>>
>>> I noticed two somewhat major changes in the Katello codebase, both of
>>> which (at least in part) should be remedied.
>>>
>>> - Lots of tests now depend on configuration file values.  For example:
>>>
>>>       Failure/Error: user.save!
>>>       Katello::Configuration::Node::NoKey:
>>>          missing key 'random_password' in configuration
>>>       # ./lib/katello_config.rb:135:in `rescue in method_missing'
>>>       # ./lib/katello_config.rb:132:in `method_missing'
>>>       # ./app/models/glue/foreman/user.rb:69:in `create_foreman_user'
>>>       # ./app/models/glue.rb:177:in `execute'
>>>       # ./app/models/glue.rb:127:in `block in process'
>>>       # ./app/models/glue.rb:114:in `each'
>>>       # ./app/models/glue.rb:114:in `process'
>>>       # ./app/models/glue.rb:38:in `on_save'
>>>       # ./lib/lazy_accessor.rb:108:in `save!'
>>>       # ./spec/models/glue/foreman/user_spec.rb:49:in `block (2 levels)
>>> in <top (required)>'
>>
>> The error you've shown should be easily fixed by updating
>> `katello.yml` from `katello.template.yml`. The `katello.yml` is
>> probably just outdated. It's failing because Katello is misconfigured.
>>
>>> Developer tests in general should not depend on external configuration,
>>> please avoid such techniques in the future.
>>
>> We were always dependent on `katello.yml`. It just somehow worked even
>> if the configuration was wrong. Database and other default
>> configurations are in `katello.yml`. Therefore even if I like it, I
>> think it is not worth it to pursue this goal.
>>
>> Validation of configuration in test environment could be more relaxed
>> but I would rather see these misconfiguration errors.
>
> The issue is that we have conditional code in tests, which means test
> results may depend on the external configuration, which is A BAD THING(tm):
>   - you don't know which functionality you are exercising when executing
> tests.
>   - you are not exercising *ALL* functionality - changes that work under
> 'Katello' mode can break 'Headpin' mode.
>   - incorrect/incomplete configuration file breaks a ton of tests
>

I agree. I just did not see a easy solution for the problem but I may 
just got an idea. It would not be difficult to load an invariable 
configuration (only for test) in `spec_helper` and deep_merge in 
selectively only database configuration. Classes from 
`lib/katello_config.rb` should help to achieve it. This would froze the 
configuration in test environment.

Making the tests truly independent on configuration would be much more 
difficult.

> -d
>
>
>
> _______________________________________________
> katello-devel mailing list
> katello-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/katello-devel