Skip to content

refactor(test): improve type safety for spawned process URLs#7219

Merged
watson merged 2 commits intomasterfrom
watson/DEBUG-4402/spawn-proc-refactor
Jan 14, 2026
Merged

refactor(test): improve type safety for spawned process URLs#7219
watson merged 2 commits intomasterfrom
watson/DEBUG-4402/spawn-proc-refactor

Conversation

@watson
Copy link
Copy Markdown
Collaborator

@watson watson commented Jan 12, 2026

What does this PR do?

Refactors the spawnProc test helper to improve TypeScript type safety by splitting it into two distinct functions with clear lifecycle expectations and return types.

Motivation

When using spawnProc, TypeScript would complain that proc.url might be undefined, even in cases where we know the process is long-running and will definitely send a port message. This was because the function could return either SpawnedProcess (with url) or void depending on whether the process exited or stayed alive.

This refactoring:

  • Makes spawnProc always return SpawnedProcess with a guaranteed url property (rejects if process exits)
  • Adds spawnProcAndExpectExit for processes that should run and exit cleanly (returns void on success)
  • Implements url as a lazy getter that throws if accessed before the port message is received (prevents timing bugs)
  • Provides clear JSDoc documentation explaining when to use each function

This ensures TypeScript correctly understands that proc.url is always defined when using spawnProc, eliminating false-positive type errors.

Additional Notes

This is purely a refactoring of test infrastructure with no impact on production code.

Changes to test helpers:

  • Added spawnPluginIntegrationTestProcAndExpectExit wrapper for short-lived plugin integration tests

Updated tests:
Updated various plugin integration tests and other test files to use the appropriate spawn function based on whether they spawn long-running servers (use spawnProc/spawnPluginIntegrationTestProc) or short-lived scripts that run and exit (use spawnProcAndExpectExit/spawnPluginIntegrationTestProcAndExpectExit).

@watson watson self-assigned this Jan 12, 2026
Copy link
Copy Markdown
Collaborator Author

watson commented Jan 12, 2026

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.56%. Comparing base (3e204e4) to head (47e45ba).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7219   +/-   ##
=======================================
  Coverage   84.56%   84.56%           
=======================================
  Files         532      532           
  Lines       22656    22656           
=======================================
  Hits        19159    19159           
  Misses       3497     3497           

☔ 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 12, 2026

Overall package size

Self size: 4.39 MB
Deduped: 5.21 MB
No deduping: 5.21 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@datadog-datadog-prod-us1

This comment has been minimized.

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Jan 12, 2026

Benchmarks

Benchmark execution time: 2026-01-14 13:44:54

Comparing candidate commit 47e45ba in PR branch watson/DEBUG-4402/spawn-proc-refactor with baseline commit 3e204e4 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 289 metrics, 31 unstable metrics.

@watson watson force-pushed the watson/DEBUG-4402/spawn-proc-refactor branch 2 times, most recently from 3a0da08 to 09f5e10 Compare January 14, 2026 09:25
@watson watson marked this pull request as ready for review January 14, 2026 10:01
@watson watson requested review from a team as code owners January 14, 2026 10:01
@watson watson force-pushed the watson/DEBUG-4402/spawn-proc-refactor branch 2 times, most recently from 2fe3616 to 02625e4 Compare January 14, 2026 11:40
- Split `spawnProc` into two functions with distinct return types:
  - `spawnProc`: expects long-running processes, always returns SpawnedProcess with `url`
  - `spawnProcAndExpectExit`: expects clean exit, may return void
- Make `url` property a lazy getter that throws if accessed before port message
- Ensures TypeScript knows `proc.url` is always defined for long-running processes
- Update startup test to use `spawnProcAndExpectExit` for short-lived process
@watson watson force-pushed the watson/DEBUG-4402/spawn-proc-refactor branch from 02625e4 to dbfb772 Compare January 14, 2026 11:48
Copy link
Copy Markdown
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, as it already improves things further. I believe overload should just be added to all cases to remove even more type error reports.

Comment thread integration-tests/helpers/index.js Outdated
@watson watson merged commit 4f81eaf into master Jan 14, 2026
831 of 832 checks passed
@watson watson deleted the watson/DEBUG-4402/spawn-proc-refactor branch January 14, 2026 15:14
dd-octo-sts Bot pushed a commit that referenced this pull request Jan 15, 2026
- Split `spawnProc` into two functions with distinct return types:
  - `spawnProc`: expects long-running processes, always returns SpawnedProcess with `url`
  - `spawnProcAndExpectExit`: expects clean exit, may return void
- Make `url` property a lazy getter that throws if accessed before port message
- Ensures TypeScript knows `proc.url` is always defined for long-running processes
@dd-octo-sts dd-octo-sts Bot mentioned this pull request Jan 15, 2026
nina9753 pushed a commit that referenced this pull request Jan 15, 2026
- Split `spawnProc` into two functions with distinct return types:
  - `spawnProc`: expects long-running processes, always returns SpawnedProcess with `url`
  - `spawnProcAndExpectExit`: expects clean exit, may return void
- Make `url` property a lazy getter that throws if accessed before port message
- Ensures TypeScript knows `proc.url` is always defined for long-running processes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants