refactor(test): dynamic ports with fixture injection by vringar · Pull Request #1153 · openwpm/OpenWPM

vringar · 2026-04-04T21:11:26Z

Summary

Use OS-assigned ports (port=0) instead of hardcoded localhost:8000 to enable parallel test execution
Inject ServerUrls fixture via pytest, removing global state from test infrastructure
Fix test_get_with_popup_blocking NameError (missing server fixture)
Replace os.chdir() with SimpleHTTPRequestHandler(directory=) parameter
Fix FileNotFoundError in dump_page_source tests by using absolute paths

Supersedes #1136. Incorporates all fixes identified through adversarial review (VDD methodology).

VDD Review History

Round 1: Found NameError in popup test, dead code, os.chdir() hazard
Round 2: Found FileNotFoundError from incomplete os.chdir() removal → fixed with absolute paths
Final verdict: Begrudgingly Adequate (converged — only nitpicks remain)

Test plan

All test_simple_commands.py tests pass including test_dump_page_source_valid and test_recursive_dump_page_source_valid
test_get_with_popup_blocking no longer raises NameError
pre-commit run --all-files passes

Copilot

Pull request overview

This PR refactors the Python test HTTP server and related tests to use OS-assigned dynamic ports and pytest fixture injection, enabling parallel test execution and removing reliance on hardcoded localhost:8000 URLs / global state.

Changes:

Replace hardcoded server URLs with a ServerUrls dataclass provided via a session-scoped server fixture.
Update test pages (HTML/JS) to use root-relative paths instead of http://localhost:8000/....
Fix test path assumptions by removing os.chdir() usage and switching to absolute paths / handler directory=.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/utilities.py	Introduces `ServerUrls`, binds server to port 0, removes `os.chdir()`, adds caching header behavior.
test/conftest.py	Updates `server` fixture to yield `ServerUrls` and threads it through `task_manager_creator`.
test/openwpmtest.py	Injects `ServerUrls` into `OpenWPMTest` instances and uses it to build absolute URLs.
test/manual_test.py	Adapts manual helpers to new `start_server()` return signature and dynamic base URL.
test/test_xvfb_browser.py	Switches test URL construction to `server.base`.
test/test_timer.py	Switches test URL construction to `server.base`.
test/test_task_manager.py	Switches test URL construction to `server.base`.
test/test_storage_vectors.py	Uses `ServerUrls` for page URL; keeps host expectation based on `BASE_TEST_URL_DOMAIN`.
test/test_simple_commands.py	Removes global URL constants, builds URLs from `server.base`, fixes expected-source path handling.
test/test_profile.py	Switches profile-related navigation from global base URL to `server.base`.
test/test_js_instrument.py	Removes global TOP_URL constants; computes URLs from `self.server.base` at runtime.
test/test_http_instrumentation.py	Converts expected HTTP record sets into functions parameterized by `ServerUrls`; updates test URLs.
test/test_extension.py	Refactors expected URL-dependent call sets into helper functions parameterized by `server.base`.
test/test_dns_instrument.py	Uses dynamic port for `test.localhost`.
test/test_custom_function_command.py	Builds expected link set from dynamic `server.base`.
test/test_callstack_instrument.py	Builds expected stack traces from dynamic `server.base` and fixes row handling.
test/test_callback.py	Switches callback test URL construction to `server.base`.
test/test_pages/simple_a.html	Replaces absolute link to `localhost:8000` with root-relative `/test_pages/...`.
test/test_pages/expected_source.html	Updates expected HTML to match new root-relative link form.
test/test_pages/shared/worker.js	Replaces absolute fetch URL with root-relative `/test_pages/...`.
test/test_pages/shared/service_worker.js	Replaces absolute resource URL with root-relative `/test_pages/...`.
test/test_pages/http_service_worker_page.html	Replaces absolute fetch URL with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/parent.html	Replaces absolute iframe src URLs with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/child1a.html	Replaces absolute iframe src URLs with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/child1c.html	Replaces absolute iframe src URL with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/child2a.html	Replaces absolute iframe src URL with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/child2c.html	Replaces absolute iframe src URLs with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/child3a.html	Replaces absolute iframe src URL with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/child4a.html	Replaces absolute iframe src URL with root-relative `/test_pages/...`.
test/test_pages/recursive_iframes/child5a.html	Replaces absolute iframe src URL with root-relative `/test_pages/...`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-04T21:20:09Z

test/utilities.py

+    handler = lambda *args, **kwargs: MyHandler(*args, directory=test_dir, **kwargs)
+    server = MyTCPServer(("0.0.0.0", 0), handler)


socketserver.TCPServer is typed to take a RequestHandlerClass (a handler type), but start_server passes a lambda factory. This is likely to fail mypy (incompatible type) and also makes the handler less introspectable. Prefer defining an inner MyHandler subclass (or a small wrapper class) that hard-binds directory=test_dir and pass that class to MyTCPServer.

Suggested change

handler = lambda *args, **kwargs: MyHandler(*args, directory=test_dir, **kwargs)

server = MyTCPServer(("0.0.0.0", 0), handler)

class BoundMyHandler(MyHandler):

def __init__(self, *args, **kwargs):

super().__init__(*args, directory=test_dir, **kwargs)

server = MyTCPServer(("0.0.0.0", 0), BoundMyHandler)

codecov · 2026-04-04T21:29:47Z

Codecov Report

❌ Patch coverage is 5.55556% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.14%. Comparing base (a088fd3) to head (c2e5f58).

Files with missing lines	Patch %	Lines
test/test_profile.py	0.00%	18 Missing ⚠️
test/test_custom_function_command.py	0.00%	7 Missing ⚠️
test/test_task_manager.py	0.00%	5 Missing ⚠️
test/test_xvfb_browser.py	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1153      +/-   ##
==========================================
- Coverage   62.18%   62.14%   -0.04%     
==========================================
  Files          40       40              
  Lines        3898     3902       +4     
==========================================
+ Hits         2424     2425       +1     
- Misses       1474     1477       +3

☔ View full report in Codecov by Sentry.
📢 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.

…llelization

- CRITICAL: add missing server: ServerUrls fixture to test_get_with_popup_blocking and construct url_a/url_b dynamically instead of using undefined module-level vars - MAJOR: replace os.chdir() in start_server() with SimpleHTTPRequestHandler directory= parameter to avoid mutating global process state - Remove dead code: ServerUrls.url() and ServerUrls.base_noscheme (never used)

Run black on test_http_instrumentation.py and test_custom_function_command.py to satisfy pre-commit checks.

- test_dns_instrument: replace hardcoded port 8000 with server.port fixture in assertion and connection abort test - test_profile: use Path(__file__).parent for profile.tar.gz to avoid dependency on CWD being set to test/

Copilot AI review requested due to automatic review settings April 4, 2026 21:11

Copilot started reviewing on behalf of vringar April 4, 2026 21:12 View session

Copilot AI reviewed Apr 4, 2026

View reviewed changes

vringar and others added 5 commits April 7, 2026 23:36

refactor(test): use dynamic ports and fixture injection for test para…

2eec151

…llelization

fix(test): use absolute paths in dump_page_source tests [CL-1]

71cfa4b

style: fix black formatting in test files

5912c17

Run black on test_http_instrumentation.py and test_custom_function_command.py to satisfy pre-commit checks.

style: fix black/isort formatting [CL-1]

939c7e2

vringar force-pushed the fix/dynamic-test-ports-v2 branch from 9a71702 to 939c7e2 Compare April 7, 2026 23:37

vringar added 2 commits April 8, 2026 01:45

fix(test): fix mypy type error in dns instrument test

3769ebc

fix(test): fix dynamic port in dns test and profile path

c2e5f58

- test_dns_instrument: replace hardcoded port 8000 with server.port fixture in assertion and connection abort test - test_profile: use Path(__file__).parent for profile.tar.gz to avoid dependency on CWD being set to test/

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(test): dynamic ports with fixture injection#1153

refactor(test): dynamic ports with fixture injection#1153
vringar wants to merge 7 commits intomasterfrom
fix/dynamic-test-ports-v2

vringar commented Apr 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

codecov bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		handler = lambda args, kwargs: MyHandler(args, directory=test_dir, **kwargs)
		server = MyTCPServer(("0.0.0.0", 0), handler)

-    handler = lambda *args, **kwargs: MyHandler(*args, directory=test_dir, **kwargs)
-    server = MyTCPServer(("0.0.0.0", 0), handler)
+    class BoundMyHandler(MyHandler):
+        def __init__(self, *args, **kwargs):
+            super().__init__(*args, directory=test_dir, **kwargs)
+    server = MyTCPServer(("0.0.0.0", 0), BoundMyHandler)

Conversation

vringar commented Apr 4, 2026

Summary

VDD Review History

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Apr 4, 2026 •

edited

Loading