From e8ef3ca94ddfc9fcdb10276b2991a1138fa82985 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Thu, 29 May 2025 17:20:36 -0300 Subject: [PATCH 1/2] fix: only instanciate toplevel abstract tasks when instanciating placeholders Placeholders would instanciate a whole composition when based on a composition model --- lib/syskit/models/placeholder.rb | 9 ++++++ test/actions/test_profile.rb | 50 ++++++++++++++++++++++---------- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/lib/syskit/models/placeholder.rb b/lib/syskit/models/placeholder.rb index dffcc4dec..91835daf7 100644 --- a/lib/syskit/models/placeholder.rb +++ b/lib/syskit/models/placeholder.rb @@ -201,6 +201,14 @@ def task_extension Syskit::Placeholder end + def instanciate( + plan, _context = DependencyInjectionContext.new, + task_arguments: {}, ** + ) + plan.add(task = new(**task_arguments)) + task + end + # Create a task model that is an aggregate of all the provided # models (components and services) # @@ -222,6 +230,7 @@ def create_for(models, component_model: nil, as: nil) model.concrete_model = nil model.include task_extension model.extend self + model.extend self::Creation model.proxied_component_model = task_model model.proxied_data_service_models = service_models.dup model.update_proxy_mappings diff --git a/test/actions/test_profile.rb b/test/actions/test_profile.rb index 5c99da2c7..474ebb016 100644 --- a/test/actions/test_profile.rb +++ b/test/actions/test_profile.rb @@ -354,22 +354,40 @@ module Actions end describe "#tag" do - it "cannot be merged with another tag of the same type" do - srv_m = DataService.new_submodel - profile = Profile.new - profile.tag "test", srv_m - profile.tag "other", srv_m - test_task = profile.test_tag.instanciate(plan) - other_task = profile.other_tag.instanciate(plan) - refute test_task.can_merge?(other_task) - end - it "can be merged with another instance of itself" do - srv_m = DataService.new_submodel - profile = Profile.new - profile.tag "test", srv_m - test_task1 = profile.test_tag.instanciate(plan) - test_task2 = profile.test_tag.instanciate(plan) - assert test_task1.can_merge?(test_task2) + describe "a tag instance" do + it "cannot be merged with an instance of another tag " \ + "of the same type" do + srv_m = DataService.new_submodel + profile = Profile.new + profile.tag "test", srv_m + profile.tag "other", srv_m + test_task = profile.test_tag.instanciate(plan) + other_task = profile.other_tag.instanciate(plan) + refute test_task.can_merge?(other_task) + end + + it "can be merged with another instance of itself" do + srv_m = DataService.new_submodel + profile = Profile.new + profile.tag "test", srv_m + test_task1 = profile.test_tag.instanciate(plan) + test_task2 = profile.test_tag.instanciate(plan) + assert test_task1.can_merge?(test_task2) + end + + it "overrides component model instanciation to always only add " \ + "an abstract placeholder task" do + task_m = TaskContext.new_submodel + cmp_m = Composition.new_submodel + cmp_m.add task_m, as: "child" + + profile = Profile.new + profile.tag "test", cmp_m + task = profile.test_tag.instanciate(plan) + assert task.abstract? + assert task.placeholder? + refute task.find_child_from_role("child") + end end end From 13b35532cbef8b1d5ad0c729e8b22f240561e0e4 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Thu, 29 May 2025 17:54:41 -0300 Subject: [PATCH 2/2] chore: add test that compositions that reference tags instanciate them properly See the test. This aims at reproducing a possible source for a bug found during development (but fails). Note that the test passes even if the previous fix is not applied ("fix: only instanciate toplevel abstract tasks when instanciating placeholders") --- test/actions/test_profile.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/actions/test_profile.rb b/test/actions/test_profile.rb index 474ebb016..398c6a6ea 100644 --- a/test/actions/test_profile.rb +++ b/test/actions/test_profile.rb @@ -388,6 +388,26 @@ module Actions assert task.placeholder? refute task.find_child_from_role("child") end + + it "adds a task instance that can be found by its composition " \ + "when based on one" do + # This is a reproduction of a bug, making sure the source + # is indeed not this code + task_m = TaskContext.new_submodel + cmp_m = Composition.new_submodel + cmp_m.add task_m, as: "child" + root_cmp_m = Composition.new_submodel + root_cmp_m.add cmp_m, as: "test" + + profile = Profile.new + profile.tag "test", cmp_m + profile.define "test", root_cmp_m.use("test" => profile.test_tag) + + root_cmp = profile.test_def.instanciate(plan) + placeholder = plan.find_tasks(cmp_m).abstract.first + assert placeholder + assert placeholder.placeholder? + end end end