Skip to content

Commit cbd87b7

Browse files
committed
chore: minor cleanup for clarity
Removed the one-line, one-use policy_default_init_flag method, and changed the argument order of merge_policy to match the hash merge (i.e. computed, explicit instead of the opposite)
1 parent 3a872de commit cbd87b7

File tree

2 files changed

+30
-94
lines changed

2 files changed

+30
-94
lines changed

lib/syskit/network_generation/dataflow_dynamics.rb

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -553,13 +553,15 @@ def compute_connection_policies
553553
policy_graph
554554
end
555555

556-
def merge_policy(explicit_policy, computed_policy)
557-
merged_policy = computed_policy.merge(explicit_policy)
558-
559-
if merged_policy[:type] == :data
560-
merged_policy.delete(:size)
561-
end
562-
556+
# Merges two connection policies
557+
#
558+
# @param [Hash] default_policy the policy that has the lowest
559+
# priority
560+
# @param [Hash] explicit_policy the policy that has the highest
561+
# priority
562+
def merge_policy(default_policy, explicit_policy)
563+
merged_policy = default_policy.merge(explicit_policy)
564+
merged_policy.delete(:size) if merged_policy[:type] == :data
563565
merged_policy
564566
end
565567

@@ -583,10 +585,6 @@ def compute_policies_from(connection_graph, source_task, policy_graph = {})
583585
policy_graph
584586
end
585587

586-
def policy_default_init_flag(policy, model)
587-
policy.merge(init: model.init_policy?)
588-
end
589-
590588
def policy_compute_data_element(
591589
source_task, source_port_name, sink_task, sink_port_name, fallback_policy
592590
)
@@ -645,9 +643,9 @@ def policy_for(
645643
end
646644

647645
model = source_task.find_output_port(source_port_name).model
648-
computed_policy = policy_default_init_flag(computed_policy, model)
646+
computed_policy[:init] = model.init_policy
649647

650-
merge_policy(explicit_policy, computed_policy)
648+
merge_policy(computed_policy, explicit_policy)
651649
end
652650

653651
def compute_reliable_connection_policy(

test/network_generation/test_dataflow_dynamics.rb

Lines changed: 19 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -414,33 +414,33 @@ module NetworkGeneration
414414
it "merges policies by preferring explicit values over " \
415415
"computed values" do
416416
explicit_policy = { type: :buffer, size: 20, init: true }
417-
computed_policy = { type: :buffer, size: 10, init: true }
417+
computed_policy = { type: :buffer, size: 10, init: false }
418418

419419
merged_policy =
420-
@dynamics.merge_policy(explicit_policy, computed_policy)
420+
@dynamics.merge_policy(computed_policy, explicit_policy)
421421

422422
assert_equal({ type: :buffer, size: 20, init: true }, merged_policy)
423423
end
424424

425425
it "removes the size value when the type is set to :data" do
426426
explicit_policy = { type: :data, init: true }
427-
computed_policy = { type: :buffer, size: 10, init: true }
427+
computed_policy = { type: :buffer, size: 10, init: false }
428428

429429
merged_policy =
430-
@dynamics.merge_policy(explicit_policy, computed_policy)
430+
@dynamics.merge_policy(computed_policy, explicit_policy)
431431

432432
assert_equal({ type: :data, init: true }, merged_policy)
433433
end
434434

435435
it "falls back to computed values when explicit values " \
436436
"are not provided" do
437-
explicit_policy = {}
437+
explicit_policy = { init: false }
438438
computed_policy = { type: :buffer, size: 10, init: true }
439439

440440
merged_policy =
441-
@dynamics.merge_policy(explicit_policy, computed_policy)
441+
@dynamics.merge_policy(computed_policy, explicit_policy)
442442

443-
assert_equal({ type: :buffer, size: 10, init: true }, merged_policy)
443+
assert_equal({ type: :buffer, size: 10, init: false }, merged_policy)
444444
end
445445
end
446446

@@ -506,31 +506,6 @@ module NetworkGeneration
506506
end
507507
end
508508

509-
describe "#policy_default_init_flag" do
510-
before do
511-
@task_m = Syskit::TaskContext.new_submodel do
512-
input_port "in", "/double"
513-
output_port "out", "/double"
514-
end
515-
@source_task_m = @task_m.new_submodel
516-
@sink_task_m = @task_m.new_submodel
517-
plan.add(@source_t = @source_task_m.new)
518-
plan.add(@sink_t = @sink_task_m.new)
519-
end
520-
521-
it "merges the init flag from the source port's model" do
522-
flexmock(@source_t.out_port.model)
523-
.should_receive(:init_policy?).explicitly
524-
.and_return(true)
525-
526-
policy = { type: :data }
527-
updated_policy = @dynamics.policy_default_init_flag(
528-
policy, @source_t.out_port.model
529-
)
530-
assert updated_policy[:init]
531-
end
532-
end
533-
534509
describe "#policy_for" do
535510
before do
536511
@task_m = Syskit::TaskContext.new_submodel do
@@ -588,71 +563,34 @@ module NetworkGeneration
588563
it "returns the value from compute_reliable_connection_policy if " \
589564
"the sink port is marked as needs_reliable_connection" do
590565
@sink_task_m.in_port.needs_reliable_connection
591-
fallback_policy = flexmock
592-
connection_policy = flexmock
593-
computed_policy = flexmock
594-
merged_policy = flexmock
595-
596-
flexmock(@dynamics)
597-
.should_receive(:merge_policy)
598-
.with({}, computed_policy)
599-
.and_return(merged_policy)
600-
601-
flexmock(@dynamics)
602-
.should_receive(:policy_default_init_flag)
603-
.with(connection_policy, @source_t.out_port.model)
604-
.once.and_return(computed_policy)
605-
606566
flexmock(@dynamics)
607567
.should_receive(:compute_reliable_connection_policy)
608-
.with(@source_t.out_port, @sink_t.in_port, fallback_policy)
609-
.once.and_return(connection_policy)
568+
.with(@source_t.out_port, @sink_t.in_port, { stage: "fallback" })
569+
.once.and_return({ stage: "reliable" })
610570

611571
policy = @dynamics.policy_for(
612-
@source_t, "out", "in", @sink_t, fallback_policy
572+
@source_t, "out", "in", @sink_t, { stage: "fallback" }
613573
)
614-
assert_equal merged_policy, policy
574+
assert_equal({ stage: "reliable", init: nil }, policy)
615575
end
616576

617-
it "merges init policy when sink requires reliable connection" do
618-
@sink_task_m.in_port.needs_reliable_connection
619-
@source_t.out_port.model.init_policy(true)
620-
fallback_policy = flexmock
621-
622-
flexmock(@dynamics)
623-
.should_receive(:compute_reliable_connection_policy)
624-
.with(@source_t.out_port, @sink_t.in_port, fallback_policy)
625-
.once.and_return({})
626-
627-
policy = @dynamics.policy_for(
628-
@source_t, "out", "in", @sink_t, fallback_policy
629-
)
630-
631-
assert policy[:init]
632-
end
633-
634-
it "merges init policy when sink requires 'buffer' connection type" do
635-
@sink_task_m.in_port.needs_buffered_connection
636-
577+
it "uses the init flag from the source port's model by default" do
637578
flexmock(@source_t.out_port.model)
638579
.should_receive(:init_policy?).explicitly
639580
.and_return(true)
640581

641-
@source_t.out_port.model.init_policy(true)
642-
policy = @dynamics.policy_for(@source_t, "out", "in", @sink_t, nil)
643-
582+
policy = @dynamics.policy_for(
583+
@source_t, "out", "in", @sink_t, nil
584+
)
644585
assert policy[:init]
645586
end
646587

647-
it "always applies the init flag from the source port's model" do
648-
flexmock(@source_t.out_port.model)
649-
.should_receive(:init_policy?).explicitly
650-
.and_return(true)
651-
588+
it "uses the explicitly given init flag over the one from the port model" do
589+
@source_t.out_port.model.init_policy(true)
652590
policy = @dynamics.policy_for(
653-
@source_t, "out", "in", @sink_t, nil
591+
@source_t, "out", "in", @sink_t, {}, { init: false }
654592
)
655-
assert policy[:init]
593+
refute policy[:init]
656594
end
657595
end
658596

0 commit comments

Comments
 (0)