[swarming_retry] Refactor: Make TaskTracker's "attempts" field private
Introduce get_all_attempts() as a getter method for the few remaining
uses outside of TaskTracker.
Bug: 52328
Test: recipe unit tests
Change-Id: I375d193d50927ea903472569f4f2a78578d7249c
Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/recipes/+/404585
Commit-Queue: Mark Seaborn <mseaborn@google.com>
Reviewed-by: Oliver Newman <olivernewman@google.com>
diff --git a/recipe_modules/swarming_retry/api.py b/recipe_modules/swarming_retry/api.py
index 5fb0200..842e3c6 100644
--- a/recipe_modules/swarming_retry/api.py
+++ b/recipe_modules/swarming_retry/api.py
@@ -94,7 +94,7 @@
self._launch_deadline_time = launch_deadline_time
self._current_time = 0
self._max_attempts = task.max_attempts
- self.attempts = []
+ self._attempts = []
self._in_progress_attempts = []
self._successes_required = run_count
self._successes_got = 0
@@ -132,7 +132,7 @@
# builders.
attempts_allowed = self._max_attempts * self._successes_required
remaining_needed = self._successes_required - self._successes_got
- remaining_allowed = attempts_allowed - len(self.attempts)
+ remaining_allowed = attempts_allowed - len(self._attempts)
if remaining_needed > remaining_allowed:
return self._OVERALL_FAILURE, 0
# Apply the "no futile retries" strategy: If we need multiple
@@ -143,7 +143,7 @@
if (
self._successes_required > 1
and self._successes_got == 0
- and len(self.attempts) >= self._successes_required
+ and len(self._attempts) >= self._successes_required
):
return self._OVERALL_FAILURE, 0
return self._LAUNCH_MORE, remaining_needed
@@ -172,33 +172,36 @@
# This means that if there is a long queue for Swarming tasks to run,
# only the first attempts should wait. Subsequent attempts should
# jump ahead in the queue.
- priority_boost_amount = len(self.attempts)
+ priority_boost_amount = len(self._attempts)
task_ids = []
for _ in xrange(number_to_launch):
- attempt_index = len(self.attempts)
+ attempt_index = len(self._attempts)
task_name = "%s (attempt %d)" % (self.name, attempt_index)
with self._api.step.nest(task_name) as presentation:
attempt = self._task.launch(priority_boost_amount)
attempt.index = attempt_index
- self.attempts.append(attempt)
+ self._attempts.append(attempt)
self._in_progress_attempts.append(attempt)
task_ids.append(attempt.task_id)
presentation.links["Swarming task"] = attempt.task_ui_link
return task_ids
def get_successful_attempts(self):
- return [attempt for attempt in self.attempts if attempt.success]
+ return [attempt for attempt in self._attempts if attempt.success]
def get_failed_attempts(self):
attempts = []
if not self.in_progress:
- attempts = [attempt for attempt in self.attempts if not attempt.success]
+ attempts = [attempt for attempt in self._attempts if not attempt.success]
return attempts
def get_in_progress_attempts(self):
return self._in_progress_attempts[:]
+ def get_all_attempts(self):
+ return self._attempts[:]
+
@property
def in_progress(self):
state, _ = self._get_state()
@@ -251,7 +254,7 @@
None
"""
with self._api.step.nest(self.name) as task_step_presentation:
- for attempt in self.attempts:
+ for attempt in self._attempts:
self._task.present_attempt(task_step_presentation, attempt, **kwargs)
# Show incomplete tasks in green so as not to be confused with
diff --git a/recipe_modules/swarming_retry/examples/full.py b/recipe_modules/swarming_retry/examples/full.py
index 5d3706e..05594ba 100644
--- a/recipe_modules/swarming_retry/examples/full.py
+++ b/recipe_modules/swarming_retry/examples/full.py
@@ -227,6 +227,7 @@
)
for task in tasks:
# Call these methods to satisfy code coverage checks.
+ assert isinstance(task.get_all_attempts(), list)
assert isinstance(task.get_successful_attempts(), list)
assert isinstance(task.get_failed_attempts(), list)
else:
diff --git a/recipe_modules/testing/examples/full.py b/recipe_modules/testing/examples/full.py
index 7ced9ac..1397ca3 100644
--- a/recipe_modules/testing/examples/full.py
+++ b/recipe_modules/testing/examples/full.py
@@ -162,9 +162,9 @@
# Upload test results
if upload_results:
final_results = [
- task.attempts[-1].test_results
+ task.get_all_attempts()[-1].test_results
for task in testing_tasks
- if task.attempts[-1].test_results
+ if task.get_all_attempts()[-1].test_results
]
for test_results in final_results:
test_results.upload_results(
diff --git a/recipes/fuchsia/fuchsia.py b/recipes/fuchsia/fuchsia.py
index f7f1da3..26bc8fb 100644
--- a/recipes/fuchsia/fuchsia.py
+++ b/recipes/fuchsia/fuchsia.py
@@ -235,11 +235,11 @@
all_results = []
successful_results = []
- for t in testing_tasks:
- for attempt in t.attempts:
+ for task in testing_tasks:
+ for attempt in task.get_all_attempts():
if attempt.test_results:
all_results.append(attempt.test_results)
- for attempt in t.get_successful_attempts():
+ for attempt in task.get_successful_attempts():
assert attempt.test_results
successful_results.append(attempt.test_results)
diff --git a/recipes/fuchsia_perf.py b/recipes/fuchsia_perf.py
index e9dc378..6064735 100644
--- a/recipes/fuchsia_perf.py
+++ b/recipes/fuchsia_perf.py
@@ -231,8 +231,9 @@
)
assert len(testing_tasks) == 1, "%d != 1" % len(testing_tasks)
testing_task = testing_tasks[0]
- assert len(testing_task.attempts) == 1, "%d != 1" % len(testing_task.attempts)
- test_results = testing_task.attempts[0].test_results
+ attempts = testing_task.get_all_attempts()
+ assert len(attempts) == 1, "%d != 1" % len(attempts)
+ test_results = attempts[0].test_results
# Upload results for all of the benchmarks that ran successfully.
if test_results and not api.buildbucket_util.is_tryjob:
