bug fix to avoid swallowing exceptions/failed pages inside fetch_all_pages/small refactor
This commit is contained in:
1 parent
321546f9b9
commit
b071ac35bf
17 files changed
+309
-1650
No files matched your search
@@ -8,7 +8,11 @@ Mongo. The claim query sorts priority descending, so a priority of 0 means an
|
||||
alert is only drained once everything else is.
|
||||
"""
|
||||
|
||||
from monitor_utils.database import NO_MSRP_PRIORITY, calculate_priority
|
||||
from monitor_utils.database import (
|
||||
NEW_LISTING_PRIORITY,
|
||||
NO_MSRP_PRIORITY,
|
||||
calculate_priority,
|
||||
)
|
||||
|
||||
|
||||
def document(current, original, **overrides) -> dict:
|
||||
@@ -44,3 +48,21 @@ def test_broken_prices_still_outrank_nothing():
|
||||
assert calculate_priority(document(50.0, None, hasMsrp=False)) == 0
|
||||
assert calculate_priority(document(50.0, 0, hasMsrp=False)) == 0
|
||||
assert calculate_priority(document(None, 100.0, hasMsrp=False)) == 0
|
||||
|
||||
|
||||
def test_new_listing_is_floored_not_buried():
|
||||
"""A low-discount new listing sends regardless, so it must not sort last."""
|
||||
assert (
|
||||
calculate_priority(document(95.0, 100.0, newListing=True))
|
||||
== NEW_LISTING_PRIORITY
|
||||
)
|
||||
|
||||
|
||||
def test_new_listing_keeps_a_higher_percentage_rank():
|
||||
"""The floor is a floor: a 75%-off new listing still ranks as 75% off."""
|
||||
assert calculate_priority(document(25.0, 100.0, newListing=True)) == 7
|
||||
|
||||
|
||||
def test_repeat_alerts_keep_the_percentage_path():
|
||||
assert calculate_priority(document(95.0, 100.0, newListing=False)) == 0
|
||||
assert calculate_priority(document(95.0, 100.0)) == 0
|
||||
@@ -122,3 +122,73 @@ def test_past_prices_read_through():
|
||||
product = build_product_details(_doc(pastPrices=history), min_percentage=0)
|
||||
|
||||
assert product.pricing.past_prices == history
|
||||
|
||||
|
||||
# ----------------------------------------------------------- bypass_thresholds
|
||||
|
||||
|
||||
def test_bypass_builds_a_below_threshold_product():
|
||||
"""A new listing sends at any discount when the caller says so."""
|
||||
|
||||
doc = _doc(currentPrice=95.0, originalPrice=100.0) # 5%, way under 40
|
||||
product = build_product_details(doc, min_percentage=40, bypass_thresholds=True)
|
||||
assert product is not None
|
||||
assert product.metadata.percentage == 5
|
||||
|
||||
|
||||
def test_bypass_defaults_off():
|
||||
doc = _doc(currentPrice=95.0, originalPrice=100.0)
|
||||
assert build_product_details(doc, min_percentage=40) is None
|
||||
|
||||
|
||||
def test_bypass_does_not_rescue_broken_prices():
|
||||
"""The missing-price skip is a data guard, not a threshold."""
|
||||
|
||||
doc = _doc(currentPrice=95.0, originalPrice=None)
|
||||
assert build_product_details(doc, min_percentage=40, bypass_thresholds=True) is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------- newListing
|
||||
|
||||
|
||||
def test_new_listing_flag_reads_through():
|
||||
product = build_product_details(_doc(newListing=True), min_percentage=0)
|
||||
assert product.metadata.new_listing is True
|
||||
|
||||
|
||||
def test_legacy_docs_without_the_flag_build_as_none():
|
||||
"""Documents written before newListing existed must not KeyError."""
|
||||
|
||||
product = build_product_details(_doc(), min_percentage=0)
|
||||
assert product.metadata.new_listing is None
|
||||
|
||||
|
||||
# ------------------------------------------------------------ msrp-less guard
|
||||
|
||||
|
||||
def test_msrpless_doc_with_missing_original_price_skips_not_crashes():
|
||||
"""Regression: the guard lived inside the has_msrp branch, so this doc
|
||||
shape raised ValidationError (not an expected error) and killed the
|
||||
consumer."""
|
||||
|
||||
doc = _doc(hasMsrp=False, originalPrice=None)
|
||||
assert build_product_details(doc, min_percentage=40) is None
|
||||
|
||||
|
||||
def test_msrpless_doc_with_zero_original_price_skips():
|
||||
doc = _doc(hasMsrp=False, originalPrice=0)
|
||||
assert build_product_details(doc, min_percentage=40) is None
|
||||
|
||||
|
||||
def test_bypass_does_not_rescue_msrpless_broken_prices():
|
||||
doc = _doc(hasMsrp=False, originalPrice=None)
|
||||
assert build_product_details(doc, min_percentage=40, bypass_thresholds=True) is None
|
||||
|
||||
|
||||
def test_msrpless_doc_with_prices_builds_unthresholded():
|
||||
"""A priced MSRP-less doc still skips the threshold gate entirely."""
|
||||
|
||||
doc = _doc(hasMsrp=False, currentPrice=95.0, originalPrice=100.0) # 5% < 40
|
||||
product = build_product_details(doc, min_percentage=40)
|
||||
assert product is not None
|
||||
assert product.pricing.has_msrp is False
|
||||
@@ -0,0 +1,43 @@
|
||||
"""unit test: fetch_all_pages result accounting for any iterable input
|
||||
|
||||
run from the repo root:
|
||||
uv run pytest tests/test_fetch_all_pages.py
|
||||
|
||||
pure and offline — no HTTP. Regression: a generator page_range was exhausted
|
||||
building the tasks, so the result pairing dropped every failure and swallowed
|
||||
unexpected exceptions.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
from aioweb import AiowebError
|
||||
|
||||
from monitor_utils.runner import fetch_all_pages
|
||||
|
||||
|
||||
async def _failing_fetch(page: int):
|
||||
raise AiowebError(f"page {page} failed")
|
||||
|
||||
|
||||
async def _crashing_fetch(page: int):
|
||||
raise ValueError(f"page {page} crashed")
|
||||
|
||||
|
||||
def test_generator_input_still_reports_failed_pages():
|
||||
failed = asyncio.run(
|
||||
fetch_all_pages((p for p in [2, 3]), _failing_fetch, "cat", "page")
|
||||
)
|
||||
assert failed == [2, 3]
|
||||
|
||||
|
||||
def test_generator_input_still_raises_unexpected_errors():
|
||||
with pytest.raises(ValueError):
|
||||
asyncio.run(fetch_all_pages((p for p in [2]), _crashing_fetch, "cat", "page"))
|
||||
|
||||
|
||||
def test_range_input_reports_failed_pages():
|
||||
"""The re-iterable path the in-repo caller uses keeps working."""
|
||||
|
||||
failed = asyncio.run(fetch_all_pages(range(2, 4), _failing_fetch, "cat", "page"))
|
||||
assert failed == [2, 3]
|
||||
@@ -0,0 +1,119 @@
|
||||
"""unit test: add_products_to_database stamps newListing and copies it onto alerts
|
||||
|
||||
run from the repo root:
|
||||
uv run pytest tests/test_new_listing_stamp.py
|
||||
|
||||
offline — drives add_products_to_database against fake collections that record
|
||||
the UpdateOne ops instead of touching Mongo. The recorded ops are inspected via
|
||||
UpdateOne's private _doc/_filter, the same fields its own __eq__ compares.
|
||||
|
||||
The stamp is the database layer's job: only it knows whether the doc already
|
||||
exists, so monitors never set the flag. The alert carries its own copy because
|
||||
the product doc is rewritten every cycle (its flag flips False one cycle after
|
||||
discovery) while a discovery alert may outlive that in a backed-up queue.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
|
||||
from monitor_utils.database import add_products_to_database, build_alert
|
||||
from monitor_utils.models import PricingInfo, Product
|
||||
|
||||
from datetime import datetime, timezone
|
||||
|
||||
NOW = datetime(2026, 8, 10, tzinfo=timezone.utc)
|
||||
|
||||
|
||||
class FakeCursor:
|
||||
def __init__(self, docs):
|
||||
self._docs = list(docs)
|
||||
|
||||
def __aiter__(self):
|
||||
return self
|
||||
|
||||
async def __anext__(self):
|
||||
if not self._docs:
|
||||
raise StopAsyncIteration
|
||||
return self._docs.pop(0)
|
||||
|
||||
|
||||
class FakeCollection:
|
||||
"""Records bulk_write ops; doubles as its own .database for alerts lookup."""
|
||||
|
||||
def __init__(self, existing_docs=(), name="products"):
|
||||
self.name = name
|
||||
self._existing = list(existing_docs)
|
||||
self.bulk_writes = []
|
||||
self.database = self
|
||||
self.alerts = None
|
||||
|
||||
def __getitem__(self, name):
|
||||
if self.alerts is None:
|
||||
self.alerts = FakeCollection(name=name)
|
||||
return self.alerts
|
||||
|
||||
def find(self, _filter, _projection):
|
||||
return FakeCursor(self._existing)
|
||||
|
||||
async def bulk_write(self, ops, ordered=True):
|
||||
self.bulk_writes.append(list(ops))
|
||||
|
||||
|
||||
def _product(product_id="p1", current=95.0, original=100.0) -> Product:
|
||||
return Product(
|
||||
product_id=product_id,
|
||||
title="Widget",
|
||||
url="http://x/p1",
|
||||
pricing=PricingInfo(current_price=current, original_price=original),
|
||||
)
|
||||
|
||||
|
||||
def run(collection, products):
|
||||
asyncio.run(
|
||||
add_products_to_database(products, collection, "tools", "bestbuy")
|
||||
)
|
||||
|
||||
|
||||
def product_update(collection):
|
||||
return collection.bulk_writes[0][0]._doc
|
||||
|
||||
|
||||
def alert_doc(collection):
|
||||
return collection.alerts.bulk_writes[0][0]._doc["$setOnInsert"]
|
||||
|
||||
|
||||
def test_insert_stamps_new_listing_true_on_doc_and_alert():
|
||||
collection = FakeCollection()
|
||||
run(collection, [_product()])
|
||||
|
||||
assert product_update(collection)["$set"]["newListing"] is True
|
||||
assert alert_doc(collection)["newListing"] is True
|
||||
|
||||
|
||||
def test_existing_doc_with_new_price_stamps_false():
|
||||
existing = {"_id": "p1", "currentPrice": 100.0, "pastPrices": []}
|
||||
collection = FakeCollection([existing])
|
||||
run(collection, [_product(current=95.0)])
|
||||
|
||||
assert product_update(collection)["$set"]["newListing"] is False
|
||||
# a never-seen price still alerts, but not as a new listing
|
||||
assert alert_doc(collection)["newListing"] is False
|
||||
|
||||
|
||||
def test_existing_doc_with_known_price_emits_no_alert():
|
||||
existing = {
|
||||
"_id": "p1",
|
||||
"currentPrice": 95.0,
|
||||
"pastPrices": [{"price": 95.0, "date": NOW}],
|
||||
}
|
||||
collection = FakeCollection([existing])
|
||||
run(collection, [_product(current=95.0)])
|
||||
|
||||
assert product_update(collection)["$set"]["newListing"] is False
|
||||
assert collection.alerts is None
|
||||
|
||||
|
||||
def test_build_alert_defaults_missing_flag_to_false():
|
||||
"""Direct callers passing an unstamped document must not KeyError."""
|
||||
|
||||
alert = build_alert("bestbuy", {"_id": "p1"}, NOW)
|
||||
assert alert["newListing"] is False
|
||||
Reference in new issue
Block a user