From 72722c14a40ccee514781d553c026bdf6262a196 Mon Sep 17 00:00:00 2001 From: Jhonas Date: Mon, 23 Dec 2024 18:11:14 -0300 Subject: [PATCH] fix(test): update unit tests with resolution errors They should take the place and former raised errors --- test/network_generation/test_async.rb | 22 +++++--- .../test_system_network_generator.rb | 55 ++++++------------- test/test/test_network_manipulation.rb | 2 +- test/test_exceptions.rb | 40 +++++++++----- 4 files changed, 61 insertions(+), 58 deletions(-) diff --git a/test/network_generation/test_async.rb b/test/network_generation/test_async.rb index da2dd844a..7ddc53285 100644 --- a/test/network_generation/test_async.rb +++ b/test/network_generation/test_async.rb @@ -68,7 +68,8 @@ def assert_future_fulfilled(future) resolution = subject.prepare(requirements) engine = flexmock(resolution.engine, :strict) engine.should_receive(:resolve_system_network) - .with(requirements, any).once.and_return(ret = flexmock) + .with(requirements, any).once + .and_return([ret = flexmock, []]) if RUBY_VERSION >= "2.7" engine.should_receive(:apply_system_network_to_plan).with(ret).once @@ -89,7 +90,8 @@ def assert_future_fulfilled(future) ) engine = flexmock(resolution.engine, :strict) engine.should_receive(:resolve_system_network) - .with(requirements, any).once.and_return(ret = flexmock) + .with(requirements, any).once + .and_return([ret = flexmock, []]) engine.should_receive(:apply_system_network_to_plan) .with(ret, compute_deployments: false).once resolution.execute @@ -103,7 +105,8 @@ def assert_future_fulfilled(future) requirements = Set[flexmock] resolution = subject.prepare(requirements) engine = flexmock(resolution.engine, :strict) - engine.should_receive(:resolve_system_network).and_return(ret = flexmock) + engine.should_receive(:resolve_system_network) + .and_return([ret = flexmock, []]) engine.should_receive(:apply_system_network_to_plan).and_raise(error_t) flexmock(engine.work_plan).should_receive(:discard_transaction).once resolution.execute @@ -129,17 +132,22 @@ def assert_future_fulfilled(future) srv_m = Syskit::DataService.new_submodel plan.add(task = Models::Placeholder.create_for([srv_m]).new) - resolution_errors = [ResolutionError.new(task, RuntimeError)] + resolution_errors = + [InternalResolutionFailure.new([[task, 0]], RuntimeError)] requirements = Set[flexmock] resolution = subject.prepare(requirements) engine = flexmock(resolution.engine, :strict) engine.should_receive(:resolve_system_network) .and_return([[], resolution_errors]) engine.should_receive(:apply_system_network_to_plan).once - flexmock(engine).should_receive(:replace_resolution_errors_tasks) - .once.pass_thru + errors = [] + flexmock(InternalResolutionFailure) + .new_instances + .should_receive(:to_resolution_error) + .pass_thru { errors = _1 } flexmock(subject.plan.execution_engine).should_receive(:add_error) - .with(*resolution_errors) + .with(*errors) + .with_any_kw_args resolution.execute subject.join assert subject.finished? diff --git a/test/network_generation/test_system_network_generator.rb b/test/network_generation/test_system_network_generator.rb index 1d3b36caf..40d8396b0 100644 --- a/test/network_generation/test_system_network_generator.rb +++ b/test/network_generation/test_system_network_generator.rb @@ -111,32 +111,6 @@ def arg=(value) flexmock(generator).should_receive(:validate_generated_network).and_return([]).once generator.compute_system_network([], validate_generated_network: true) end - - it "removes tasks with errors and their dependencies on the generated network" do - srv_m = Syskit::DataService.new_submodel - cmp_m = Syskit::Composition.new_submodel do - add srv_m, as: "test" - end - task = cmp_m.instanciate(plan).to_task - requirements = cmp_m.to_instance_requirements - plan.add(task) - - generator = SystemNetworkGenerator.new(plan) - error = ResolutionError.new(task) - flexmock(generator).should_receive(:validate_network) - .and_return([error]) - flexmock(generator).should_receive(:instanciate) - .pass_thru { [task] } - # Turn off garbage collect because the mocked resolution error task - # would be garbage collected. - toplevel_tasks, = - generator.compute_system_network([requirements], - garbage_collect: false, - validate_generated_network: true, - validate_abstract_network: false) - refute plan.has_task? task - assert toplevel_tasks.empty? - end end describe "#generate" do @@ -188,13 +162,18 @@ def compute_system_network(*requirements) it "validates that there are no placeholder tasks left in the plan" do srv_m = Syskit::DataService.new_submodel plan.add(task = Models::Placeholder.create_for([srv_m]).new) + flexmock(SystemNetworkGenerator) + .should_receive(:find_toplevel_tasks_with_index_of) + .and_return([[task, 0]]) generator = SystemNetworkGenerator.new(plan) actual = generator.validate_generated_network - expected = [ - ResolutionError.new(task, - TaskAllocationFailed.new(generator, [task])) - ] - assert_equal expected, actual + + expected = InternalResolutionFailure.new( + [[task, 0]], TaskAllocationFailed.new(generator, [task]), + "could not find implementation for the following abstract " \ + "task: #{task}" + ) + assert_equal [expected], actual end it "validates that devices are allocated at most once" do @@ -207,6 +186,9 @@ def compute_system_network(*requirements) plan.add(task1 = task_m.new(arg: 1, test_dev: robot.test_dev)) plan.add(task2 = task_m.new(arg: 2, test_dev: robot.test_dev)) + flexmock(SystemNetworkGenerator) + .should_receive(:find_toplevel_tasks_with_index_of) + .and_return([[task1, 0]], [[task2, 1]]) generator = SystemNetworkGenerator.new(plan) actual = generator.validate_generated_network @@ -231,12 +213,11 @@ def compute_system_network(*requirements) conf: default(["default"]), read_only: default(false) MSG - actual.each_with_index do |e, i| - formatted = PP.pp(e, +"") - assert_equal expected_failed_tasks[i], e.failed_task - assert_equal expected_message, - formatted.gsub(//, "") - end + assert_equal 1, actual.size + e = actual.first + formatted = PP.pp(e.original_exception, +"") + assert_equal expected_failed_tasks, e.failed_toplevel_tasks + assert_equal expected_message, formatted.gsub(//, "") end end diff --git a/test/test/test_network_manipulation.rb b/test/test/test_network_manipulation.rb index 17589ccf6..7b6e4c7ba 100644 --- a/test/test/test_network_manipulation.rb +++ b/test/test/test_network_manipulation.rb @@ -376,7 +376,7 @@ module Test end it "fails if the plan can't be deployed" do - plan.add_mission_task(@cmp_m.as_plan) + plan.add_mission_task(@cmp_m.use("srv" => @task_m).as_plan) assert_raises(Roby::Test::ExecutionExpectations::UnexpectedErrors) do deploy_current_plan end diff --git a/test/test_exceptions.rb b/test/test_exceptions.rb index af3b71b08..5ef9cc7e5 100644 --- a/test/test_exceptions.rb +++ b/test/test_exceptions.rb @@ -23,13 +23,19 @@ module Syskit plan.add(task1 = driver_m.new(arg: 1, test_dev: robot.test_dev)) plan.add(task2 = driver_m.new(arg: 2, test_dev: robot.test_dev)) - e = assert_raises(ConflictingDeviceAllocation) do - NetworkGeneration::SystemNetworkGenerator + flexmock(NetworkGeneration::SystemNetworkGenerator) + .should_receive(:find_toplevel_tasks_with_index_of) + .and_return([[task1, 0]], [[task2, 1]]) + errors = NetworkGeneration::SystemNetworkGenerator .new(plan).validate_generated_network - end - assert_equal Set[task1, task2], e.tasks.to_set - formatted = PP.pp(e, +"") + expected_tasks = [task1, task2] + assert_equal 1, errors.size + + e = errors.first + assert_kind_of ConflictingDeviceAllocation, e.original_exception + assert_equal expected_tasks, e.failed_toplevel_tasks + formatted = PP.pp(e.original_exception, +"") expected = <<~PP.chomp device 'test' of type D is assigned to two tasks that cannot be merged @@ -71,11 +77,13 @@ module Syskit .use("test" => robot.test_dev.with_arguments(arg: 2)) self.syskit_run_planner_validate_network = true - e = assert_raises(ConflictingDeviceAllocation) do + e = assert_raises(Syskit::NetworkGeneration::ResolutionError) do run_planners([profile.test1_def, profile.test2_def]) end - formatted = PP.pp(e, +"") + assert_equal 1, e.original_exceptions.size + assert_kind_of ConflictingDeviceAllocation, e.original_exceptions.first + formatted = PP.pp(e.original_exceptions.first, +"") expected = <<~PP.chomp device 'test' of type D is assigned to two tasks that cannot be merged Chain 1 cannot be merged in chain 2: @@ -123,13 +131,19 @@ module Syskit plan.add(task2 = task_m.new(arg: 2)) task1.out_port.connect_to driver1.in_port task2.out_port.connect_to driver2.in_port - e = assert_raises(ConflictingDeviceAllocation) do - NetworkGeneration::SystemNetworkGenerator - .new(plan).validate_generated_network - end + flexmock(NetworkGeneration::SystemNetworkGenerator) + .should_receive(:find_toplevel_tasks_with_index_of) + .and_return([[driver1, 0]], [[driver2, 1]]) + generator = NetworkGeneration::SystemNetworkGenerator.new(plan) + errors = generator.validate_generated_network + + expected_tasks = [driver1, driver2] - assert_equal Set[driver1, driver2], e.tasks.to_set - formatted = PP.pp(e, +"") + assert_equal 1, errors.size + e = errors.first + assert_kind_of ConflictingDeviceAllocation, e.original_exception + assert_equal expected_tasks, e.failed_toplevel_tasks + formatted = PP.pp(e.original_exception, +"") expected = <<~PP.chomp device 'test' of type D is assigned to two tasks that cannot be merged