From 72bb3ff1e4f7b1e14aa26d49eb200de8442d716c Mon Sep 17 00:00:00 2001 From: James Raphael Tiovalen Date: Mon, 31 Mar 2025 18:24:01 +0800 Subject: [PATCH] Fix Valgrind plugin and add tests This commit fixes the Valgrind plugin by removing some deprecated functions and updating some function signatures. This commit also adds tests to serve as regression testing to prevent the Valgrind plugin from being broken by future refactoring changes. Signed-off-by: James Raphael Tiovalen --- assets/project_as_gem.yml | 1 + examples/temp_sensor/mixin/add_valgrind.yml | 13 +++ examples/temp_sensor/project.yml | 1 + plugins/valgrind/config/defaults_valgrind.rb | 1 - plugins/valgrind/valgrind.rake | 86 +++++++++++---- spec/valgrind/valgrind_deployment_spec.rb | 108 +++++++++++++++++++ spec/valgrind/valgrind_test_cases_spec.rb | 98 +++++++++++++++++ 7 files changed, 287 insertions(+), 21 deletions(-) create mode 100644 examples/temp_sensor/mixin/add_valgrind.yml create mode 100644 spec/valgrind/valgrind_deployment_spec.rb create mode 100644 spec/valgrind/valgrind_test_cases_spec.rb diff --git a/assets/project_as_gem.yml b/assets/project_as_gem.yml index b5f276e11..bd763b21c 100644 --- a/assets/project_as_gem.yml +++ b/assets/project_as_gem.yml @@ -63,6 +63,7 @@ #- dependencies # automatically fetch 3rd party libraries, etc. #- subprojects # managing builds and test for static libraries #- fake_function_framework # use FFF instead of CMock + #- valgrind # detect memory management and threading bugs using valgrind. Requires valgrind for your platform # Report options (You'll want to choose one stdout option, but may choose multiple stored options if desired) #- report_build_warnings_log diff --git a/examples/temp_sensor/mixin/add_valgrind.yml b/examples/temp_sensor/mixin/add_valgrind.yml new file mode 100644 index 000000000..8c3d39f10 --- /dev/null +++ b/examples/temp_sensor/mixin/add_valgrind.yml @@ -0,0 +1,13 @@ +# ========================================================================= +# Ceedling - Test-Centered Build System for C +# ThrowTheSwitch.org +# Copyright (c) 2010-25 Mike Karlesky, Mark VanderVoord, & Greg Williams +# SPDX-License-Identifier: MIT +# ========================================================================= + +--- + +# Enable valgrind plugin +:plugins: + :enabled: + - valgrind diff --git a/examples/temp_sensor/project.yml b/examples/temp_sensor/project.yml index 027fb5520..1fdc25055 100644 --- a/examples/temp_sensor/project.yml +++ b/examples/temp_sensor/project.yml @@ -65,6 +65,7 @@ #- dependencies # automatically fetch 3rd party libraries, etc. #- subprojects # managing builds and test for static libraries #- fake_function_framework # use FFF instead of CMock + #- valgrind # detect memory management and threading bugs using valgrind. Requires valgrind for your platform # Report options (You'll want to choose one stdout option, but may choose multiple stored options if desired) #- report_build_warnings_log diff --git a/plugins/valgrind/config/defaults_valgrind.rb b/plugins/valgrind/config/defaults_valgrind.rb index 654914e32..0603034af 100644 --- a/plugins/valgrind/config/defaults_valgrind.rb +++ b/plugins/valgrind/config/defaults_valgrind.rb @@ -3,7 +3,6 @@ :executable => ENV['VALGRIND'].nil? ? FilePathUtils.os_executable_ext('valgrind').freeze : ENV['VALGRIND'].split[0], :name => 'default_valgrind'.freeze, :stderr_redirect => StdErrRedirect::NONE.freeze, - :background_exec => BackgroundExec::NONE.freeze, :optional => false.freeze, :arguments => [ "--leak-check=full".freeze, diff --git a/plugins/valgrind/valgrind.rake b/plugins/valgrind/valgrind.rake index 3ba0239f8..9820daebc 100644 --- a/plugins/valgrind/valgrind.rake +++ b/plugins/valgrind/valgrind.rake @@ -7,18 +7,16 @@ CLOBBER.include(File.join(VALGRIND_BUILD_PATH, '**/*')) task directories: [VALGRIND_BUILD_OUTPUT_PATH] namespace VALGRIND_SYM do - task source_coverage: COLLECTION_ALL_SOURCE.pathmap("#{VALGRIND_BUILD_OUTPUT_PATH}/%n#{@ceedling[:configurator].extension_object}") - desc 'Run Valgrind for all tests' - task all: [:test_deps] do + task all: [:prepare] do @ceedling[:configurator].replace_flattened_config(@ceedling[VALGRIND_SYM].config) COLLECTION_ALL_TESTS.each do |test| - executable = @ceedling[:file_path_utils].form_test_executable_filepath(test) + executable = @ceedling[:file_path_utils].form_test_executable_filepath(File.join(VALGRIND_BUILD_OUTPUT_PATH, File.basename(test, @ceedling[:configurator].extension_source)), test) command = @ceedling[:tool_executor].build_command_line(TOOLS_VALGRIND, [], executable) - @ceedling[:streaminator].stdout_puts("\nINFO: #{command[:line]}\n\n") - @ceedling[:tool_executor].exec(command[:line], command[:options]) + @ceedling[:loginator].log("\nINFO: #{command[:line]}\n\n") + shell_result = @ceedling[:tool_executor].exec(command) + @ceedling[:loginator].log("#{shell_result[:output]}\n") end - @ceedling[:configurator].restore_config end desc 'Run Valgrind for a single test or executable ([*] real test or source file name, no path).' @@ -27,23 +25,71 @@ namespace VALGRIND_SYM do "Use a real test or source file name (no path) in place of the wildcard.\n" \ "Example: rake #{VALGRIND_ROOT_NAME}:foo.c\n\n" - @ceedling[:streaminator].stdout_puts(message) + @ceedling[:loginator].log( message, Verbosity::ERRORS ) end - # use a rule to increase efficiency for large projects - # valgrind test tasks by regex - rule(/^#{VALGRIND_TASK_ROOT}\S+$/ => [ + desc 'Run Valgrind for tests by matching regular expression pattern.' + task :pattern, [:regex] => [:prepare] do |_t, args| + matches = [] + + COLLECTION_ALL_TESTS.each do |test| + matches << test if test =~ /#{args.regex}/ + end + + if !matches.empty? + @ceedling[:configurator].replace_flattened_config(@ceedling[VALGRIND_SYM].config) + matches.each do |test| + executable = @ceedling[:file_path_utils].form_test_executable_filepath(File.join(VALGRIND_BUILD_OUTPUT_PATH, File.basename(test, @ceedling[:configurator].extension_source)), test) + command = @ceedling[:tool_executor].build_command_line(TOOLS_VALGRIND, [], executable) + @ceedling[:loginator].log("\nINFO: #{command[:line]}\n\n") + shell_result = @ceedling[:tool_executor].exec(command) + @ceedling[:loginator].log("#{shell_result[:output]}\n") + end + else + @ceedling[:loginator].log("\nFound no tests matching pattern /#{args.regex}/.") + end + end + + desc 'Run Valgrind for tests whose test path contains [dir] or [dir] substring.' + task :path, [:dir] => [:prepare] do |_t, args| + matches = [] + + COLLECTION_ALL_TESTS.each do |test| + matches << test if File.dirname(test).include?(args.dir.tr('\\', '/')) + end + + if !matches.empty? + @ceedling[:configurator].replace_flattened_config(@ceedling[VALGRIND_SYM].config) + matches.each do |test| + executable = @ceedling[:file_path_utils].form_test_executable_filepath(File.join(VALGRIND_BUILD_OUTPUT_PATH, File.basename(test, @ceedling[:configurator].extension_source)), test) + command = @ceedling[:tool_executor].build_command_line(TOOLS_VALGRIND, [], executable) + @ceedling[:loginator].log("\nINFO: #{command[:line]}\n\n") + shell_result = @ceedling[:tool_executor].exec(command) + @ceedling[:loginator].log("#{shell_result[:output]}\n") + end + else + @ceedling[:loginator].log( 'Found no tests including the given path or path component', Verbosity::ERRORS ) + end + end + + # Use a rule to increase efficiency for large projects + rule(/^#{VALGRIND_TASK_ROOT}\S+$/ => [ # valgrind test tasks by regex proc do |task_name| - test = task_name.sub(/#{VALGRIND_TASK_ROOT}/, '') - test = "#{PROJECT_TEST_FILE_PREFIX}#{test}" unless test.start_with?(PROJECT_TEST_FILE_PREFIX) - @ceedling[:file_finder].find_test_from_file_path(test) + # Yield clean test name => Strip the task string, remove Rake test task prefix, and remove any code file extension + test = task_name.strip().sub(/^#{VALGRIND_TASK_ROOT}/, '').chomp( EXTENSION_SOURCE ) + + # Ensure the test name begins with a test name prefix + test = PROJECT_TEST_FILE_PREFIX + test if not (test.start_with?( PROJECT_TEST_FILE_PREFIX )) + + # Provide the filepath for the target test task back to the Rake task + @ceedling[:file_finder].find_test_file_from_name( test ) end - ]) do test + ]) do |test| @ceedling[:configurator].replace_flattened_config(@ceedling[VALGRIND_SYM].config) - executable = @ceedling[:file_path_utils].form_test_executable_filepath(test.source) + executable = @ceedling[:file_path_utils].form_test_executable_filepath(File.join(VALGRIND_BUILD_OUTPUT_PATH, File.basename(test.source, @ceedling[:configurator].extension_source)), test.source) command = @ceedling[:tool_executor].build_command_line(TOOLS_VALGRIND, [], executable) - @ceedling[:streaminator].stdout_puts("\nINFO: #{command[:line]}\n\n") - @ceedling[:tool_executor].exec(command[:line], command[:options]) - @ceedling[:configurator].restore_config + @ceedling[:loginator].log("\nINFO: #{command[:line]}\n\n") + shell_result = @ceedling[:tool_executor].exec(command) + @ceedling[:loginator].log("#{shell_result[:output]}\n") end -end \ No newline at end of file +end diff --git a/spec/valgrind/valgrind_deployment_spec.rb b/spec/valgrind/valgrind_deployment_spec.rb new file mode 100644 index 000000000..638696720 --- /dev/null +++ b/spec/valgrind/valgrind_deployment_spec.rb @@ -0,0 +1,108 @@ +# ========================================================================= +# Ceedling - Test-Centered Build System for C +# ThrowTheSwitch.org +# Copyright (c) 2010-25 Mike Karlesky, Mark VanderVoord, & Greg Williams +# SPDX-License-Identifier: MIT +# ========================================================================= + +require 'spec_system_helper' +require 'valgrind/valgrind_test_cases_spec' + +describe "Ceedling" do + describe "Valgrind" do + include CeedlingTestCases + include ValgrindTestCases + before :all do + determine_reports_to_test + @c = SystemContext.new + @c.deploy_gem + end + + after :all do + @c.done! + end + + before { @proj_name = "fake_project" } + after { @c.with_context { FileUtils.rm_rf @proj_name } } + + describe "basic operations" do + before do + @c.with_context do + `bundle exec ruby -S ceedling new --local #{@proj_name} 2>&1` + end + end + + it { can_test_projects_with_valgrind_with_success } + it { can_test_projects_with_valgrind_with_fail } + it { can_test_projects_with_valgrind_with_compile_error } + it { can_fetch_project_help_for_valgrind } + end + + describe "command: `ceedling example temp_sensor`" do + describe "temp_sensor" do + before do + @c.with_context do + output = `bundle exec ruby -S ceedling example temp_sensor 2>&1` + expect(output).to match(/created/) + + Dir.chdir "temp_sensor" do + @output = `bundle exec ruby -S ceedling --mixin=add_valgrind test:all 2>&1` + expect(@output).to match(/TESTED:\s+51/) + expect(@output).to match(/PASSED:\s+51/) + end + end + end + + it "should be testable" do + @c.with_context do + Dir.chdir "temp_sensor" do + @output = `bundle exec ruby -S ceedling --mixin=add_valgrind valgrind:all 2>&1` + expect(@output).to match(/==\d+== All heap blocks were freed -- no leaks are possible/) + expect(@output).to match(/==\d+== ERROR SUMMARY: 0 errors from 0 contexts/) + end + end + end + + it "should be able to test a single module (it should INHERIT file-specific flags)" do + @c.with_context do + Dir.chdir "temp_sensor" do + @output = `bundle exec ruby -S ceedling --mixin=add_valgrind valgrind:TemperatureCalculator 2>&1` + expect(@output).to match(/==\d+== All heap blocks were freed -- no leaks are possible/) + expect(@output).to match(/==\d+== ERROR SUMMARY: 0 errors from 0 contexts/) + end + end + end + + it "should be able to test multiple files matching a pattern" do + @c.with_context do + Dir.chdir "temp_sensor" do + @output = `bundle exec ruby -S ceedling --mixin=add_valgrind valgrind:pattern[Temp] 2>&1` + expect(@output).to match(/==\d+== All heap blocks were freed -- no leaks are possible/) + expect(@output).to match(/==\d+== ERROR SUMMARY: 0 errors from 0 contexts/) + end + end + end + + it "should be able to test all files matching in a path" do + @c.with_context do + Dir.chdir "temp_sensor" do + @output = `bundle exec ruby -S ceedling --mixin=add_valgrind valgrind:path[adc] 2>&1` + expect(@output).to match(/==\d+== All heap blocks were freed -- no leaks are possible/) + expect(@output).to match(/==\d+== ERROR SUMMARY: 0 errors from 0 contexts/) + end + end + end + + it "should be able to test specific test cases in a file" do + @c.with_context do + Dir.chdir "temp_sensor" do + @output = `bundle exec ruby -S ceedling --mixin=add_valgrind valgrind:path[adc] --test-case="RunShouldNot" 2>&1` + expect(@output).to match(/==\d+== All heap blocks were freed -- no leaks are possible/) + expect(@output).to match(/==\d+== ERROR SUMMARY: 0 errors from 0 contexts/) + end + end + end + end + end + end +end diff --git a/spec/valgrind/valgrind_test_cases_spec.rb b/spec/valgrind/valgrind_test_cases_spec.rb new file mode 100644 index 000000000..95f6f78dd --- /dev/null +++ b/spec/valgrind/valgrind_test_cases_spec.rb @@ -0,0 +1,98 @@ +# ========================================================================= +# Ceedling - Test-Centered Build System for C +# ThrowTheSwitch.org +# Copyright (c) 2010-25 Mike Karlesky, Mark VanderVoord, & Greg Williams +# SPDX-License-Identifier: MIT +# ========================================================================= + +require 'fileutils' +require 'tmpdir' +require 'yaml' +require 'spec_system_helper' +require 'pp' + +module ValgrindTestCases + def determine_reports_to_test + @valgrind_reports = [] + + begin + `valgrind --version 2>&1` + @valgrind_reports << :valgrind if $?.exitstatus == 0 + rescue + puts "No Valgrind exec to test against" + end + end + + def prep_project_yml_for_valgrind + FileUtils.cp test_asset_path("project_as_gem.yml"), "project.yml" + @c.uncomment_project_yml_option_for_test("- valgrind") + end + + def can_test_projects_with_valgrind_with_success + @c.with_context do + Dir.chdir @proj_name do + prep_project_yml_for_valgrind + FileUtils.cp test_asset_path("example_file.h"), 'src/' + FileUtils.cp test_asset_path("example_file.c"), 'src/' + FileUtils.cp test_asset_path("test_example_file_success.c"), 'test/' + + output = `bundle exec ruby -S ceedling test:all 2>&1` + expect($?.exitstatus).to match(0) + + output = `bundle exec ruby -S ceedling valgrind:all 2>&1` + expect($?.exitstatus).to match(0) + expect(output).to match(/==\d+== All heap blocks were freed -- no leaks are possible/) + expect(output).to match(/==\d+== ERROR SUMMARY: 0 errors from 0 contexts/) + end + end + end + + def can_test_projects_with_valgrind_with_fail + @c.with_context do + Dir.chdir @proj_name do + prep_project_yml_for_valgrind + FileUtils.cp test_asset_path("example_file.h"), 'src/' + FileUtils.cp test_asset_path("example_file.c"), 'src/' + FileUtils.cp test_asset_path("test_example_file.c"), 'test/' + + output = `bundle exec ruby -S ceedling test:all 2>&1` + expect($?.exitstatus).to match(1) + + output = `bundle exec ruby -S ceedling valgrind:all 2>&1` + expect($?.exitstatus).to match(1) + expect(output).to match(/==\d+== All heap blocks were freed -- no leaks are possible/) + expect(output).to match(/==\d+== ERROR SUMMARY: 0 errors from 0 contexts/) + end + end + end + + def can_test_projects_with_valgrind_with_compile_error + @c.with_context do + Dir.chdir @proj_name do + prep_project_yml_for_valgrind + FileUtils.cp test_asset_path("example_file.h"), 'src/' + FileUtils.cp test_asset_path("example_file.c"), 'src/' + FileUtils.cp test_asset_path("test_example_file_boom.c"), 'test/' + + output = `bundle exec ruby -S ceedling test:all 2>&1` + expect($?.exitstatus).to match(1) + + output = `bundle exec ruby -S ceedling valgrind:all 2>&1` + expect($?.exitstatus).to match(1) + expect(output).to match(/(?:ERROR: Ceedling Failed)|(?:Ceedling could not complete operations because of errors)/) + end + end + end + + def can_fetch_project_help_for_valgrind + @c.with_context do + Dir.chdir @proj_name do + prep_project_yml_for_valgrind + output = `bundle exec ruby -S ceedling help 2>&1` + expect($?.exitstatus).to match(0) + expect(output).to match(/ceedling valgrind:\*/i) + expect(output).to match(/ceedling valgrind:all/i) + end + end + end +end