Address code review: refactor move_to_and_click retries, simplify wait_for_then_click, resolve conflicts with main

src/element_selectors.py

@@ -6,6 +6,20 @@ from selenium.common.exceptions import NoSuchElementException, StaleElementRefer
 from selenium import webdriver
 
 
+class ElementNotReady(NoSuchElementException):
+	"""The element is in the page but not usable yet.
+
+	A section this market does not ship and a section that has not finished
+	hydrating both reach the caller as NoSuchElementException, which is why a
+	run could report "not available in this UI variant" for something that was
+	on screen. They need different messages and different next steps, so the
+	second case gets its own type.
+
+	Subclassed rather than separate, so every existing `except
+	NoSuchElementException` keeps catching it.
+	"""
+
+
 class Labels:
 	"""Visible labels the selectors match on.
 
@@ -45,7 +59,9 @@ class ElementSelectionUtils:
 	   pick the copy that is visible and actually has content.
 
 	Anything the current variant does not ship raises NoSuchElementException so
-	the caller can skip that task instead of aborting the whole run.
+	the caller can skip that task instead of aborting the whole run. Something
+	that is present but not usable yet raises ElementNotReady instead, because
+	skipping it is the wrong answer and so is the message that goes with it.
 	"""
 
 	def __init__(self, driver: webdriver.Edge):
@@ -66,6 +82,11 @@ class ElementSelectionUtils:
 		their `.text` is empty, so returning one produces silent no-ops further
 		up. Raising instead lets the caller's WebDriverWait retry while the page
 		finishes hydrating.
+
+		The two failures are not the same finding. No element with the id means
+		this variant does not ship the section. An id that is there but has no
+		usable copy means it is still rendering, so that one raises
+		ElementNotReady.
 		"""
 		matches = self.driver.find_elements(By.ID, element_id)
 
@@ -79,7 +100,7 @@ class ElementSelectionUtils:
 			except StaleElementReferenceException:
 				continue
 
-		raise NoSuchElementException(
+		raise ElementNotReady(
 			f"{element_id!r} is present but no visible copy has content yet"
 		)
 

src/main.py

@@ -8,7 +8,11 @@ import rewards_tasks
 from selenium import webdriver
 from selenium.common.exceptions import SessionNotCreatedException
 
-HEADLESS = os.environ.get("REWARDS_HEADLESS", "").strip().lower() in ("1", "true", "yes")
+HEADLESS = os.environ.get("REWARDS_HEADLESS", "").strip().lower() in (
+	"1",
+	"true",
+	"yes",
+)
 
 logger = logging.getLogger(__name__)
 
@@ -17,7 +21,7 @@ def build_options(account: accounts.Account) -> webdriver.EdgeOptions:
 	options = webdriver.EdgeOptions()
 
 	options.add_experimental_option("excludeSwitches", ["enable-automation"])
-	options.add_experimental_option('useAutomationExtension', False)
+	options.add_experimental_option("useAutomationExtension", False)
 	options.add_argument("--disable-blink-features=AutomationControlled")
 	options.add_argument(f"--user-data-dir={account.user_data_dir}")
 	options.add_argument(f"--profile-directory={account.profile_name}")
@@ -45,7 +49,9 @@ def run_account(account: accounts.Account) -> bool:
 		# the profile nor the other window.
 		logger.error("[FAIL] %s: could not start Edge with this profile.", account.name)
 		logger.error("       profile directory: %s", account.user_data_dir)
-		logger.error("       The usual cause is that this profile is already open in another")
+		logger.error(
+			"       The usual cause is that this profile is already open in another"
+		)
 		logger.error("       Edge window, including one left over from a previous run.")
 		logger.error("       driver said: %s", log_utils.exception_summary(exc))
 
@@ -63,7 +69,8 @@ def run_account(account: accounts.Account) -> bool:
 			# own error, and the process it is meant to end is dead anyway.
 			logger.warning(
 				"%s: the driver did not shut down cleanly: %s",
-				account.name, log_utils.exception_summary(exc)
+				account.name,
+				log_utils.exception_summary(exc),
 			)
 
 	return True
@@ -97,8 +104,10 @@ def main() -> int:
 		except Exception as exc:
 			logger.error(
 				"[FAIL] %s: %s: %s",
-				account.name, type(exc).__name__, log_utils.exception_summary(exc),
-				exc_info=logger.isEnabledFor(logging.DEBUG)
+				account.name,
+				type(exc).__name__,
+				log_utils.exception_summary(exc),
+				exc_info=logger.isEnabledFor(logging.DEBUG),
 			)
 
 	if len(configured) > 1:

src/random_image_for_visual_search.py

@@ -489,11 +489,3 @@ def get_random_image():
 
 if __name__ == "__main__":
 	get_random_image()
-
-
-# ============================================================
-# MAIN
-# ============================================================
-
-if __name__ == "__main__":
-    get_random_image()

src/rewards_tasks.py

@@ -20,10 +20,62 @@ VISUAL_SEARCH_IMAGE_PATH = os.path.abspath("visual_search.jpg")
 
 logger = logging.getLogger(__name__)
 
+
+class ElementNeverAppeared(TimeoutException):
+	"""A wait expired without the element ever being in the page.
+
+	WebDriverWait reports only that the wait ran out, so a section this market
+	does not ship and a section that was on screen and slow arrived as the same
+	TimeoutException. Reporting both as "not available in this UI variant" was
+	wrong for the second one, which is what #52 describes.
+
+	Subclassed from TimeoutException so the handlers that already wait on a
+	control being absent, claim_bonus_points and complete_bing_daily_set, keep
+	working unchanged.
+	"""
+
+
+def task_failure_report(exc: BaseException) -> tuple[str, str]:
+	"""The tag and the reason a failed task is reported with.
+
+	Absence and an expired wait need different next steps. A section this market
+	does not ship is nothing to act on, so it stays a [SKIP]. A section that was
+	on the page and never became usable may have left points behind, so it is
+	reported as a failure instead of being folded into the same sentence.
+
+	Ordered from the most specific case outwards, not by exception hierarchy:
+	ElementNeverAppeared is a TimeoutException and ElementNotReady is a
+	NoSuchElementException, so each has to be tested before the class it
+	refines.
+	"""
+	unavailable = f"not available in this UI variant ({type(exc).__name__})"
+
+	if isinstance(exc, ElementNeverAppeared):
+		return "SKIP", unavailable
+
+	if isinstance(exc, (element_selectors.ElementNotReady, TimeoutException)):
+		return "FAIL", f"on the page but not ready in time ({type(exc).__name__})"
+
+	if isinstance(exc, NoSuchElementException):
+		return "SKIP", unavailable
+
+	return "FAIL", f"{type(exc).__name__}: {log_utils.exception_summary(exc)}"
+
+
 class RewardsTaskUtils:
 	def __init__(self, driver: webdriver.Edge):
 		self.driver = driver
 
+		# Set headers to spoof the rewards app for the rewards only quests
+		self.driver.execute_cdp_cmd("Network.enable", {})
+
+		headers = {
+			"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/151.0.0.0 Safari/537.36 Edg/151.0.0.0 MSRewards/Desktop/1.1.0",
+			"X-Rewards-Source": "msrewards-desktop",
+		}
+
+		self.driver.execute_cdp_cmd("Network.setExtraHTTPHeaders", {"headers": headers})
+
 		self.driver.get("https://rewards.bing.com/")
 
 		self.tab_utils = tab_utils.TabUtils(driver)
@@ -48,15 +100,45 @@ class RewardsTaskUtils:
 		return self.driver.find_element(By.XPATH, xpath)
 
 	def wait_for_element(self, element_getter: Callable[[], WebElement | list[WebElement]], timeout: int = 10) -> WebElement | list[WebElement]:
+		# Keep the last reason the getter gave. Without it a wait that expires
+		# cannot say whether the element was missing the whole time or was on
+		# the page and not ready, and those are reported differently.
+		last_error: BaseException | None = None
+
 		def condition(_: webdriver.Edge):
+			nonlocal last_error
+
 			try:
 				element_or_elements = element_getter()
+			except Exception as exc:
+				# Exception rather than a bare except, so Ctrl+C during a
+				# getter ends the run instead of being retried away.
+				last_error = exc
 
-				return element_or_elements
-			except:
 				return False
 
+			last_error = None
+
+			return element_or_elements
+
+		try:
 			return WebDriverWait(self.driver, timeout).until(condition)
+		except TimeoutException:
+			# A falsy return means the getter found something and rejected it,
+			# and ElementNotReady means it was there but still rendering. Only
+			# a plain NoSuchElementException every time means it was never
+			# there at all.
+			never_there = (
+				isinstance(last_error, NoSuchElementException)
+				and not isinstance(last_error, element_selectors.ElementNotReady)
+			)
+
+			if not never_there:
+				raise
+
+			raise ElementNeverAppeared(
+				f"nothing matched during the {timeout}s wait: {log_utils.exception_summary(last_error)}"
+			) from last_error
 
 	def switch_to_earn_page(self):
 		self.move_to_and_click(self.elements.get_earn_tab())
@@ -65,25 +147,25 @@ class RewardsTaskUtils:
 		self.move_to_and_click(self.elements.get_dashboard_tab())
 
 	def move_to_and_click(self, elem_or_getter: WebElement | Callable[[], WebElement], retries: int = 3):
+		if callable(elem_or_getter):
 			for attempt in range(retries):
 				try:
-				if callable(elem_or_getter):
 					target_elem = elem_or_getter()
-				else:
-					target_elem = elem_or_getter
-
 					self.mouse.move_to_element(target_elem)
 					self.mouse.human_like_click()
 					return
 				except StaleElementReferenceException as exc:
 					if attempt == retries - 1:
 						raise exc
-				print(f"[WARNING] StaleElementReferenceException during click attempt {attempt + 1}/{retries}, retrying...")
+					logger.warning("StaleElementReferenceException during click attempt %d/%d, retrying...", attempt + 1, retries)
 					time.sleep(0.5)
+		else:
+			self.mouse.move_to_element(elem_or_getter)
+			self.mouse.human_like_click()
 
 	def wait_for_then_click(self, element_getter: Callable[[], WebElement], timeout: int = 10):
-		elem = self.wait_for_element(element_getter, timeout)
-		self.move_to_and_click(element_getter if callable(element_getter) else elem)
+		self.wait_for_element(element_getter, timeout)
+		self.move_to_and_click(element_getter)
 
 	def complete_bing_daily_set(self, expected_activities: int = 3):
 		self.switch_to_earn_page()
@@ -347,11 +429,12 @@ class RewardsTaskUtils:
 			try:
 				step()
 				logger.info("[OK] %s", name)
-			except (NoSuchElementException, TimeoutException) as exc:
-				logger.warning("[SKIP] %s: not available in this UI variant (%s)", name, type(exc).__name__)
 			except Exception as exc:
-				logger.error(
-					"[FAIL] %s: %s: %s", name, type(exc).__name__, log_utils.exception_summary(exc),
+				tag, reason = task_failure_report(exc)
+
+				logger.log(
+					logging.WARNING if tag == "SKIP" else logging.ERROR,
+					"[%s] %s: %s", tag, name, reason,
 					exc_info=logger.isEnabledFor(logging.DEBUG)
 				)
 

tests/test_element_selectors.py

@@ -163,6 +163,19 @@ class DuplicatedContainer(unittest.TestCase):
 
 		self.assertEqual(len(cards), 7)
 
+	def test_a_container_that_is_there_but_empty_is_not_reported_as_missing(self):
+		# Both failures used to raise NoSuchElementException, so a section that
+		# was on the page and still rendering got reported as one this market
+		# does not ship. Waiting is the answer to this one.
+		with self.assertRaises(element_selectors.ElementNotReady):
+			selectors_for(self._driver(visible_links=0, hidden_links=7)).get_all_misc_cards()
+
+	def test_a_container_that_is_absent_is_reported_as_missing(self):
+		with self.assertRaises(NoSuchElementException) as caught:
+			selectors_for(FakeDriver()).get_all_misc_cards()
+
+		self.assertNotIsInstance(caught.exception, element_selectors.ElementNotReady)
+
 
 class DailySetOpener(unittest.TestCase):
 	"""The opener label has to be distinguished from the level up entry."""

tests/test_task_outcomes.py

@@ -0,0 +1,262 @@
+"""Tests for what a run says about a task that did not complete.
+
+The reported reason used to be a guess. Every wait that expired and every
+lookup that missed produced "not available in this UI variant", so a section
+that was on the page and slow read exactly like one this market does not ship,
+which is #52. These pin down which failures are absence and which are not.
+
+None of them need a browser.
+
+	python -m unittest discover -s tests
+"""
+
+import logging
+import os
+import sys
+import types
+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
+from element_selectors import ElementNotReady
+from fakes import FakeDriver
+from rewards_tasks import ElementNeverAppeared, task_failure_report
+
+# Long enough for one poll, short enough that the suite stays quick.
+# WebDriverWait sleeps 0.5s between attempts, so a wait that expires costs
+# about that regardless of the timeout asked for.
+BRIEF = 0.05
+
+
+def make_tasks():
+	"""A RewardsTaskUtils without the browser its __init__ opens."""
+	tasks = rewards_tasks.RewardsTaskUtils.__new__(rewards_tasks.RewardsTaskUtils)
+
+	tasks.driver = FakeDriver()
+	tasks.tab_utils = types.SimpleNamespace(close_all_other_tabs=lambda: None)
+
+	return tasks
+
+
+class WaitClassification(unittest.TestCase):
+	"""wait_for_element has to say why it gave up, not just that it did."""
+
+	def test_a_getter_that_never_finds_anything_is_absence(self):
+		def missing():
+			raise NoSuchElementException("no button containing 'visual search streak'")
+
+		with self.assertRaises(ElementNeverAppeared):
+			make_tasks().wait_for_element(missing, timeout=BRIEF)
+
+	def test_a_section_that_is_still_rendering_is_not_absence(self):
+		# The id is in the page, no visible copy has content yet. Waiting
+		# longer is the answer here, skipping the task is not.
+		def not_ready():
+			raise ElementNotReady("'moreactivities' is present but no visible copy has content yet")
+
+		with self.assertRaises(TimeoutException) as caught:
+			make_tasks().wait_for_element(not_ready, timeout=BRIEF)
+
+		self.assertNotIsInstance(caught.exception, ElementNeverAppeared)
+
+	def test_a_getter_that_rejects_what_it_finds_is_not_absence(self):
+		# complete_bing_daily_set holds out for all three activities and
+		# returns False until they are there. The panel itself is open.
+		with self.assertRaises(TimeoutException) as caught:
+			make_tasks().wait_for_element(lambda: [], timeout=BRIEF)
+
+		self.assertNotIsInstance(caught.exception, ElementNeverAppeared)
+
+	def test_an_element_that_arrives_late_is_still_returned(self):
+		attempts = []
+
+		def slow():
+			attempts.append(None)
+
+			if len(attempts) < 2:
+				raise NoSuchElementException("not yet")
+
+			return "the element"
+
+		self.assertEqual(make_tasks().wait_for_element(slow, timeout=5), "the element")
+		# A single lucky first attempt would prove nothing about the retry.
+		self.assertGreater(len(attempts), 1)
+
+	def test_the_getters_own_reason_survives(self):
+		def missing():
+			raise NoSuchElementException("no button containing 'points breakdown'")
+
+		with self.assertRaises(ElementNeverAppeared) as caught:
+			make_tasks().wait_for_element(missing, timeout=BRIEF)
+
+		self.assertIn("points breakdown", str(caught.exception))
+
+
+class ExistingTimeoutHandlers(unittest.TestCase):
+	"""The new type has to stay catchable where TimeoutException was."""
+
+	def test_it_is_still_a_timeout(self):
+		self.assertTrue(issubclass(ElementNeverAppeared, TimeoutException))
+
+	def test_having_no_bonus_points_is_still_only_a_warning(self):
+		# There is no Claim button when there is nothing to claim, so this
+		# path reaches the wait expecting to be disappointed. If the new type
+		# escaped its `except TimeoutException`, an ordinary run would start
+		# reporting a failed task every day.
+		def bonus_button():
+			pass
+
+		def claim_button():
+			pass
+
+		tasks = make_tasks()
+		tasks.switch_to_dashboard = lambda: None
+		tasks.elements = types.SimpleNamespace(
+			get_bonus_button_on_dashboard=bonus_button,
+			get_claim_bonus_points_button=claim_button,
+		)
+
+		def wait_for_then_click(getter, timeout=10):
+			if getter is claim_button:
+				raise ElementNeverAppeared("nothing matched during the 10s wait")
+
+		tasks.wait_for_then_click = wait_for_then_click
+
+		with self.assertLogs(rewards_tasks.logger, level=logging.WARNING) as captured:
+			tasks.claim_bonus_points()
+
+		self.assertIn("no bonus points to claim", "\n".join(captured.output).lower())
+
+
+class FailureReport(unittest.TestCase):
+	def test_a_section_this_variant_does_not_ship_is_skipped(self):
+		tag, reason = task_failure_report(
+			NoSuchElementException("no element with id 'moreactivities'")
+		)
+
+		self.assertEqual(tag, "SKIP")
+		self.assertIn("not available in this UI variant", reason)
+
+	def test_a_wait_that_never_saw_the_element_is_skipped(self):
+		tag, reason = task_failure_report(ElementNeverAppeared("nothing matched"))
+
+		self.assertEqual(tag, "SKIP")
+		self.assertIn("not available in this UI variant", reason)
+
+	def test_a_section_that_never_finished_rendering_is_not_skipped(self):
+		tag, reason = task_failure_report(
+			ElementNotReady("'moreactivities' is present but no visible copy has content yet")
+		)
+
+		self.assertEqual(tag, "FAIL")
+		self.assertNotIn("not available", reason)
+
+	def test_an_expired_wait_is_not_skipped(self):
+		# The line in #52, reported for a panel that was on screen the whole
+		# time: "[SKIP] Required searches: not available in this UI variant
+		# (TimeoutException)".
+		tag, reason = task_failure_report(TimeoutException("Message: "))
+
+		self.assertEqual(tag, "FAIL")
+		self.assertNotIn("not available", reason)
+
+	def test_the_exception_name_is_kept(self):
+		# It is the difference between a lookup that missed and a wait that
+		# expired, and someone pasting a log should not lose it.
+		self.assertIn("ElementNeverAppeared", task_failure_report(ElementNeverAppeared("x"))[1])
+		self.assertIn("TimeoutException", task_failure_report(TimeoutException("x"))[1])
+
+	def test_anything_else_keeps_its_own_message(self):
+		tag, reason = task_failure_report(WebDriverException("chrome not reachable"))
+
+		self.assertEqual(tag, "FAIL")
+		self.assertIn("WebDriverException", reason)
+		self.assertIn("chrome not reachable", reason)
+
+
+class TaskLoop(unittest.TestCase):
+	"""complete_all_tasks, with the six tasks replaced by recorded calls."""
+
+	STEPS = (
+		("Bing daily set", "complete_bing_daily_set"),
+		("Explore on Bing", "complete_explore_on_bing_tasks"),
+		("Visual search", "complete_visual_search"),
+		("Misc cards", "complete_misc_cards"),
+		("Required searches", "complete_required_searches"),
+		("Bonus points", "claim_bonus_points"),
+	)
+
+	def setUp(self):
+		self.ran = []
+
+	def _tasks(self, failures=None):
+		failures = failures or {}
+		tasks = make_tasks()
+
+		for _, attribute in self.STEPS:
+			def step(name=attribute):
+				self.ran.append(name)
+
+				if name in failures:
+					raise failures[name]
+
+			setattr(tasks, attribute, step)
+
+		return tasks
+
+	def _run(self, failures=None):
+		with self.assertLogs(rewards_tasks.logger, level=logging.INFO) as captured:
+			self._tasks(failures).complete_all_tasks()
+
+		return "\n".join(captured.output)
+
+	def test_a_failing_task_does_not_stop_the_ones_after_it(self):
+		self._run({"complete_visual_search": WebDriverException("chrome not reachable")})
+
+		self.assertEqual(self.ran, [attribute for _, attribute in self.STEPS])
+
+	def test_absence_and_an_expired_wait_read_differently(self):
+		output = self._run({
+			"complete_visual_search": ElementNeverAppeared("nothing matched"),
+			"complete_required_searches": TimeoutException("Message: "),
+		})
+
+		self.assertIn("[SKIP] Visual search: not available in this UI variant", output)
+		self.assertIn("[FAIL] Required searches: on the page but not ready in time", output)
+		self.assertIn("[OK] Bing daily set", output)
+
+	def test_a_section_that_never_rendered_is_not_called_unavailable(self):
+		output = self._run({
+			"complete_misc_cards": ElementNotReady(
+				"'moreactivities' is present but no visible copy has content yet"
+			),
+		})
+
+		self.assertIn("[FAIL] Misc cards: on the page but not ready in time", output)
+		self.assertNotIn("Misc cards: not available", output)
+
+	def test_a_task_this_variant_does_not_ship_is_still_skipped(self):
+		output = self._run({
+			"complete_explore_on_bing_tasks": NoSuchElementException(
+				"no Explore on Bing section in this UI variant"
+			),
+		})
+
+		self.assertIn("[SKIP] Explore on Bing: not available in this UI variant", output)
+
+	def test_an_unexpected_failure_still_reports_what_went_wrong(self):
+		output = self._run({"complete_misc_cards": WebDriverException("chrome not reachable")})
+
+		self.assertIn("[FAIL] Misc cards: WebDriverException", output)
+		self.assertIn("chrome not reachable", output)
+
+
+if __name__ == "__main__":
+	unittest.main()