From 6ef83e6514019935382406a476963f7860d98029 Mon Sep 17 00:00:00 2001 From: Mike Karlesky Date: Fri, 6 Oct 2023 13:37:50 -0400 Subject: [PATCH] Fixed stale cmock configuration bug It was possible for mocks to be generated with an out-of-date cmock configuration while other functionality used the correct CMock configuration. Also cleaned up code structure and comments around mock and test runner generation. --- lib/ceedling/cmock_builder.rb | 19 ----------- lib/ceedling/configurator.rb | 21 ++++++++---- lib/ceedling/generator.rb | 34 +++++++------------ lib/ceedling/generator_mocks.rb | 31 +++++++++++++++++ lib/ceedling/generator_test_runner.rb | 6 +++- lib/ceedling/objects.yml | 9 ++--- lib/ceedling/tasks_base.rake | 9 ----- spec/build_invoker_utils_spec.rb | 2 +- ...erator_test_results_sanity_checker_spec.rb | 2 +- spec/generator_test_results_spec.rb | 2 +- 10 files changed, 71 insertions(+), 64 deletions(-) delete mode 100644 lib/ceedling/cmock_builder.rb create mode 100644 lib/ceedling/generator_mocks.rb diff --git a/lib/ceedling/cmock_builder.rb b/lib/ceedling/cmock_builder.rb deleted file mode 100644 index 44b410da..00000000 --- a/lib/ceedling/cmock_builder.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'cmock' - -class CmockBuilder - - attr_writer :default_config - - def setup - @default_config = nil - end - - def get_default_config - return @default_config.clone - end - - def manufacture(config) - return CMock.new(config) - end - -end diff --git a/lib/ceedling/configurator.rb b/lib/ceedling/configurator.rb index 837882df..d64a8069 100644 --- a/lib/ceedling/configurator.rb +++ b/lib/ceedling/configurator.rb @@ -10,7 +10,7 @@ class Configurator attr_reader :project_config_hash, :script_plugins, :rake_plugins attr_accessor :project_logging, :project_debug, :project_verbosity, :sanity_checks - constructor(:configurator_setup, :configurator_builder, :configurator_plugins, :cmock_builder, :yaml_wrapper, :system_wrapper) do + constructor(:configurator_setup, :configurator_builder, :configurator_plugins, :yaml_wrapper, :system_wrapper) do @project_logging = false @project_debug = false @project_verbosity = Verbosity::NORMAL @@ -18,8 +18,11 @@ class Configurator end def setup - # special copy of cmock config to provide to cmock for construction - @cmock_config_hash = {} + # Cmock config reference to provide to CMock for mock generation + @cmock_config = {} # Default empty hash, replaced by reference below + + # Runner config reference to provide to runner generation + @runner_config = {} # Default empty hash, replaced by reference below # note: project_config_hash is an instance variable so constants and accessors created # in eval() statements in build() have something of proper scope and persistence to reference @@ -84,7 +87,7 @@ def populate_defaults(config) def populate_unity_defaults(config) unity = config[:unity] || {} - @runner_config = unity.merge(@runner_config || config[:test_runner] || {}) + @runner_config = unity.merge(config[:test_runner] || {}) end def populate_cmock_defaults(config) @@ -100,6 +103,7 @@ def populate_cmock_defaults(config) cmock[:enforce_strict_ordering] = true if (cmock[:enforce_strict_ordering].nil?) cmock[:mock_path] = File.join(config[:project][:build_root], TESTS_BASE_PATH, 'mocks') if (cmock[:mock_path].nil?) + cmock[:verbosity] = @project_verbosity if (cmock[:verbosity].nil?) cmock[:plugins] = [] if (cmock[:plugins].nil?) @@ -116,7 +120,7 @@ def populate_cmock_defaults(config) @runner_config = cmock.merge(@runner_config || config[:test_runner] || {}) - @cmock_builder.default_config = cmock + @cmock_config = cmock end @@ -130,7 +134,12 @@ def copy_vendor_defines(config) def get_runner_config - @runner_config + return @runner_config.clone + end + + + def get_cmock_config + return @cmock_config.clone end diff --git a/lib/ceedling/generator.rb b/lib/ceedling/generator.rb index 85fc2862..877d2c2b 100644 --- a/lib/ceedling/generator.rb +++ b/lib/ceedling/generator.rb @@ -1,14 +1,12 @@ require 'ceedling/constants' require 'ceedling/file_path_utils' -# Pull in Unity's Test Runner Generator -require 'generate_test_runner.rb' class Generator constructor :configurator, :generator_helper, :preprocessinator, - :cmock_builder, + :generator_mocks, :generator_test_runner, :generator_test_results, :test_context_extractor, @@ -33,25 +31,16 @@ def generate_mock(context:, mock:, test:, input_filepath:, output_path:) @plugin_manager.pre_mock_generate( arg_hash ) begin - # TODO: Add option to CMock to generate mock to any destination path - # Below is a hack that insantiates CMock anew for each desired output path + # Below is a workaround that nsantiates CMock anew: + # 1. To allow dfferent output path per mock + # 2. To avoid any thread safety complications + + # TODO: + # - Add option to CMock to generate mock to any destination path + # - Make CMock thread-safe # Get default config created by Ceedling and customize it - config = @cmock_builder.get_default_config - config[:mock_path] = output_path - - # Verbosity management for logging messages - case @configurator.project_verbosity - when Verbosity::SILENT - config[:verbosity] = 0 # CMock is silent - when Verbosity::ERRORS - when Verbosity::COMPLAIN - when Verbosity::NORMAL - when Verbosity::OBNOXIOUS - config[:verbosity] = 1 # Errors and warnings only so we can customize generation message ourselves - else # DEBUG - config[:verbosity] = 3 # Max verbosity - end + config = @generator_mocks.build_configuration( output_path ) # Generate mock msg = @reportinator.generate_module_progress( @@ -61,7 +50,8 @@ def generate_mock(context:, mock:, test:, input_filepath:, output_path:) ) @streaminator.stdout_puts(msg, Verbosity::NORMAL) - @cmock_builder.manufacture(config).setup_mocks( arg_hash[:header_file] ) + cmock = @generator_mocks.manufacture( config ) + cmock.setup_mocks( arg_hash[:header_file] ) rescue raise ensure @@ -81,7 +71,7 @@ def generate_test_runner(context:, mock_list:, test_filepath:, input_filepath:, # Instantiate the test runner generator each time needed for thread safety # TODO: Make UnityTestRunnerGenerator thread-safe - generator = UnityTestRunnerGenerator.new( @configurator.get_runner_config ) + generator = @generator_test_runner.manufacture() # collect info we need module_name = File.basename( arg_hash[:test_file] ) diff --git a/lib/ceedling/generator_mocks.rb b/lib/ceedling/generator_mocks.rb new file mode 100644 index 00000000..b4025b50 --- /dev/null +++ b/lib/ceedling/generator_mocks.rb @@ -0,0 +1,31 @@ +require 'cmock' + +class GeneratorMocks + + constructor :configurator + + def manufacture(config) + return CMock.new(config) + end + + def build_configuration( output_path ) + config = @configurator.get_cmock_config + config[:mock_path] = output_path + + # Verbosity management for logging messages + case @configurator.project_verbosity + when Verbosity::SILENT + config[:verbosity] = 0 # CMock is silent + when Verbosity::ERRORS + when Verbosity::COMPLAIN + when Verbosity::NORMAL + when Verbosity::OBNOXIOUS + config[:verbosity] = 1 # Errors and warnings only so we can customize generation message ourselves + else # DEBUG + config[:verbosity] = 3 # Max verbosity + end + + return config + end + +end diff --git a/lib/ceedling/generator_test_runner.rb b/lib/ceedling/generator_test_runner.rb index 52b89d6c..0c84e2f2 100644 --- a/lib/ceedling/generator_test_runner.rb +++ b/lib/ceedling/generator_test_runner.rb @@ -1,9 +1,13 @@ - +require 'generate_test_runner.rb' # Unity's test runner generator class GeneratorTestRunner constructor :configurator, :file_path_utils, :file_wrapper + def manufacture() + return UnityTestRunnerGenerator.new( @configurator.get_runner_config ) + end + def find_test_cases(generator:, test_filepath:, input_filepath:) if (@configurator.project_use_test_preprocessor) diff --git a/lib/ceedling/objects.yml b/lib/ceedling/objects.yml index d76337db..07e236a7 100644 --- a/lib/ceedling/objects.yml +++ b/lib/ceedling/objects.yml @@ -11,8 +11,6 @@ yaml_wrapper: system_wrapper: -cmock_builder: - reportinator: rake_utils: @@ -86,7 +84,6 @@ configurator: - configurator_setup - configurator_plugins - configurator_builder - - cmock_builder - yaml_wrapper - system_wrapper @@ -215,7 +212,7 @@ generator: - configurator - generator_helper - preprocessinator - - cmock_builder + - generator_mocks - generator_test_runner - generator_test_results - test_context_extractor @@ -245,6 +242,10 @@ generator_test_results_sanity_checker: - configurator - streaminator +generator_mocks: + compose: + - configurator + generator_test_runner: compose: - configurator diff --git a/lib/ceedling/tasks_base.rake b/lib/ceedling/tasks_base.rake index a35cde75..ad1d7c20 100644 --- a/lib/ceedling/tasks_base.rake +++ b/lib/ceedling/tasks_base.rake @@ -14,15 +14,6 @@ desc "Set verbose output (silent:[#{Verbosity::SILENT}] - obnoxious:[#{Verbosity task :verbosity, :level do |t, args| verbosity_level = args.level.to_i - if (PROJECT_USE_MOCKS) - # don't store verbosity level in setupinator's config hash, use a copy; - # otherwise, the input configuration will change and trigger entire project rebuilds - hash = @ceedling[:setupinator].config_hash[:cmock].clone - hash[:verbosity] = verbosity_level - - @ceedling[:cmock_builder].manufacture( hash ) - end - @ceedling[:configurator].project_verbosity = verbosity_level # control rake's verbosity with new setting diff --git a/spec/build_invoker_utils_spec.rb b/spec/build_invoker_utils_spec.rb index bd01c3c6..8a0f8b84 100644 --- a/spec/build_invoker_utils_spec.rb +++ b/spec/build_invoker_utils_spec.rb @@ -8,7 +8,7 @@ before(:each) do # this will always be mocked @configurator = Configurator.new({:configurator_setup => nil, :configurator_builder => nil, - :configurator_plugins => nil, :cmock_builder => nil, + :configurator_plugins => nil, :yaml_wrapper => nil, :system_wrapper => nil}) @streaminator = Streaminator.new({:streaminator_helper => nil, :verbosinator => nil, :loginator => nil, :stream_wrapper => nil}) diff --git a/spec/generator_test_results_sanity_checker_spec.rb b/spec/generator_test_results_sanity_checker_spec.rb index 2a09646d..f72360a5 100644 --- a/spec/generator_test_results_sanity_checker_spec.rb +++ b/spec/generator_test_results_sanity_checker_spec.rb @@ -7,7 +7,7 @@ describe GeneratorTestResultsSanityChecker do before(:each) do # this will always be mocked - @configurator = Configurator.new({:configurator_setup => nil, :configurator_builder => nil, :configurator_plugins => nil, :cmock_builder => nil, :yaml_wrapper => nil, :system_wrapper => nil}) + @configurator = Configurator.new({:configurator_setup => nil, :configurator_builder => nil, :configurator_plugins => nil, :yaml_wrapper => nil, :system_wrapper => nil}) @streaminator = Streaminator.new({:streaminator_helper => nil, :verbosinator => nil, :loginator => nil, :stream_wrapper => nil}) @sanity_checker = described_class.new({:configurator => @configurator, :streaminator => @streaminator}) diff --git a/spec/generator_test_results_spec.rb b/spec/generator_test_results_spec.rb index b5d35f06..1ca236fd 100644 --- a/spec/generator_test_results_spec.rb +++ b/spec/generator_test_results_spec.rb @@ -56,7 +56,7 @@ describe GeneratorTestResults do before(:each) do # these will always be mocked - @configurator = Configurator.new({:configurator_setup => nil, :configurator_builder => nil, :configurator_plugins => nil, :cmock_builder => nil, :yaml_wrapper => nil, :system_wrapper => nil}) + @configurator = Configurator.new({:configurator_setup => nil, :configurator_builder => nil, :configurator_plugins => nil, :yaml_wrapper => nil, :system_wrapper => nil}) @streaminator = Streaminator.new({:streaminator_helper => nil, :verbosinator => nil, :loginator => nil, :stream_wrapper => nil}) # these will always be used as is.