Skip to content

Commit c907c4d

Browse files
Remove a broken optimization from _attempt_merge in minimization. (google#1963)
1 parent 8d9cdfb commit c907c4d

File tree

1 file changed

+10
-18
lines changed

1 file changed

+10
-18
lines changed

src/python/bot/minimizer/minimizer.py

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ def _handle_failing_hypothesis(self, hypothesis):
355355
with self.merge_lock:
356356
self._attempt_merge(hypotheses_to_merge)
357357

358-
def _attempt_merge(self, hypotheses, sibling_merge_succeeded=False):
358+
def _attempt_merge(self, hypotheses):
359359
"""Update the required token list if the queued changes don't conflict."""
360360
# If there's nothing to merge, we're done.
361361
if not hypotheses:
@@ -367,18 +367,10 @@ def _attempt_merge(self, hypotheses, sibling_merge_succeeded=False):
367367
aggregate_tokens.add(token)
368368
aggregate_hypothesis = list(aggregate_tokens)
369369

370-
if sibling_merge_succeeded:
371-
# We were able to remove all tokens from the other half of this
372-
# hypothesis, so we can assume that this would fail without running the
373-
# test. If this would also pass, there would not have been a conflict
374-
# while testing this set. Well, this could be a flaky test, but then we
375-
# have bigger problems.
376-
test_passed = True
377-
else:
378-
complement = self._range_complement(aggregate_hypothesis)
379-
test_file = self._prepare_test_input(self.tokens, complement)
380-
test_passed = self.minimizer.test_function(test_file)
381-
self._delete_file_if_needed(test_file)
370+
complement = self._range_complement(aggregate_hypothesis)
371+
test_file = self._prepare_test_input(self.tokens, complement)
372+
test_passed = self.minimizer.test_function(test_file)
373+
self._delete_file_if_needed(test_file)
382374

383375
# Failed (crashed), so there was no conflict here.
384376
if not test_passed:
@@ -395,11 +387,11 @@ def _attempt_merge(self, hypotheses, sibling_merge_succeeded=False):
395387
front = hypotheses[:middle]
396388
back = hypotheses[middle:]
397389

398-
# If we could remove either one of two hypotheses, favor removing the first.
399-
# FIXME: Fix this. Tracked in #1845.
400-
# pylint: disable=assignment-from-none
401-
front_merged_successfully = self._attempt_merge(front)
402-
self._attempt_merge(back, sibling_merge_succeeded=front_merged_successfully)
390+
# This could potentially be optimized to assume that if one test fails the
391+
# other would pass, but because of flaky tests it's safer to run the test
392+
# unconditionally.
393+
self._attempt_merge(front)
394+
self._attempt_merge(back)
403395

404396
def _do_single_pass_process(self):
405397
"""Process through a single pass of our test queue."""

0 commit comments

Comments
 (0)