From 6c562a8e98914fddfd22f27aeb98a17f5da5b86d Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Thu, 5 Sep 2024 14:03:20 -0600 Subject: [PATCH 1/4] Fix spec return code --- openc3/spec/config/config_parser_spec.rb | 5 +++++ openc3/spec/spec_helper.rb | 1 + 2 files changed, 6 insertions(+) diff --git a/openc3/spec/config/config_parser_spec.rb b/openc3/spec/config/config_parser_spec.rb index 823d9c8d31..d86f8bc980 100644 --- a/openc3/spec/config/config_parser_spec.rb +++ b/openc3/spec/config/config_parser_spec.rb @@ -34,6 +34,11 @@ module OpenC3 ConfigParser.progress_callback = nil end + after(:each) do + ConfigParser.message_callback = nil + ConfigParser.progress_callback = nil + end + describe "parse_file", no_ext: true do it "yields keyword, parameters to the block" do tf = Tempfile.new('unittest') diff --git a/openc3/spec/spec_helper.rb b/openc3/spec/spec_helper.rb index 7052a63fe3..47e8343c9f 100644 --- a/openc3/spec/spec_helper.rb +++ b/openc3/spec/spec_helper.rb @@ -310,6 +310,7 @@ def kill_leftover_threads alias old_exit exit def exit(*args) $system_exit_count += 1 + super(*args) end RSpec.configure do |config| From ef87414e82f44f550cb8d07efe1fbaf569527c61 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Thu, 5 Sep 2024 15:30:28 -0600 Subject: [PATCH 2/4] Fix ruby tests --- openc3/lib/openc3/models/tool_config_model.rb | 2 +- .../spec/microservices/interface_microservice_spec.rb | 10 ++++++++-- openc3/spec/microservices/router_microservice_spec.rb | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/openc3/lib/openc3/models/tool_config_model.rb b/openc3/lib/openc3/models/tool_config_model.rb index 330b92bca0..abf73a5df0 100644 --- a/openc3/lib/openc3/models/tool_config_model.rb +++ b/openc3/lib/openc3/models/tool_config_model.rb @@ -25,7 +25,7 @@ module OpenC3 class ToolConfigModel def self.config_tool_names(scope: $openc3_scope) - cursor, keys = Store.scan(0, match: "#{scope}__config__*", type: 'hash', count: 100) + _, keys = Store.scan(0, match: "#{scope}__config__*", type: 'hash', count: 100) # Just return the tool name that is used in the other APIs return keys.map! { |key| key.split('__')[2] }.sort end diff --git a/openc3/spec/microservices/interface_microservice_spec.rb b/openc3/spec/microservices/interface_microservice_spec.rb index b52ac7e6b6..78ac597ef2 100644 --- a/openc3/spec/microservices/interface_microservice_spec.rb +++ b/openc3/spec/microservices/interface_microservice_spec.rb @@ -32,7 +32,7 @@ module OpenC3 describe InterfaceMicroservice do before(:each) do # This must be here in order to work when running more than this individual file - class TestInterface < Interface + class TestInterface < Interface # rubocop:disable Lint/ConstantDefinitionInBlock def initialize(hostname = "default", port = 12345) @hostname = hostname @port = port @@ -110,7 +110,7 @@ def read_interface CvtModel.set(json_hash, target_name: packet.target_name, packet_name: packet.packet_name, scope: "DEFAULT") end - class ApiTest + class ApiTest # rubocop:disable Lint/ConstantDefinitionInBlock include Extract include Api include Authorization @@ -266,8 +266,14 @@ class ApiTest interface = im.instance_variable_get(:@interface) interface.reconnect_delay = 0.1 # Override the reconnect delay to be quick + # Mock this because it calls exit which breaks SimpleCov + allow(OpenC3).to receive(:handle_fatal_exception) do |exception, _message| + expect(exception.message).to eql "test-error" + end + capture_io do |stdout| Thread.new { im.run } + sleep 0.1 # Allow to start and immediately crash expect(stdout.string).to include("RuntimeError") diff --git a/openc3/spec/microservices/router_microservice_spec.rb b/openc3/spec/microservices/router_microservice_spec.rb index 22eccb3224..48af84df9a 100644 --- a/openc3/spec/microservices/router_microservice_spec.rb +++ b/openc3/spec/microservices/router_microservice_spec.rb @@ -29,13 +29,13 @@ module OpenC3 describe RouterMicroservice do - class ApiTest + class ApiTest # rubocop:disable Lint/ConstantDefinitionInBlock include Extract include Api include Authorization end - class TestInterface < Interface + class TestRouter < Interface # rubocop:disable Lint/ConstantDefinitionInBlock def initialize super @connected = false @@ -82,7 +82,7 @@ def read_interface interface = double("Interface").as_null_object allow(interface).to receive(:connected?).and_return(true) allow(System).to receive(:targets).and_return({ "INST" => interface }) - model = RouterModel.new(name: "TEST_INT", scope: "DEFAULT", target_names: ["INST"], cmd_target_names: ["INST"], tlm_target_names: ["INST"], config_params: ["TestInterface"]) + model = RouterModel.new(name: "TEST_INT", scope: "DEFAULT", target_names: ["INST"], cmd_target_names: ["INST"], tlm_target_names: ["INST"], config_params: ["TestRouter"]) model.create model = MicroserviceModel.new(folder_name: "TEST", name: "DEFAULT__ROUTER__TEST_INT", scope: "DEFAULT", target_names: ["INST"]) model.create From aa29e3d6b52662c4563eecfc86d4de923a11b912 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Thu, 5 Sep 2024 18:01:11 -0600 Subject: [PATCH 3/4] Fix rubocop and a warning --- .rubocop.yml | 3 +++ openc3/lib/openc3/accessors/json_accessor.rb | 8 +++++--- openc3/spec/microservices/interface_microservice_spec.rb | 4 ++-- openc3/spec/microservices/router_microservice_spec.rb | 4 ++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index eea6d46f55..5ddd8a0d22 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -45,3 +45,6 @@ Lint/UnusedMethodArgument: # This rule doesn't allow for value.to_s(16) Lint/RedundantStringCoercion: Enabled: false +# This rule interferes with a lot of unit tests as well as json_accessor.rb +Lint/ConstantDefinitionInBlock: + Enabled: false diff --git a/openc3/lib/openc3/accessors/json_accessor.rb b/openc3/lib/openc3/accessors/json_accessor.rb index 2af1b0bc39..8c559baff7 100644 --- a/openc3/lib/openc3/accessors/json_accessor.rb +++ b/openc3/lib/openc3/accessors/json_accessor.rb @@ -22,9 +22,11 @@ require 'openc3/accessors/accessor' # Monkey patch JsonPath to enable create_additions and allow_nan to support binary strings, and NaN, Infinity, -Infinity -class JsonPath - def self.process_object(obj_or_str, opts = {}) - obj_or_str.is_a?(String) ? MultiJson.decode(obj_or_str, max_nesting: opts[:max_nesting], create_additions: true, allow_nan: true) : obj_or_str +OpenC3.disable_warnings do + class JsonPath + def self.process_object(obj_or_str, opts = {}) + obj_or_str.is_a?(String) ? MultiJson.decode(obj_or_str, max_nesting: opts[:max_nesting], create_additions: true, allow_nan: true) : obj_or_str + end end end diff --git a/openc3/spec/microservices/interface_microservice_spec.rb b/openc3/spec/microservices/interface_microservice_spec.rb index 7edf27c957..c3b8a7c477 100644 --- a/openc3/spec/microservices/interface_microservice_spec.rb +++ b/openc3/spec/microservices/interface_microservice_spec.rb @@ -32,7 +32,7 @@ module OpenC3 describe InterfaceMicroservice do before(:each) do # This must be here in order to work when running more than this individual file - class TestInterface < Interface # rubocop:disable Lint/ConstantDefinitionInBlock + class TestInterface < Interface def initialize(hostname = "default", port = 12345) @hostname = hostname @port = port @@ -115,7 +115,7 @@ def write_interface(data, extra = nil) CvtModel.set(json_hash, target_name: packet.target_name, packet_name: packet.packet_name, scope: "DEFAULT") end - class ApiTest # rubocop:disable Lint/ConstantDefinitionInBlock + class ApiTest include Extract include Api include Authorization diff --git a/openc3/spec/microservices/router_microservice_spec.rb b/openc3/spec/microservices/router_microservice_spec.rb index 48af84df9a..519583c71a 100644 --- a/openc3/spec/microservices/router_microservice_spec.rb +++ b/openc3/spec/microservices/router_microservice_spec.rb @@ -29,13 +29,13 @@ module OpenC3 describe RouterMicroservice do - class ApiTest # rubocop:disable Lint/ConstantDefinitionInBlock + class ApiTest include Extract include Api include Authorization end - class TestRouter < Interface # rubocop:disable Lint/ConstantDefinitionInBlock + class TestRouter < Interface def initialize super @connected = false From 8bebed5d139614abb181c3ce80888d814ec53f17 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Wed, 25 Sep 2024 22:04:14 -0600 Subject: [PATCH 4/4] Fix top_level_spec --- openc3/spec/spec_helper.rb | 16 --------------- openc3/spec/top_level/top_level_spec.rb | 26 +++++++++---------------- 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/openc3/spec/spec_helper.rb b/openc3/spec/spec_helper.rb index 47e8343c9f..fb8b90389c 100644 --- a/openc3/spec/spec_helper.rb +++ b/openc3/spec/spec_helper.rb @@ -305,14 +305,6 @@ def kill_leftover_threads end end -$system_exit_count = 0 -# Overload exit so we know when it is called -alias old_exit exit -def exit(*args) - $system_exit_count += 1 - super(*args) -end - RSpec.configure do |config| # Enforce the new expect() syntax instead of the old should syntax config.expect_with :rspec do |c| @@ -326,14 +318,6 @@ def exit(*args) $saved_stdout_const = Object.const_get(:STDOUT) end - config.after(:all) do - OpenC3.disable_warnings do - def Object.exit(*args) - old_exit(*args) - end - end - end - # Before each test make sure $stdout and STDOUT are set. They might be messed # up if a spec fails in the middle of capture_io and we don't have a chance # to return and reset them. diff --git a/openc3/spec/top_level/top_level_spec.rb b/openc3/spec/top_level/top_level_spec.rb index 3d6dded4a3..288f1d315d 100644 --- a/openc3/spec/top_level/top_level_spec.rb +++ b/openc3/spec/top_level/top_level_spec.rb @@ -81,7 +81,7 @@ def self.cleanup_exceptions describe "self.marshal_dump, self.marshal_load" do it "dumps and load a Ruby object" do - capture_io do |stdout| + capture_io do |_stdout| array = [1, 2, 3, 4] OpenC3.marshal_dump('marshal_test', array) array_load = OpenC3.marshal_load('marshal_test') @@ -93,9 +93,7 @@ def self.cleanup_exceptions it "rescues marshal dump errors" do capture_io do |stdout| - system_exit_count = $system_exit_count - OpenC3.marshal_dump('marshal_test', Proc.new { '' }) - expect($system_exit_count).to be > system_exit_count + expect { OpenC3.marshal_dump('marshal_test', Proc.new { '' }) }.to raise_error(SystemExit) expect(stdout.string).to match("is defined for class Proc") end OpenC3.cleanup_exceptions() @@ -103,12 +101,10 @@ def self.cleanup_exceptions it "rescues marshal dump errors in a Packet with a Mutex" do capture_io do |stdout| - system_exit_count = $system_exit_count pkt = Packet.new("TGT", "PKT") pkt.append_item("ITEM", 16, :UINT) pkt.read_all - OpenC3.marshal_dump('marshal_test', pkt) - expect($system_exit_count).to be > system_exit_count + expect { OpenC3.marshal_dump('marshal_test', pkt) }.to raise_error(SystemExit) expect(stdout.string).to match("Mutex exists in a packet") end OpenC3.cleanup_exceptions() @@ -139,7 +135,7 @@ def self.cleanup_exceptions describe "run_process" do it "returns a Thread" do if Kernel.is_windows? - capture_io do |stdout| + capture_io do |_stdout| thread = OpenC3.run_process("ping 127.0.0.1 -n 2 -w 1000 > nul") sleep 0.1 expect(thread).to be_a Thread @@ -164,10 +160,10 @@ def self.cleanup_exceptions end describe "hash_files" do - xit "calculates a hashing sum across files in md5 mode" do + it "calculates a hashing sum across files in md5 mode" do File.open(File.join(OpenC3::PATH, 'test1.txt'), 'w') { |f| f.puts "test1" } File.open(File.join(OpenC3::PATH, 'test2.txt'), 'w') { |f| f.puts "test2" } - digest = OpenC3.hash_files(["test1.txt", "test2.txt"]) + digest = OpenC3.hash_files(["test1.txt", "test2.txt"], nil, 'MD5') expect(digest.digest.length).to be 16 expect(digest.hexdigest).to eql 'e51dfbea83de9c7e6b49560089d8a170' File.delete(File.join(OpenC3::PATH, 'test1.txt')) @@ -207,7 +203,7 @@ def self.cleanup_exceptions # Move the defaults output dir out of the way for this test begin FileUtils.mv('outputs', 'outputs_bak') - rescue => err + rescue => e Dir.entries('outputs/logs').each do |entry| next if entry[0] == '.' @@ -217,7 +213,7 @@ def self.cleanup_exceptions STDOUT.puts entry end end - raise err + raise e end # Create a logs directory as the first order backup @@ -251,9 +247,7 @@ def self.cleanup_exceptions describe "handle_fatal_exception" do it "writes to the Logger and exit" do capture_io do |stdout| - system_exit_count = $system_exit_count - OpenC3.handle_fatal_exception(RuntimeError.new) - expect($system_exit_count).to eql(system_exit_count + 1) + expect { OpenC3.handle_fatal_exception(RuntimeError.new) }.to raise_error(SystemExit) expect(stdout.string).to match("Fatal Exception! Exiting...") end OpenC3.cleanup_exceptions() @@ -263,9 +257,7 @@ def self.cleanup_exceptions describe "handle_critical_exception" do it "writes to the Logger" do capture_io do |stdout| - system_exit_count = $system_exit_count OpenC3.handle_critical_exception(RuntimeError.new) - expect($system_exit_count).to eql(system_exit_count) expect(stdout.string).to match("Critical Exception!") end OpenC3.cleanup_exceptions()