From e17053bd48365bc6a2f6f045e98b186798740556 Mon Sep 17 00:00:00 2001 From: Ethan Stoner Date: Fri, 4 Sep 2026 08:36:14 -0700 Subject: [PATCH] keep the rewards tab when a task fails, and put it back on the rewards page The cleanup after each task called close_all_other_tabs() with no arguments. That defaults `exceptions` to the currently focused handle, so when a task died with a Bing tab in focus the cleanup kept the Bing tab and closed the Rewards one. Every later task then looked for Rewards controls on a search results page and reported [SKIP] for a UI that was present and working the whole time. One failure cost the rest of the run. Measured in the container rather than reasoned about: with a Rewards tab and a Bing tab open and the Bing one focused, close_all_other_tabs() leaves ['https://www.bing.com/search?q=weather'] behind and the Rewards handle is gone. Two separate things have to hold before the next task can work, and they failed independently. The right tab has to survive, so restore_main_tab names the handle recorded at startup instead of trusting whatever happens to be focused. The surviving tab also has to be showing the right page, which the tab fix alone does not give you: run_search_batch navigates the main tab to bing.com itself, so a failure part way through that batch strands the main tab on a search page with no second tab involved at all. Navigating only happens when the step did not finish, and is skipped when the browser is already on the Rewards host, so a clean run navigates nowhere and a panel that merely failed to render does not cost a page load. Both recoveries swallow their own errors. Raising here would replace the real failure with the tidy-up's, which is why the close_all_other_tabs call was wrapped in the first place. End to end in a real browser: two tabs with the Bing one focused become one tab, the original handle, on rewards.bing.com/about (the logged out redirect), and a main tab stranded on bing.com recovers too. Fifteen unit tests cover the tab choice, the missing-main-tab fallback, the failure and skip paths, a clean run navigating nowhere, and a driver too broken to do either. Refs #63 Claude-Session: https://claude.ai/code/session_0113hp36oUBeMqG7twfuC8rE --- src/rewards_tasks.py | 74 +++++++++- tests/test_task_recovery.py | 274 ++++++++++++++++++++++++++++++++++++ 2 files changed, 341 insertions(+), 7 deletions(-) create mode 100644 tests/test_task_recovery.py diff --git a/src/rewards_tasks.py b/src/rewards_tasks.py index 0c327b4..8dc6f98 100644 --- a/src/rewards_tasks.py +++ b/src/rewards_tasks.py @@ -18,6 +18,8 @@ import element_selectors VISUAL_SEARCH_IMAGE_PATH = os.path.abspath("visual_search.jpg") +REWARDS_HOME_URL = "https://rewards.bing.com/" + logger = logging.getLogger(__name__) @@ -76,11 +78,16 @@ class RewardsTaskUtils: self.driver.execute_cdp_cmd("Network.setExtraHTTPHeaders", {"headers": headers}) - self.driver.get("https://rewards.bing.com/") + self.driver.get(REWARDS_HOME_URL) self.tab_utils = tab_utils.TabUtils(driver) self.tab_utils.ensure_focus() + # The tab the tasks work in. Recorded rather than looked up later, + # because "the current tab" stops meaning this one the moment a task + # opens a card in a new one. + self.main_window = driver.current_window_handle + self.mouse = mouse_trajectory.MouseUtils(driver) self.keyboard = mimic_typing.KeyboardUtils(driver) self.elements = element_selectors.ElementSelectionUtils(driver) @@ -396,9 +403,57 @@ class RewardsTaskUtils: ) self.wait_for_then_click(self.elements.get_clear_bing_search_query_button) - self.driver.get("https://rewards.bing.com/") + self.driver.get(REWARDS_HOME_URL) self.tab_utils.ensure_focus() + def restore_main_tab(self): + """Close the stray tabs, keeping the one the tasks work in. + + close_all_other_tabs with no arguments keeps whatever tab is focused + right now. After a task that died on a Bing tab that is the Bing tab, so + the cleanup closed the Rewards tab and kept the search results. Naming + the tab to keep is the difference between tidying up and destroying the + only tab the next task can use. + + If the main tab is gone, whatever is left is better than nothing: the + page fix below still has to run either way. + """ + try: + handles = self.driver.window_handles + + if not handles: + return + + keep = self.main_window if self.main_window in handles else handles[0] + + self.tab_utils.close_all_other_tabs(exceptions=[keep]) + except Exception as exc: + logger.warning( + "Could not tidy the open tabs: %s", log_utils.exception_summary(exc) + ) + + def return_to_rewards_home(self): + """Put the browser back on the Rewards home page. + + Only called when a task did not finish. Navigating after every task + would reload the page six times a run for no reason, and the tasks that + succeed already leave the browser somewhere their successor can work + from. + """ + try: + if self.driver.current_url.startswith(REWARDS_HOME_URL): + return + + self.driver.get(REWARDS_HOME_URL) + self.tab_utils.ensure_focus() + except Exception as exc: + # Recovery is best effort. If even this fails the next task will + # report its own [SKIP], which is no worse than before. + logger.warning( + "Could not return to the Rewards home page: %s", + log_utils.exception_summary(exc) + ) + def claim_bonus_points(self): self.switch_to_dashboard() @@ -426,9 +481,12 @@ class RewardsTaskUtils: # The tags stay in the message rather than being folded into the # level, they are the per-task outcome summary and reading a run # means scanning for them. + completed = False + try: step() logger.info("[OK] %s", name) + completed = True except Exception as exc: tag, reason = task_failure_report(exc) @@ -438,8 +496,10 @@ class RewardsTaskUtils: exc_info=logger.isEnabledFor(logging.DEBUG) ) - # Leave a clean tab state behind for the next task. - try: - self.tab_utils.close_all_other_tabs() - except Exception: - pass \ No newline at end of file + # Leave a clean tab state behind for the next task. Both halves of + # this matter, and they are separate failures: the right tab has to + # survive, and it has to be showing the right page. + self.restore_main_tab() + + if not completed: + self.return_to_rewards_home() \ No newline at end of file diff --git a/tests/test_task_recovery.py b/tests/test_task_recovery.py new file mode 100644 index 0000000..12a9a9f --- /dev/null +++ b/tests/test_task_recovery.py @@ -0,0 +1,274 @@ +"""Tests for getting back to a known page after a task fails. + +The bug (#63): a task that dies partway through leaves the browser wherever it +stopped, which for the search and visual search tasks is a Bing page rather than +the Rewards site. Every later task begins by looking for controls that only +exist on rewards.bing.com, so one failure made all of them report [SKIP] for a +UI that was present and working the whole time. + +The stand-ins here are local rather than in fakes.py: that module fakes the DOM +lookups the selectors do, and what these need is a driver that remembers where +it navigated. No browser, so they run anywhere. + + python -m unittest discover -s tests +""" + +import logging +import os +import sys +import unittest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src")) + +from selenium.common.exceptions import NoSuchElementException, TimeoutException, WebDriverException + +import rewards_tasks + +HOME = rewards_tasks.REWARDS_HOME_URL +BING_RESULTS = "https://www.bing.com/search?q=weather" + + +MAIN_TAB = "main-tab" +BING_TAB = "bing-tab" + + +class RecordingDriver: + def __init__(self, url=HOME, get_raises=None, handles=None, current_handle=MAIN_TAB): + self.current_url = url + self.visited = [] + self.get_raises = get_raises + self.window_handles = list(handles) if handles is not None else [MAIN_TAB] + self.current_window_handle = current_handle + + def get(self, url): + if self.get_raises is not None: + raise self.get_raises + + self.visited.append(url) + self.current_url = url + + +class RecordingTabUtils: + def __init__(self): + self.closes = 0 + self.focuses = 0 + self.kept = [] + + def close_all_other_tabs(self, exceptions=None): + self.closes += 1 + self.kept.append(exceptions) + + def ensure_focus(self): + self.focuses += 1 + + +class StubTasks(rewards_tasks.RewardsTaskUtils): + """RewardsTaskUtils with the six tasks replaced, and nothing else stubbed. + + complete_all_tasks and return_to_rewards_home are the real ones, which is + the whole point. + """ + + def __init__(self, driver, failures=None): + self.driver = driver + self.tab_utils = RecordingTabUtils() + self.main_window = MAIN_TAB + self.failures = failures or {} + self.ran = [] + + def _task(self, name, strands_browser_at=None): + self.ran.append(name) + + if name not in self.failures: + # A task that finishes switches back to the Rewards tab itself, so + # only a failure part way through leaves the browser elsewhere. + return + + if strands_browser_at is not None: + self.driver.current_url = strands_browser_at + + raise self.failures[name] + + def complete_bing_daily_set(self): + self._task("Bing daily set") + + def complete_explore_on_bing_tasks(self): + self._task("Explore on Bing", strands_browser_at=BING_RESULTS) + + def complete_visual_search(self): + self._task("Visual search", strands_browser_at=BING_RESULTS) + + def complete_misc_cards(self): + self._task("Misc cards") + + def complete_required_searches(self): + self._task("Required searches") + + def claim_bonus_points(self): + self._task("Bonus points") + + +ALL_TASKS = [ + "Bing daily set", "Explore on Bing", "Visual search", + "Misc cards", "Required searches", "Bonus points", +] + + +class FailedTaskRecoveryTests(unittest.TestCase): + def test_a_failure_on_a_bing_page_navigates_back(self): + driver = RecordingDriver() + tasks = StubTasks(driver, {"Visual search": TimeoutException("no file input")}) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING): + tasks.complete_all_tasks() + + self.assertEqual(driver.visited, [HOME]) + self.assertEqual(driver.current_url, HOME) + + def test_the_later_tasks_still_run(self): + # The point of the fix: one failure used to cost every task after it. + driver = RecordingDriver() + tasks = StubTasks(driver, {"Explore on Bing": TimeoutException("slow panel")}) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING): + tasks.complete_all_tasks() + + self.assertEqual(tasks.ran, ALL_TASKS) + + def test_a_skip_recovers_too(self): + # NoSuchElementException is the [SKIP] path rather than [FAIL], and it + # strands the browser just the same. + driver = RecordingDriver() + tasks = StubTasks(driver, {"Visual search": NoSuchElementException("no sidebar")}) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING): + tasks.complete_all_tasks() + + self.assertEqual(driver.visited, [HOME]) + + def test_a_clean_run_never_navigates(self): + # Six extra page loads a run would be a real cost for no benefit. + driver = RecordingDriver() + tasks = StubTasks(driver) + + tasks.complete_all_tasks() + + self.assertEqual(driver.visited, []) + self.assertEqual(tasks.ran, ALL_TASKS) + + def test_no_redundant_navigation_when_already_home(self): + # Plenty of failures happen on the Rewards page itself, e.g. a panel + # that never renders. Reloading it would only cost time. + driver = RecordingDriver() + tasks = StubTasks(driver, {"Misc cards": TimeoutException("no cards")}) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING): + tasks.complete_all_tasks() + + self.assertEqual(driver.visited, []) + + def test_tabs_are_still_tidied_after_a_failure(self): + driver = RecordingDriver() + tasks = StubTasks(driver, {"Visual search": TimeoutException("no file input")}) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING): + tasks.complete_all_tasks() + + self.assertEqual(tasks.tab_utils.closes, len(ALL_TASKS)) + + def test_the_rewards_tab_is_the_one_kept(self): + # The actual bug: close_all_other_tabs() with no argument keeps whatever + # is focused, and after a task dies on a Bing tab that is the Bing tab, + # so the cleanup closed the Rewards tab and kept the search results. + driver = RecordingDriver(handles=[MAIN_TAB, BING_TAB], current_handle=BING_TAB) + tasks = StubTasks(driver, {"Visual search": TimeoutException("no file input")}) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING): + tasks.complete_all_tasks() + + self.assertTrue(all(kept == [MAIN_TAB] for kept in tasks.tab_utils.kept)) + + +class RestoreMainTabTests(unittest.TestCase): + def test_names_the_main_tab_rather_than_the_focused_one(self): + driver = RecordingDriver(handles=[MAIN_TAB, BING_TAB], current_handle=BING_TAB) + tasks = StubTasks(driver) + + tasks.restore_main_tab() + + self.assertEqual(tasks.tab_utils.kept, [[MAIN_TAB]]) + + def test_falls_back_when_the_main_tab_is_gone(self): + # A task can close it, and a stale handle would only raise. + driver = RecordingDriver(handles=[BING_TAB], current_handle=BING_TAB) + tasks = StubTasks(driver) + + tasks.restore_main_tab() + + self.assertEqual(tasks.tab_utils.kept, [[BING_TAB]]) + + def test_no_windows_left_is_not_an_error(self): + driver = RecordingDriver(handles=[], current_handle=MAIN_TAB) + tasks = StubTasks(driver) + + tasks.restore_main_tab() + + self.assertEqual(tasks.tab_utils.kept, []) + + def test_a_failure_to_tidy_is_survivable(self): + driver = RecordingDriver(handles=[MAIN_TAB]) + tasks = StubTasks(driver) + + def boom(exceptions=None): + raise WebDriverException("browser gone") + + tasks.tab_utils.close_all_other_tabs = boom + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING) as logged: + tasks.restore_main_tab() + + self.assertTrue(any("Could not tidy" in line for line in logged.output)) + + +class ReturnToRewardsHomeTests(unittest.TestCase): + def test_navigates_and_refocuses(self): + driver = RecordingDriver(url=BING_RESULTS) + tasks = StubTasks(driver) + + tasks.return_to_rewards_home() + + self.assertEqual(driver.visited, [HOME]) + self.assertEqual(tasks.tab_utils.focuses, 1) + + def test_already_home_is_left_alone(self): + driver = RecordingDriver(url=HOME + "?foo=1") + tasks = StubTasks(driver) + + tasks.return_to_rewards_home() + + self.assertEqual(driver.visited, []) + self.assertEqual(tasks.tab_utils.focuses, 0) + + def test_a_driver_that_cannot_navigate_is_survivable(self): + # Recovery is best effort. Raising here would replace the real failure + # with the tidy-up's own, and take the rest of the run down with it. + driver = RecordingDriver(url=BING_RESULTS, get_raises=WebDriverException("browser gone")) + tasks = StubTasks(driver) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING) as logged: + tasks.return_to_rewards_home() + + self.assertTrue(any("Could not return" in line for line in logged.output)) + + def test_a_failing_recovery_does_not_end_the_run(self): + driver = RecordingDriver(get_raises=WebDriverException("browser gone")) + tasks = StubTasks(driver, {"Explore on Bing": TimeoutException("slow panel")}) + + with self.assertLogs(rewards_tasks.logger, level=logging.WARNING): + tasks.complete_all_tasks() + + self.assertEqual(tasks.ran, ALL_TASKS) + + +if __name__ == "__main__": + unittest.main()