Skip to content

Commit 00a9fca

Browse files
authored
test: Deflake adaptive crawler tests against slow browser navigation (#2221)
Two adaptive crawler tests flake on the Windows CI shard for the same reason. This supersedes #2207, which fixed one of them from a base that predates #2203. The browser sub crawler navigates under `PlaywrightCrawler`'s own `navigation_timeout`, a 60s `SharedTimeout` shared by the pre/post-navigation hooks and `goto`. That budget is separate from `request_handler_timeout`, because `BasicCrawler` applies the handler timeout only around the router call. So a cold Chromium launch on a saturated runner can blow the navigation budget while the handler timeout still has minutes left, and relaxing the handler timeout does nothing about it. - `test_adaptive_crawling_statistics` fails with `assert 3 == 1` ([example run](https://github.com/apify/crawlee-python/actions/runs/34120120329/job/101736011265)). When navigation exceeds the 60s budget, `BasicCrawler` retries the whole request, which is correct behavior, and every adaptive counter increments again. The failing run timed out twice before succeeding: 60 + 60 + 17s matches its 146s crawler runtime. It now passes a 5 minute `navigation_timeout`, the same ceiling #2203 already gave its handler. - `test_adaptive_playwright_crawler_timeout_in_sub_crawler` fails with `Expected 'browser_handler' to be called once. Called 0 times.` ([example run](https://github.com/apify/crawlee-python/actions/runs/33467016711/job/99728873095)). It sets `max_request_retries=0`, so one slow navigation fails the only attempt before the handler ever runs. It now passes a 120s `navigation_timeout`, matching the handler timeout it already relaxes. Both keep their existing assertions, so a real double-counting or missed-increment regression still fails them. Passing the key needs `navigation_timeout` in `_PlaywrightCrawlerAdditionalOptions`; `PlaywrightCrawler.__init__` already accepts it and `ty` reports `invalid-key` without it. Verified with deterministic fault injection in both cases. For the statistics test, monkeypatching `Page.goto` to stall the first two navigations by 70s reproduces the exact `assert 3 == 1` with two retries under the 60s ceiling, and passes in 71s under the 5 minute one with all counters at 1; plus 0 failures in 39 runs of that test alone and 31/31 for the whole file serially. For the sub crawler timeout test, a playwright-only pre-navigation delay of 70s consumes the shared budget and reproduces the CI failure under the default ceiling, passing under 120s, with 0/96 failures across 8 concurrent Chromium-saturated lanes afterwards. The adaptive module passes under `-n auto` in 6 of 7 runs; the exception hit an unrelated `Errno 98` in the uvicorn test server fixture, caused by running several pytest processes at once locally. *✍️ Drafted by Claude Code*
1 parent 9068dd4 commit 00a9fca

2 files changed

Lines changed: 7 additions & 2 deletions

File tree

src/crawlee/crawlers/_playwright/_playwright_crawler.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,9 @@ class _PlaywrightCrawlerAdditionalOptions(TypedDict):
630630
headless: NotRequired[bool]
631631
"""Whether to run the browser in headless mode. This option should not be used if `browser_pool` is provided."""
632632

633+
navigation_timeout: NotRequired[timedelta]
634+
"""Timeout for navigation (the process between opening a Playwright page and calling the request handler)."""
635+
633636

634637
class PlaywrightCrawlerOptions(
635638
_PlaywrightCrawlerAdditionalOptions,

tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,9 +492,9 @@ async def test_adaptive_crawling_statistics(test_urls: list[str]) -> None:
492492
crawler = AdaptivePlaywrightCrawler.with_beautifulsoup_static_parser(
493493
rendering_type_predictor=static_only_predictor_no_detection,
494494
result_checker=lambda result: False, # noqa: ARG005 # Intentionally unused argument.
495-
# Generous ceiling for each sub crawler run: a slow browser launch that exceeded it would make `BasicCrawler`
496-
# retry the request and increment every counter below once more.
495+
# Navigation has its own budget. Exceeding either one retries the request and re-increments every counter.
497496
request_handler_timeout=timedelta(minutes=5),
497+
playwright_crawler_specific_kwargs={'navigation_timeout': timedelta(minutes=5)},
498498
)
499499

500500
@crawler.router.default_handler
@@ -604,6 +604,8 @@ async def test_adaptive_playwright_crawler_timeout_in_sub_crawler(test_urls: lis
604604
max_request_retries=0,
605605
rendering_type_predictor=static_only_predictor_no_detection,
606606
request_handler_timeout=request_handler_timeout,
607+
# Navigation has its own budget, separate from the handler timeout, and no retry is left to absorb it.
608+
playwright_crawler_specific_kwargs={'navigation_timeout': timedelta(seconds=120)},
607609
)
608610
mocked_static_handler = Mock(name='static_handler')
609611
mocked_browser_handler = Mock(name='browser_handler')

0 commit comments

Comments
 (0)