Arvados

Fix at c8a640d52 - branch 16826-unlogged-attrs-fix
Test run: developer-run-tests: #2089

The problem was caused by the fact that the ActiveSupport::OrderedOptions class we use on the config.yml loading code always return keys as symbols even if they're assigned as strings:

2.5.5 :001 > require 'active_support'
 => true
2.5.5 :002 > opts = ActiveSupport::OrderedOptions.new
 => {}
2.5.5 :003 > opts['someKey'] = 'someValue'
 => "someValue" 
2.5.5 :004 > opts.keys
 => [:someKey]

This was tested on rails 5.0, 5.2 & 6.0

• Added a test that confirms loaded config maps are OrderedOptions classes by checking Users.AutoSetupUsernameBlacklist that has predefined members by default, with a check that the keys are in fact being returned as symbols.
• Set up the AuditLogs.UnloggedAttributes related tests with OrderedOptions instead of hashes to expose the bug.
• Applied the fix proposed by the customer
• Also set up other config map related tests accordingly to see if any test started to fail (no failures, fortunately)

This LGTM, thanks.

I'm a little concerned that it's too easy to repeat the same mistake (assigning regular hashes to config) in future. Maybe restore_configuration() in source:services/api/test/test_helper.rb could do a recursive "symbol keys" test on Rails.configuration to catch that?

Tom Clegg wrote:

I'm a little concerned that it's too easy to repeat the same mistake (assigning regular hashes to config) in future. Maybe restore_configuration() in source:services/api/test/test_helper.rb could do a recursive "symbol keys" test on Rails.configuration to catch that?

Good idea!!

Updates at f4d0ea9ca
Test run: developer-run-tests: #2092

• Adds config checks on TestCase.teardown.
• Moves restore_configuration() to TestCase.setup to avoid additional test failures after a config check assertion fail.
• Fixes illegal test config settings discovered by the new check.
• Fixes API.DisabledAPIs auto-setting depending on Containers.JobsAPI.Enable's value.

Great, more hidden bugs revealed & squashed! LGTM, thanks.