Skip to content

Commit 2d43c30

Browse files
committed
Fix insertion ordering of repeat control
Signed-off-by: Sylvain Hellegouarch <[email protected]>
1 parent e1c7d89 commit 2d43c30

File tree

3 files changed

+93
-12
lines changed

3 files changed

+93
-12
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
### Fixed
88

99
* Make sure the `repeat` control doesn't run when it count is less than 2
10+
* Prevent endless looping
11+
* Ensure insertion ordering is correct
1012

1113
### Changed
1214

chaosaddons/controls/repeat.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ def after_activity_control(context: Activity, experiment: Experiment,
4747
Note, if `repeat_count` is less than 2, then this is a noop.
4848
"""
4949
activity = context
50+
51+
# prevent endless looping
52+
if "iteration_index" in activity:
53+
return None
54+
5055
activity_name = activity["name"]
5156
activity_type = activity["type"]
5257

@@ -58,6 +63,9 @@ def after_activity_control(context: Activity, experiment: Experiment,
5863
)
5964
return None
6065

66+
logger.debug(
67+
f"Repeat {activity_type} '{activity_name}' {repeat_count} more times"
68+
)
6169
last_status = state.get("status")
6270

6371
if break_if_previous_iteration_failed and last_status != "succeeded":
@@ -89,16 +97,12 @@ def repeat_activity(activity: Activity, activities: List[Activity],
8997
return
9098

9199
activity_name = activity["name"]
100+
copy_activities = activities[:]
92101

93-
for a in reversed(activities):
102+
for pos, a in enumerate(copy_activities):
94103
if a["name"] == activity_name:
95-
activity = deepcopy(activity)
96-
if "iteration_index" not in activity:
97-
activity["iteration_index"] = 1
98-
else:
99-
if activity["iteration_index"] == repeat_count:
100-
return
101-
activity["iteration_index"] += 1
102-
103-
activities.append(activity)
104+
for index in range(1, repeat_count+1):
105+
new_activity = deepcopy(activity)
106+
new_activity["iteration_index"] = index
107+
activities.insert(pos+index, new_activity)
104108
break

tests/test_repeat.py

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def test_repeat_n_times_in_hypothesis():
5454
assert len(x.get("steady-state-hypothesis").get("probes")) == 4
5555

5656

57-
def test_repeat_once_in_hypothesis():
57+
def test_repeat_once():
5858
a = {
5959
"name": "probe-B",
6060
"type": "probe",
@@ -103,7 +103,82 @@ def test_repeat_once_in_hypothesis():
103103

104104
after_activity_control(
105105
context=a, experiment=x, state={},
106-
repeat_count=1
106+
repeat_count=2
107107
)
108108

109109
assert len(x.get("method")) == 4
110+
111+
112+
def test_repeat_inserts():
113+
a = {
114+
"name": "probe-B",
115+
"type": "probe",
116+
"provider": {
117+
"type": "python",
118+
"module": "builtins",
119+
"func": "sum",
120+
"arguments": {
121+
"iterable": [6, 7]
122+
}
123+
}
124+
}
125+
x = {
126+
"title": "hello",
127+
"description": "n/a",
128+
"method": [
129+
{
130+
"name": "probe-A",
131+
"type": "probe",
132+
"provider": {
133+
"type": "python",
134+
"module": "builtins",
135+
"func": "sum",
136+
"arguments": {
137+
"iterable": [2, 3]
138+
}
139+
}
140+
},
141+
a,
142+
{
143+
"name": "probe-C",
144+
"type": "probe",
145+
"provider": {
146+
"type": "python",
147+
"module": "builtins",
148+
"func": "sum",
149+
"arguments": {
150+
"iterable": [2, 3]
151+
}
152+
}
153+
},
154+
{
155+
"name": "probe-D",
156+
"type": "probe",
157+
"provider": {
158+
"type": "python",
159+
"module": "builtins",
160+
"func": "sum",
161+
"arguments": {
162+
"iterable": [2, 3]
163+
}
164+
}
165+
},
166+
]
167+
}
168+
169+
assert len(x.get("method")) == 4
170+
171+
after_activity_control(
172+
context=a, experiment=x, state={},
173+
repeat_count=3
174+
)
175+
176+
assert len(x.get("method")) == 6
177+
178+
m = x.get("method")
179+
assert m[0]["name"] == "probe-A"
180+
assert m[1]["name"] == "probe-B"
181+
assert m[2]["name"] == "probe-B"
182+
assert m[3]["name"] == "probe-B"
183+
assert m[4]["name"] == "probe-C"
184+
assert m[5]["name"] == "probe-D"

0 commit comments

Comments
 (0)