From bfd5c91f72de7e5b3a2f867a371165b72bc3ab84 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Thu, 5 Feb 2026 12:56:03 -0300 Subject: [PATCH 1/3] fix: poll async resolutions within the test harness So far, if code executed during the test called apply_requirement_modifications, the test would have created an async resolution but never resolved it. This interferes with the rest of the test, and in particular if the test relies on connection management (which is "paused" during resolution) Make sure the expectation harness calls async_poll to finish the resolution --- lib/syskit/runtime/connection_management.rb | 5 +- lib/syskit/test/execution_expectations.rb | 8 ++++ .../instance_requirement_planning_handler.rb | 36 +++++++++------ test/test/test_execution_expectations.rb | 46 ++++++++++++++++++- test/test/test_spec.rb | 1 - 5 files changed, 78 insertions(+), 18 deletions(-) diff --git a/lib/syskit/runtime/connection_management.rb b/lib/syskit/runtime/connection_management.rb index 7bbc12177..72b8580d2 100644 --- a/lib/syskit/runtime/connection_management.rb +++ b/lib/syskit/runtime/connection_management.rb @@ -692,7 +692,10 @@ def active_task?(t) def update # Don't do anything if the engine is deploying - return if plan.syskit_has_async_resolution? + if plan.syskit_has_async_resolution? + debug "connection: skipping, async resolution in progress" + return + end tasks = dataflow_graph.modified_tasks tasks.delete_if { |t| !active_task?(t) } diff --git a/lib/syskit/test/execution_expectations.rb b/lib/syskit/test/execution_expectations.rb index 8406bdc35..21d052ee6 100644 --- a/lib/syskit/test/execution_expectations.rb +++ b/lib/syskit/test/execution_expectations.rb @@ -4,6 +4,14 @@ module Syskit module Test # Definition of expectations for Roby's expect_execution harness module ExecutionExpectations + Roby::Test::ExecutionExpectations.poll do |test, plan| + InstanceRequirementPlanningHandler.process_async_resolution(test, plan) + end + + Roby::Test::ExecutionExpectations.exit_allowed_condition do |test, plan| + !plan.syskit_has_async_resolution? + end + # @api private # # Helper used to resolve reader objects diff --git a/lib/syskit/test/instance_requirement_planning_handler.rb b/lib/syskit/test/instance_requirement_planning_handler.rb index c8d674455..dbdab23c6 100644 --- a/lib/syskit/test/instance_requirement_planning_handler.rb +++ b/lib/syskit/test/instance_requirement_planning_handler.rb @@ -56,15 +56,15 @@ def apply_requirements ) end - def replace_tasks_for_stub_network(results) + def self.replace_tasks_for_stub_network(test, plan, results) root_tasks = results.instance_requirement_tasks.map(&:planned_task) - stub_network = StubNetwork.new(@test) + stub_network = StubNetwork.new(test) # NOTE: this is a run-planner equivalent to syskit_stub_network # we will have to investigate whether we could implement one with # the other (probably), but in the meantime we must keep both # in sync - mapped_tasks = @plan.in_transaction do |trsc| + mapped_tasks = plan.in_transaction do |trsc| mapped_tasks = stub_network.apply_in_transaction(trsc, root_tasks) trsc.commit_transaction @@ -80,24 +80,30 @@ def replace_tasks_for_stub_network(results) # This is only relevant when we arent capturing errors during the network # resolution. When we are capturing errors, the capture pipeline already deals # with the failed task, and can deploy the stub network safely. - def consider_finished_due_to_errors?(results) + def self.consider_finished_due_to_errors?(results) !Syskit.conf.capture_errors_during_network_resolution? && results.error? end - def finished? - if @plan.syskit_has_async_resolution? - Thread.pass + def self.process_async_resolution(test, plan) + return unless plan.syskit_has_async_resolution? - async = @plan.syskit_current_resolution - @plan.syskit_poll_async_resolution(nil) - return if @plan.syskit_has_async_resolution? + Thread.pass - resolution_results = async.result - return true if consider_finished_due_to_errors?(resolution_results) - return unless @test.syskit_run_planner_stub? + async = plan.syskit_current_resolution + plan.syskit_poll_async_resolution(nil) + return if plan.syskit_has_async_resolution? - replace_tasks_for_stub_network(resolution_results) - end + resolution_results = async.result + return true if consider_finished_due_to_errors?(resolution_results) + return unless test.syskit_run_planner_stub? + + replace_tasks_for_stub_network( + test, plan, resolution_results + ) + end + + def finished? + self.class.process_async_resolution(@test, @plan) @planning_tasks.all? { |t| t.resolution_success? || t.finished? } end diff --git a/test/test/test_execution_expectations.rb b/test/test/test_execution_expectations.rb index db5519696..e8856b036 100644 --- a/test/test/test_execution_expectations.rb +++ b/test/test/test_execution_expectations.rb @@ -9,7 +9,7 @@ module Test attr_reader :task before do - task_m = Syskit::RubyTaskContext.new_submodel do + @task_m = task_m = Syskit::RubyTaskContext.new_submodel do input_port "in", "/int" output_port "out", "/int" end @@ -17,6 +17,50 @@ module Test @task = syskit_deploy_configure_and_start(task_m) end + describe "handling of apply_requirement_modifications" do + it "finishes within the test the resolutions " \ + "created during the generation block" do + plan.add(@task_m.as_plan) + expect_execution { Syskit::Runtime.apply_requirement_modifications(plan, force: true) } + .to do + achieve { plan.syskit_has_async_resolution? } + achieve { !plan.syskit_has_async_resolution? } + end + end + + it "finishes within the test the resolutions " \ + "created during the execution" do + plan.add(@task_m.as_plan) + applied = false + expect_execution + .poll do + unless applied + Syskit::Runtime.apply_requirement_modifications( + plan, force: true + ) + applied = true + end + end.to do + achieve { plan.syskit_has_async_resolution? } + achieve { !plan.syskit_has_async_resolution? } + end + end + + it "finishes at the end of the test resolutions " \ + "created during the execution" do + plan.add(@task_m.as_plan) + expect_execution + .poll do + Syskit::Runtime.apply_requirement_modifications( + plan, force: true + ) + end + .to_achieve { plan.syskit_has_async_resolution? } + + refute plan.syskit_has_async_resolution? + end + end + describe "#have_one_new_sample" do it "passes if the task emits a sample and returns it" do value = expect_execution { syskit_write task.in_port, 10 } diff --git a/test/test/test_spec.rb b/test/test/test_spec.rb index 3237abafd..a987344c8 100644 --- a/test/test/test_spec.rb +++ b/test/test/test_spec.rb @@ -53,7 +53,6 @@ module Test assert_equal false, cycles[0].planning_task_starting assert_equal true, cycles[0].planning_task_running - assert cycles[0].has_async_resolution end it "waits for the planning tasks to be started before it triggers the async resolution" do From 7fa4a09fa9149360f675fbde97ee4d17948da1eb Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Thu, 5 Feb 2026 14:31:49 -0300 Subject: [PATCH 2/3] fix: reset syskit_pending_forced_resolution on test teardown --- lib/syskit/test/spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/syskit/test/spec.rb b/lib/syskit/test/spec.rb index a1ae1c709..948273900 100644 --- a/lib/syskit/test/spec.rb +++ b/lib/syskit/test/spec.rb @@ -61,6 +61,7 @@ def kill_running_async_resolution plan.syskit_cancel_async_resolution plan.syskit_join_current_resolution + plan.syskit_pending_forced_resolution = false end def teardown_registered_plans From d19e91fb9f1f05d62bfdd86e462006ca499e5649 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Mon, 11 May 2026 13:18:05 -0300 Subject: [PATCH 3/3] fix: provide finer-grained control of the "async netgen" support in ExecutionExpectations Need disabling in the self tests that are actually testing async netgen --- lib/syskit/test/base.rb | 8 +++++ lib/syskit/test/execution_expectations.rb | 34 +++++++++++++++++-- .../network_generation/test_async_threaded.rb | 1 + .../test_apply_requirement_modifications.rb | 1 + test/test/test_execution_expectations.rb | 3 ++ test/test_instance_requirements_task.rb | 4 ++- test/test_task_context.rb | 14 ++++---- 7 files changed, 56 insertions(+), 9 deletions(-) diff --git a/lib/syskit/test/base.rb b/lib/syskit/test/base.rb index d028a7ed7..68d96b109 100644 --- a/lib/syskit/test/base.rb +++ b/lib/syskit/test/base.rb @@ -18,6 +18,8 @@ def setup @old_loglevel = Orocos.logger.level @__syskit_test_updated_attrs = {} + @expect_execution_process_async_resolutions = true + super end @@ -42,6 +44,12 @@ def teardown end end + # Controls at the test level whether the execution expectation harness + # should poll async resolution results + # + # @see ExecutionExpectations#process_async_resolutions? + attr_accessor :expect_execution_process_async_resolutions + def __syskit_test_disable_taskcontext_info_messages registered_plans.each do |p| next unless p.executable? diff --git a/lib/syskit/test/execution_expectations.rb b/lib/syskit/test/execution_expectations.rb index 21d052ee6..224d2c8e6 100644 --- a/lib/syskit/test/execution_expectations.rb +++ b/lib/syskit/test/execution_expectations.rb @@ -4,12 +4,42 @@ module Syskit module Test # Definition of expectations for Roby's expect_execution harness module ExecutionExpectations + # Controls whether the execution expectation harness should explicitly + # process async resolutions and wait for them to finish + # + # The default is true + # + # @see #process_async_resolutions? + def process_async_resolutions(flag) + @process_async_resolutions = flag + self + end + + # Controls whether the execution expectation harness should explicitly + # process async resolutions and wait for them to finish + # + # The default is true + # + # It can be controlled at the test level with + # {Base#expect_execution_process_async_resolutions=} + # + # @see #process_async_resolutions + def process_async_resolutions? + if @process_async_resolutions.nil? + @test.expect_execution_process_async_resolutions + else + @process_async_resolutions + end + end + Roby::Test::ExecutionExpectations.poll do |test, plan| - InstanceRequirementPlanningHandler.process_async_resolution(test, plan) + test.process_async_resolutions? && + InstanceRequirementPlanningHandler + .process_async_resolution(test, plan) end Roby::Test::ExecutionExpectations.exit_allowed_condition do |test, plan| - !plan.syskit_has_async_resolution? + !test.process_async_resolutions? || !plan.syskit_has_async_resolution? end # @api private diff --git a/test/network_generation/test_async_threaded.rb b/test/network_generation/test_async_threaded.rb index 74ef5b1b0..bf05d10b4 100644 --- a/test/network_generation/test_async_threaded.rb +++ b/test/network_generation/test_async_threaded.rb @@ -7,6 +7,7 @@ module NetworkGeneration describe AsyncThreaded do before do plan.syskit_async_method = AsyncThreaded + self.expect_execution_process_async_resolutions = false end describe "#poll" do diff --git a/test/runtime/test_apply_requirement_modifications.rb b/test/runtime/test_apply_requirement_modifications.rb index c979c2b21..0a64a0e58 100644 --- a/test/runtime/test_apply_requirement_modifications.rb +++ b/test/runtime/test_apply_requirement_modifications.rb @@ -9,6 +9,7 @@ module Runtime before do @__async_method = plan.syskit_async_method plan.syskit_async_method = async_method + self.expect_execution_process_async_resolutions = false end after do diff --git a/test/test/test_execution_expectations.rb b/test/test/test_execution_expectations.rb index e8856b036..4e45093f8 100644 --- a/test/test/test_execution_expectations.rb +++ b/test/test/test_execution_expectations.rb @@ -22,6 +22,7 @@ module Test "created during the generation block" do plan.add(@task_m.as_plan) expect_execution { Syskit::Runtime.apply_requirement_modifications(plan, force: true) } + .process_async_resolutions(true) .to do achieve { plan.syskit_has_async_resolution? } achieve { !plan.syskit_has_async_resolution? } @@ -33,6 +34,7 @@ module Test plan.add(@task_m.as_plan) applied = false expect_execution + .process_async_resolutions(true) .poll do unless applied Syskit::Runtime.apply_requirement_modifications( @@ -50,6 +52,7 @@ module Test "created during the execution" do plan.add(@task_m.as_plan) expect_execution + .process_async_resolutions(true) .poll do Syskit::Runtime.apply_requirement_modifications( plan, force: true diff --git a/test/test_instance_requirements_task.rb b/test/test_instance_requirements_task.rb index 0247d84fc..cde92ce5b 100644 --- a/test/test_instance_requirements_task.rb +++ b/test/test_instance_requirements_task.rb @@ -20,7 +20,9 @@ it "triggers a network resolution when started" do task = plan.add_permanent_task(cmp_m.as_plan) - execute { task.planning_task.start! } + expect_execution { task.planning_task.start! } + .process_async_resolutions(false) + .to_run assert plan.syskit_current_resolution end diff --git a/test/test_task_context.rb b/test/test_task_context.rb index 2a2b01b04..593264e85 100644 --- a/test/test_task_context.rb +++ b/test/test_task_context.rb @@ -480,12 +480,14 @@ def start_task end after do - if task.start_event.pending? - task.start_event.emit - end - if task.running? - expect_execution { task.stop! } - .to { emit task.stop_event } + if task + if task.start_event.pending? + task.start_event.emit + end + if task.running? + expect_execution { task.stop! } + .to { emit task.stop_event } + end end end