Skip to content

test: remove external network dependency from problematic-links tests - #2146

Merged
vdusek merged 1 commit into
masterfrom
claude/fix-flaky-test-c4aca1
Aug 10, 2026
Merged

test: remove external network dependency from problematic-links tests#2146
vdusek merged 1 commit into
masterfrom
claude/fix-flaky-test-c4aca1

Conversation

@vdusek

@vdusek vdusek commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Problem

test_respect_robots_txt_with_problematic_links (BeautifulSoup, Parsel and Playwright variants) crawled two live
external hosts: it expected https://avatars.githubusercontent.com/apify to be visited successfully and
https://budplaceholder.com/ to fail. Whenever the runner could not reach GitHub's avatar CDN, the expected visit
never happened and the test failed — as it did on macOS / Python 3.14 in
this CI run, where the TCP connect
hit the navigation timeout and, with max_request_retries=0, visit was never called.

Solution

Both external hosts are replaced by local test servers, so the tests no longer touch the network:

  • no_robots_http_server / no_robots_server_url — a test server that responds 404 to /robots.txt, standing in
    for an origin without a robots.txt file.
  • unreachable_url — a loopback port bound for the whole session but never listened on, so every connection is
    refused immediately, with no DNS lookup and no external traffic.
  • The problematic-links page now templates both links from query parameters, because the ports are only known at
    runtime.

The behavior under test is unchanged: the mailto: link is skipped, an origin with an unavailable robots.txt is still
crawled, and an unreachable host reaches the failed_request_handler.

Verification

The failure reproduces deterministically with non-loopback DNS blocked (2 failed before, 0/100 after under the same
condition). Also 0/40 serial, the whole tests/unit/crawlers directory under -n auto, and the full unit suite.
Serving Disallow: / from the new server makes all seven variants fail, confirming the assertions still bite.

✍️ Drafted by Claude Code

@vdusek vdusek added t-tooling Issues with this label are in the ownership of the tooling team. adhoc Ad-hoc unplanned task added during the sprint. labels Aug 10, 2026
@vdusek vdusek self-assigned this Aug 10, 2026
@github-actions github-actions Bot added this to the 147th sprint - Tooling team milestone Aug 10, 2026
@github-actions github-actions Bot added the tested Temporary label used only programatically for some analytics. label Aug 10, 2026
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.57%. Comparing base (7fc9728) to head (8cbc851).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2146   +/-   ##
=======================================
  Coverage   93.56%   93.57%           
=======================================
  Files         181      181           
  Lines       12648    12648           
=======================================
+ Hits        11834    11835    +1     
+ Misses        814      813    -1     
Flag Coverage Δ
unit 93.57% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek
vdusek requested a review from Mantisus August 10, 2026 11:21
@vdusek
vdusek marked this pull request as ready for review August 10, 2026 11:21
@vdusek
vdusek requested review from Pijukatel and removed request for Mantisus August 10, 2026 11:21
@vdusek
vdusek merged commit 165223e into master Aug 10, 2026
36 checks passed
@vdusek
vdusek deleted the claude/fix-flaky-test-c4aca1 branch August 10, 2026 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants