🐛 Fixed new defaults handling

- Moved CMock static defaults to defaults.rb
- Added logic to `merge_tool_defauts()` that handles the very likely scneario of certain optional project configuration values missing in a user-provided configuration
- Refactored `ConfigWalkinator.fetch_value()` to be more capable and safe
- Added tests for `ConfigWalkinator.fetch_value()`

bin/cli_helper.rb

@@ -68,9 +68,9 @@ def which_ceedling?(env:, config:{}, app_cfg:)
 
     # Configuration file
     if which_ceedling.nil?
-      walked = @config_walkinator.fetch_value( config, :project, :which_ceedling )
-      if !walked[:value].nil?
-        which_ceedling = walked[:value].strip()
+      value, _ = @config_walkinator.fetch_value( :project, :which_ceedling, hash:config )
+      if !value.nil?
+        which_ceedling = value.strip()
         @loginator.log( " > Set which Ceedling from config :project ↳ :which_ceedling => #{which_ceedling}", Verbosity::OBNOXIOUS )
         which_ceedling = :gem if (which_ceedling.casecmp( 'gem' ) == 0)
       end
@@ -139,13 +139,13 @@ def process_testcase_filters(config:, include:, exclude:, tasks:, default_tasks:
     #       raise an exception if --graceful-fail is set without test operations
 
     # Add test runner configuration setting necessary to use test case filters
-    walked = @config_walkinator.fetch_value( config, :test_runner )
-    if walked[:value].nil?
+    value, _ = @config_walkinator.fetch_value( :test_runner, hash:config )
+    if value.nil?
       # If no :test_runner section, create the whole thing
       config[:test_runner] = {:cmdline_args => true}
     else
       # If a :test_runner section, just set :cmdlne_args
-      walked[:value][:cmdline_args] = true
+      value[:cmdline_args] = true
     end
   end
 
@@ -162,8 +162,8 @@ def process_graceful_fail(config:, cmdline_graceful_fail:, tasks:, default_tasks
     return cmdline_graceful_fail if !cmdline_graceful_fail.nil?
 
     # If configuration contains :graceful_fail, use it
-    walked = @config_walkinator.fetch_value( config, :test_build, :graceful_fail )
-    return walked[:value] if !walked[:value].nil?
+    value, _ = @config_walkinator.fetch_value( :test_build, :graceful_fail, hash:config )
+    return value if value.nil?
 
     return false
   end
@@ -256,18 +256,18 @@ def dump_yaml(config, filepath, sections)
       _sections = sections.map {|section| section.to_sym}
 
       # Try to extract subconfig from section path
-      walked = @config_walkinator.fetch_value( config, *_sections )
+      value, _ = @config_walkinator.fetch_value( *_sections, hash:config )
 
       # If we fail to find the section path, blow up
-      if walked[:value].nil?
+      if value.nil?
         # Reformat list of symbols to list of :<section>s
         _sections.map! {|section| ":#{section.to_s}"}
         msg = "Cound not find configuration section #{_sections.join(' ↳ ')}"
         raise(msg)
       end
 
       # Update _config to subconfig with final sections path element as container
-      _config = { _sections.last => walked[:value] }
+      _config = { _sections.last => value }
     end
 
     File.open( filepath, 'w' ) {|out| YAML.dump( _config, out )}

bin/configinator.rb

@@ -96,10 +96,10 @@ def loadinate(builtin_mixins:, filepath:nil, mixins:[], env:{}, silent:false)
   def default_tasks(config:, default_tasks:)
     #  1. If :default_tasks set in config, use it
     #  2. Otherwise use the function argument (most likely a default set in the first moments of startup)
-    walked = @config_walkinator.fetch_value( config, :project, :default_tasks )
-    if walked[:value]
+    value, _ = @config_walkinator.fetch_value( :project, :default_tasks, hash:config )
+    if value
       # Update method parameter to config value
-      default_tasks = walked[:value].dup()
+      default_tasks = value.dup()
     else
       # Set key/value in config if it's not set
       config.deep_merge( {:project => {:default_tasks => default_tasks}} )

lib/ceedling/config_walkinator.rb

@@ -7,23 +7,29 @@
 
 class ConfigWalkinator
 
-  def fetch_value(hash, *keys)
-    value = nil
+  def fetch_value(*keys, hash:, default:nil)
+    # Safe initial values
+    value = default
     depth = 0
 
-    # walk into hash & extract value at requested key sequence
+    # Set walk variable
+    walk = hash
+
+    # Walk into hash & extract value at requested key sequence
     keys.each { |symbol|
-      depth += 1
-      if (not hash[symbol].nil?)
-        hash  = hash[symbol]
-        value = hash
-      else
-        value = nil
+      # Validate that we can fetch something meaningful
+      if !walk.is_a?( Hash) or !symbol.is_a?( Symbol ) or walk[symbol].nil?
+        value = default
         break
       end
-    } if !hash.nil?
+
+      # Walk into the hash one more level and update value
+      depth += 1
+      walk  = walk[symbol]
+      value = walk
+    } if !walk.nil?
 
-    return {:value => value, :depth => depth}
+    return value, depth
   end
 
 end

lib/ceedling/configurator.rb

@@ -16,10 +16,7 @@ class Configurator
   attr_reader :project_config_hash, :programmatic_plugins, :rake_plugins
   attr_accessor :project_logging, :sanity_checks, :include_test_case, :exclude_test_case
 
-  constructor(:configurator_setup, :configurator_builder, :configurator_plugins, :yaml_wrapper, :system_wrapper, :loginator, :reportinator) do
-    @project_logging = false
-    @sanity_checks   = TestResultsSanityChecks::NORMAL
-  end
+  constructor :configurator_setup, :configurator_builder, :configurator_plugins, :config_walkinator, :yaml_wrapper, :system_wrapper, :loginator, :reportinator
 
   def setup()
     # Cmock config reference to provide to CMock for mock generation
@@ -34,6 +31,9 @@ def setup()
 
     @programmatic_plugins = []
     @rake_plugins   = []
+
+    @project_logging = false
+    @sanity_checks   = TestResultsSanityChecks::NORMAL
   end
 
   # Override to prevent exception handling from walking & stringifying the object variables.
@@ -87,45 +87,39 @@ def set_verbosity(config)
   end
 
 
-  # The default values defined in defaults.rb (eg. DEFAULT_TOOLS_TEST) are populated
-  # into @param config
+  # The default tools (eg. DEFAULT_TOOLS_TEST) are merged into default config hash
   def merge_tools_defaults(config, default_config)
     msg = @reportinator.generate_progress( 'Collecting default tool configurations' )
     @loginator.log( msg, Verbosity::OBNOXIOUS )
 
+    # config[:project] is guaranteed to exist / validated to exist
+    # config[:test_build] and config[:release_build] are optional in a user project configuration
+    release_assembly, _ = @config_walkinator.fetch_value( :release_build, :use_assembly, hash:config, default:false )
+    test_assembly, _ = @config_walkinator.fetch_value( :test_build, :use_assembly, hash:config, default:false)
+
     default_config.deep_merge( DEFAULT_TOOLS_TEST.deep_clone() )
 
     default_config.deep_merge( DEFAULT_TOOLS_TEST_PREPROCESSORS.deep_clone() ) if (config[:project][:use_test_preprocessor] != :none)
-    default_config.deep_merge( DEFAULT_TOOLS_TEST_ASSEMBLER.deep_clone() )     if (config[:test_build][:use_assembly])
+    default_config.deep_merge( DEFAULT_TOOLS_TEST_ASSEMBLER.deep_clone() )     if test_assembly
 
     default_config.deep_merge( DEFAULT_TOOLS_RELEASE.deep_clone() )            if (config[:project][:release_build])
-    default_config.deep_merge( DEFAULT_TOOLS_RELEASE_ASSEMBLER.deep_clone() )  if (config[:project][:release_build] and config[:release_build][:use_assembly])
+    default_config.deep_merge( DEFAULT_TOOLS_RELEASE_ASSEMBLER.deep_clone() )  if (config[:project][:release_build] and release_assembly)
   end
 
 
   def populate_cmock_defaults(config, default_config)
     # Cmock has its own internal defaults handling, but we need to set these specific values
-    # so they're present for the build environment to access;
-    # Note: these need to end up in the hash given to initialize cmock for this to be successful
+    # so they're guaranteed values and present for the Ceedling environment to access
 
     msg = @reportinator.generate_progress( 'Collecting CMock defaults' )
     @loginator.log( msg, Verbosity::OBNOXIOUS )
 
-    # Populate defaults with CMock internal settings
-    default_cmock = default_config[:cmock] || {}
-
-    # Yes, we're duplicating the defaults in CMock, but it's because:
-    #  (A) We always need CMOCK_MOCK_PREFIX in Ceedling's environment
-    #  (B) Test runner generator uses these same configuration values
-    default_cmock[:mock_prefix] = 'Mock' if (default_cmock[:mock_prefix].nil?)
-    default_cmock[:mock_suffix] = ''     if (default_cmock[:mock_suffix].nil?)
-
-    # Just because strict ordering is the way to go
-    default_cmock[:enforce_strict_ordering] = true                                                  if (default_cmock[:enforce_strict_ordering].nil?)
+    # Begin populating defaults with CMock defaults as set by Ceedling
+    default_cmock = default_config[:cmock]
 
-    default_cmock[:mock_path] = File.join(config[:project][:build_root], TESTS_BASE_PATH, 'mocks')  if (default_cmock[:mock_path].nil?)
-
-    default_cmock[:verbosity] = project_verbosity()                                                 if (default_cmock[:verbosity].nil?)
+    # Fill in default settings programmatically
+    default_cmock[:mock_path] = File.join(config[:project][:build_root], TESTS_BASE_PATH, 'mocks')
+    default_cmock[:verbosity] = project_verbosity()
   end
 
 
@@ -210,9 +204,11 @@ def populate_cmock_config(config)
     cmock[:plugins].map! { |plugin| plugin.to_sym() }
     cmock[:plugins].uniq!
 
-    # CMock Unity helper and includes safe defaults
+    # CMock includes safe defaults
     cmock[:includes] = [] if (cmock[:includes].nil?)
-    cmock[:unity_helper_path] = [] if (cmock[:unity_helper_path].nil?)
+
+    # Reformulate CMock helper path value as array of one element if it's a string in config
+    cmock[:unity_helper_path] = [] if cmock[:unity_helper_path].nil?
     cmock[:unity_helper_path] = [cmock[:unity_helper_path]] if cmock[:unity_helper_path].is_a?( String )
 
     # CMock Unity helper handling

lib/ceedling/configurator_validator.rb

@@ -17,8 +17,8 @@ class ConfiguratorValidator
 
   # Walk into config hash verify existence of data at key depth
   def exists?(config, *keys)
-    hash  = @config_walkinator.fetch_value( config, *keys )
-    exist = !hash[:value].nil?
+    hash, _  = @config_walkinator.fetch_value( *keys, hash:config )
+    exist = !hash.nil?
 
     if (not exist)
       walk = @reportinator.generate_config_walk( keys )
@@ -32,8 +32,7 @@ def exists?(config, *keys)
   # Paths are either full simple paths or a simple portion of a path up to a glob.
   def validate_path_list(config, *keys)
     exist = true
-    hash = @config_walkinator.fetch_value( config, *keys )
-    list = hash[:value]
+    list, depth = @config_walkinator.fetch_value( *keys, hash:config )
 
     # Return early if we couldn't walk into hash and find a value
     return false if (list.nil?)
@@ -46,7 +45,7 @@ def validate_path_list(config, *keys)
 
       # If (partial) path does not exist, complain
       if (not @file_wrapper.exist?( _path ))
-        walk = @reportinator.generate_config_walk( keys, hash[:depth] )
+        walk = @reportinator.generate_config_walk( keys, depth )
         @loginator.log( "Config path #{walk} => '#{_path}' does not exist in the filesystem.", Verbosity::ERRORS ) 
         exist = false
       end 
@@ -62,8 +61,7 @@ def validate_paths_entries(config, key)
     keys = [:paths, key]
     walk = @reportinator.generate_config_walk( keys )
 
-    hash = @config_walkinator.fetch_value( config, *keys )
-    list = hash[:value]
+    list, _ = @config_walkinator.fetch_value( *keys, hash:config )
 
     # Return early if we couldn't walk into hash and find a value
     return false if (list.nil?)
@@ -114,8 +112,7 @@ def validate_files_entries(config, key)
     keys = [:files, key]
     walk = @reportinator.generate_config_walk( keys )
 
-    hash = @config_walkinator.fetch_value( config, *keys )
-    list = hash[:value]
+    list, _ = @config_walkinator.fetch_value( *keys, hash:config )
 
     # Return early if we couldn't walk into hash and find a value
     return false if (list.nil?)
@@ -162,10 +159,10 @@ def validate_filepath_simple(path, *keys)
   def validate_tool(config:, key:, respect_optional:true)
     # Get tool
     walk = [:tools, key]
-    hash = @config_walkinator.fetch_value( config, *walk )
+    tool, _ = @config_walkinator.fetch_value( *walk, hash:config )
 
     arg_hash = {
-      tool: hash[:value],
+      tool: tool,
       name: @reportinator.generate_config_walk( walk ),
       extension: config[:extension][:executable],
       respect_optional: respect_optional

lib/ceedling/objects.yml

@@ -81,6 +81,7 @@ configurator:
     - configurator_setup
     - configurator_plugins
     - configurator_builder
+    - config_walkinator
     - yaml_wrapper
     - system_wrapper
     - loginator

plugins/command_hooks/lib/command_hooks.rb

@@ -142,16 +142,12 @@ def validate_config(config)
   def validate_hook(config, *keys)
     walk = [COMMAND_HOOKS_SYM, *keys]
     name = @reportinator.generate_config_walk( walk )
-    hash = @walkinator.fetch_value( config, *walk )
-
-    tool_exists = @configurator_validator.exists?( config, *walk )
-
-    unless tool_exists
+    entry, _ = @walkinator.fetch_value( *walk, hash:config )
+
+    if entry.nil?
       raise CeedlingException.new( "Missing Command Hook plugin configuration for #{name}" )
     end
-
-    entry = hash[:value]
-
+
     unless entry.is_a?(Hash)
       error = "Expected configuration #{name} for Command Hooks plugin to be a Hash but found #{entry.class}"
       raise CeedlingException.new( error )

plugins/report_tests_log_factory/lib/tests_reporter.rb

@@ -56,16 +56,15 @@ def footer(results:, stream:)
   private
 
   def update_filename(default_filename)
-    filename = fetch_config_value(:filename)
-
-    # Otherwise, use default filename
-    return filename.nil? ? default_filename : filename
+    # Fetch configured filename if it exists, otherwise return default filename
+    filename, _ = @config_walkinator.fetch_value( *keys, hash:@config, default:default_filename )
+    return filename
   end
 
+  # Handy convenience method for subclasses
   def fetch_config_value(*keys)
-    result = @config_walkinator.fetch_value( @config, *keys )
-    return result[:value] if !result[:value].nil?
-    return nil
+    result, _ = @config_walkinator.fetch_value( *keys, hash:@config )
+    return result
   end
 
 end

spec/config_walkinator_spec.rb

@@ -0,0 +1,62 @@
+# =========================================================================
+#   Ceedling - Test-Centered Build System for C
+#   ThrowTheSwitch.org
+#   Copyright (c) 2010-24 Mike Karlesky, Mark VanderVoord, & Greg Williams
+#   SPDX-License-Identifier: MIT
+# =========================================================================
+
+require 'spec_helper'
+require 'ceedling/config_walkinator'
+
+describe ConfigWalkinator do
+  before(:each) do
+    @cw = described_class.new
+  end
+
+  describe '#fetch_value' do
+
+    it 'fetches a boolean 1 levels in' do
+      config = {:foo => false}
+      expect(@cw.fetch_value(:foo, hash:config)).to eq([false, 1])
+
+      config = {:foo => true}
+      expect(@cw.fetch_value(:foo, hash:config)).to eq([true, 1])
+    end
+
+    it 'fetches a list two levels in' do
+      config = {:foo => {:bar => [1, 2, 3]}}
+
+      expect(@cw.fetch_value(:foo, :bar, hash:config)).to eq([[1,2,3], 2])
+    end
+
+    it 'fetches a hash 3 levels in' do
+      config = {:foo => {:bar => {:baz => {:setting => 5}}}}
+
+      expect(@cw.fetch_value(:foo, :bar, :baz, hash:config)).to eq([{:setting => 5}, 3])
+    end
+
+    it 'fetches nothing for nil config hash' do
+      expect(@cw.fetch_value(:foo, :bar, :baz, hash:nil)).to eq([nil, 0])
+    end
+
+    it 'fetches nothing for a level deeper than exists' do
+      config = {:foo => {:bar => 'a'}}
+
+      expect(@cw.fetch_value(:foo, :bar, :oops, hash:config)).to eq([nil, 2])
+    end
+
+    it 'fetches nothing if non-symbol provided as key' do
+      config = {:foo => {:bar => 'a'}}
+
+      expect(@cw.fetch_value(:foo, :bar, 'a', hash:config)).to eq([nil, 2])
+    end
+
+    it 'fetches the provided default value if a key does not exist' do
+      config = {:foo => {:bar => {:baz => true}}}
+
+      expect(@cw.fetch_value(:foo, :bar, :oops, hash:config, default:false)).to eq([false, 2])
+    end
+
+  end
+
+end