Provide dynamic port assignments for wd_test sidecars (#3862)

* Provide dynamic port assignments for wd_test sidecars

* Update node:net and node:tls tests for dynamic ports

* Update WPT for dynamic ports

Author: npaun

build/BUILD.wpt

@@ -2,7 +2,6 @@
 # Licensed under the Apache 2.0 license found in the LICENSE file or at:
 #     https://opensource.org/licenses/Apache-2.0
 
-load("@bazel_skylib//rules:write_file.bzl", "write_file")
 load("@workerd//:build/wpt_get_directories.bzl", "wpt_get_directories")
 load("@workerd//:build/wpt_test.bzl", "wpt_module", "wpt_server_entrypoint")
 
@@ -24,20 +23,9 @@ load("@workerd//:build/wpt_test.bzl", "wpt_module", "wpt_server_entrypoint")
     name = dir,
 ) for dir in wpt_get_directories(root = "fetch")]
 
-write_file(
-    name = "gen_config_json",
-    out = "config.json",
-    content = [json.encode({
-        "server_host": "localhost",
-        "check_subdomains": False,
-    })],
-    visibility = ["//visibility:public"],
-)
-
 wpt_server_entrypoint(
     name = "entrypoint",
     srcs = ["wpt"] + glob(["**/*.py"]),
-    config_json = "config.json",
     python = "@python3_13_host//:python",
     visibility = ["//visibility:public"],
 )

build/wd_test.bzl

@@ -65,53 +65,35 @@ WINDOWS_TEMPLATE = """
 @echo off
 setlocal EnableDelayedExpansion
 
-REM Start sidecar if specified
-if not "%SIDECAR%" == "" (
-    start /b "" "%SIDECAR%" > nul 2>&1
-    set SIDECAR_PID=!ERRORLEVEL!
-    timeout /t 1 > nul
-)
-
-REM Run the actual test
-$(RUNTEST)
-
-REM Cleanup sidecar if it was started
-if defined SIDECAR_PID (
-    taskkill /F /PID !SIDECAR_PID! > nul 2>&1
+REM Run supervisor to start sidecar if specified
+if not "{sidecar}" == "" (
+    REM These environment variables are processed by the supervisor executable
+    set PORTS_TO_ASSIGN={port_bindings}
+    set SIDECAR_COMMAND="{sidecar}"
+    powershell -Command \"{supervisor} {runtest}\"
+) else (
+    {runtest}
 )
 
+set TEST_EXIT=!ERRORLEVEL!
 exit /b !TEST_EXIT!
 """
 
 SH_TEMPLATE = """#!/bin/sh
 set -e
 
-cleanup() {
-    if [ ! -z "$SIDECAR_PID" ]; then
-        kill $SIDECAR_PID 2>/dev/null || true
-    fi
-}
-
-trap cleanup EXIT
-
-# Start sidecar if specified
-if [ ! -z "$(SIDECAR)" ]; then
-    "$(SIDECAR)" & SIDECAR_PID=$!
-    # Wait until the process is ready
-    sleep 3
+# Run supervisor to start sidecar if specified
+if [ ! -z "{sidecar}" ]; then
+    # These environment variables are processed by the supervisor executable
+    PORTS_TO_ASSIGN={port_bindings} SIDECAR_COMMAND="{sidecar}" {supervisor} {runtest}
+else
+    {runtest}
 fi
-
-$(RUNTEST)
 """
 
-WINDOWS_RUNTEST_NORMAL = """
-powershell -Command \"%*\" `-dTEST_TMPDIR=$ENV:TEST_TMPDIR
-set TEST_EXIT=!ERRORLEVEL!
-"""
+WINDOWS_RUNTEST_NORMAL = """powershell -Command \"%*\" `-dTEST_TMPDIR=$ENV:TEST_TMPDIR"""
 
-SH_RUNTEST_NORMAL = """
-"$@" -dTEST_TMPDIR=$TEST_TMPDIR
-"""
+SH_RUNTEST_NORMAL = """"$@" -dTEST_TMPDIR=$TEST_TMPDIR"""
 
 # We need variants of the RUN_TEST command for Python memory snapshot tests. We have to invoke
 # workerd twice, once with --python-save-snapshot to produce the snapshot and once with
@@ -144,20 +126,30 @@ echo Using Python Snapshot
 def _wd_test_impl(ctx):
     is_windows = ctx.target_platform_has_constraint(ctx.attr._platforms_os_windows[platform_common.ConstraintValueInfo])
 
+    if ctx.file.sidecar and ctx.attr.python_snapshot_test:
+        # TODO(later): Implement support for generating a combined script with these two options
+        # if we have a test that requires it. Not doing it for now due to complexity.
+        return print("sidecar and python_snapshot_test currently cannot be used together")
+
     # Bazel insists that the rule must actually create the executable that it intends to run; it
     # can't just specify some other executable with some args. OK, fine, we'll use a script that
     # just execs its args.
     if is_windows:
         # Batch script executables must end with ".bat"
         executable = ctx.actions.declare_file("%s_wd_test.bat" % ctx.label.name)
-        content = WINDOWS_TEMPLATE.replace("$(SIDECAR)", ctx.file.sidecar.path if ctx.file.sidecar else "")
+        template = WINDOWS_TEMPLATE
         runtest = WINDOWS_RUNTEST_SNAPSHOT if ctx.attr.python_snapshot_test else WINDOWS_RUNTEST_NORMAL
-        content = content.replace("$(RUNTEST)", runtest)
     else:
         executable = ctx.outputs.executable
-        content = SH_TEMPLATE.replace("$(SIDECAR)", ctx.file.sidecar.short_path if ctx.file.sidecar else "")
+        template = SH_TEMPLATE
         runtest = SH_RUNTEST_SNAPSHOT if ctx.attr.python_snapshot_test else SH_RUNTEST_NORMAL
-        content = content.replace("$(RUNTEST)", runtest)
+
+    content = template.format(
+        sidecar = ctx.file.sidecar.short_path if ctx.file.sidecar else "",
+        runtest = runtest,
+        supervisor = ctx.file.sidecar_supervisor.short_path if ctx.file.sidecar_supervisor else "",
+        port_bindings = ",".join(ctx.attr.sidecar_port_bindings),
+    )
 
     ctx.actions.write(
         output = executable,
@@ -174,6 +166,13 @@ def _wd_test_impl(ctx):
         if default_runfiles:
             runfiles = runfiles.merge(default_runfiles)
 
+        runfiles = runfiles.merge(ctx.runfiles(files = [ctx.file.sidecar_supervisor]))
+
+        # Also merge the supervisor's own runfiles if it has any
+        default_runfiles = ctx.attr.sidecar_supervisor[DefaultInfo].default_runfiles
+        if default_runfiles:
+            runfiles = runfiles.merge(default_runfiles)
+
     return [
         DefaultInfo(
             executable = executable,
@@ -185,20 +184,42 @@ _wd_test = rule(
     implementation = _wd_test_impl,
     test = True,
     attrs = {
+        # The workerd executable is used to run all tests
         "workerd": attr.label(
             allow_single_file = True,
             executable = True,
             cfg = "exec",
             default = "//src/workerd/server:workerd",
         ),
-        "flags": attr.string_list(),
+        # A list of files that this test requires to be present in order to run.
         "data": attr.label_list(allow_files = True),
+        # If set, an executable that is run in parallel with the test, and provides some functionality
+        # needed for the test. This is usually a backend server, with workerd serving as the client.
+        # The sidecar will be killed once the test completes.
         "sidecar": attr.label(
             allow_single_file = True,
             executable = True,
             cfg = "exec",
         ),
+        # A list of binding names which will be filled in with random port numbers that the sidecar
+        # and test can use for communication. The test will only begin once the sidecar is
+        # listening to these ports.
+        #
+        # In the sidecar, access these bindings as environment variables. In the wd-test file, add
+        # fromEnvironment bindings to expose them to the test.
+        #
+        # Reminder: you'll also need to add a network = ( allow = ["private"] ) service as well.
+        "sidecar_port_bindings": attr.string_list(),
+        # An executable that is used to manage port assignments and child process creation when a
+        # sidecar is specified.
+        "sidecar_supervisor": attr.label(
+            allow_single_file = True,
+            executable = True,
+            cfg = "exec",
+            default = "//src/workerd/api/node:sidecar-supervisor",
+        ),
         "python_snapshot_test": attr.bool(),
+        # A reference to the Windows platform label, needed for the implementation of wd_test
         "_platforms_os_windows": attr.label(default = "@platforms//os:windows"),
     },
 )

build/wpt_test.bzl

@@ -52,6 +52,7 @@ def wpt_test(name, wpt_directory, config, autogates = [], start_server = False,
         name = "{}".format(name),
         src = wd_test_gen_rule,
         args = ["--experimental"],
+        sidecar_port_bindings = ["HTTP_PORT", "HTTPS_PORT"] if start_server else [],
         sidecar = "@wpt//:entrypoint" if start_server else None,
         data = [
             test_config_as_js,  # e.g. "url-test.js"
@@ -246,6 +247,8 @@ const unitTests :Workerd.Config = (
         bindings = [
           (name = "wpt", service = "wpt"),
           (name = "unsafe", unsafeEval = void),
+          (name = "HTTP_PORT", fromEnvironment = "HTTP_PORT"),
+          (name = "HTTPS_PORT", fromEnvironment = "HTTPS_PORT"),
           {bindings}
         ],
         compatibilityDate = embed "{compat_date}",
@@ -309,11 +312,20 @@ def generate_external_bindings(wd_test_file, base, files):
 ### (Create a single no-args script that starts the WPT server)
 
 WPT_ENTRYPOINT_SCRIPT_TEMPLATE = """
-# Make /usr/sbin/sysctl visibile (Python needs to call it on macOS)
+# Make /usr/sbin/sysctl visible (Python needs to call it on macOS)
 export PATH="$PATH:/usr/sbin"
 
 cd $(dirname $0)
-{python} wpt.py serve --config {config_json}
+{python} wpt.py serve --no-h2 --config /dev/stdin <<EOF
+{{
+  "server_host": "localhost",
+  "check_subdomains": false,
+  "ports": {{
+    "http": [$HTTP_PORT, "auto"],
+    "https": [$HTTPS_PORT, "auto"]
+  }}
+}}
+EOF
 """
 
 def _wpt_server_entrypoint_impl(ctx):
@@ -330,14 +342,13 @@ def _wpt_server_entrypoint_impl(ctx):
         is_executable = True,
         content = WPT_ENTRYPOINT_SCRIPT_TEMPLATE.format(
             python = ctx.file.python.short_path,
-            config_json = ctx.file.config_json.basename,
         ),
     )
 
     return DefaultInfo(
         runfiles = ctx.runfiles(
             files = [start_src],
-            transitive_files = depset(ctx.files.srcs + [ctx.file.python, ctx.file.config_json]),
+            transitive_files = depset(ctx.files.srcs + [ctx.file.python]),
         ),
         executable = start_src,
     )
@@ -349,8 +360,6 @@ wpt_server_entrypoint = rule(
         "python": attr.label(allow_single_file = True),
         # All the Python code that should be visible
         "srcs": attr.label_list(allow_files = True),
-        # Config file to pass to wpt serve
-        "config_json": attr.label(allow_single_file = True),
     },
 )
 

src/workerd/api/node/BUILD.bazel

@@ -318,11 +318,11 @@ wd_test(
     args = ["--experimental"],
     data = ["tests/net-nodejs-test.js"],
     sidecar = "net-nodejs-tcp-server",
-    target_compatible_with = select({
-        # TODO(soon): Investigate why this test timeout on Windows.
-        "@platforms//os:windows": ["@platforms//:incompatible"],
-        "//conditions:default": [],
-    }),
+    sidecar_port_bindings = [
+        "SERVER_PORT",
+        "ECHO_SERVER_PORT",
+        "TIMEOUT_SERVER_PORT",
+    ],
 )
 
 wd_test(
@@ -377,9 +377,14 @@ wd_test(
         "tests/tls-nodejs-test.js",
     ],
     sidecar = "tls-nodejs-tcp-server",
-    target_compatible_with = select({
-        # TODO(soon): Investigate why this test timeout on Windows.
-        "@platforms//os:windows": ["@platforms//:incompatible"],
-        "//conditions:default": [],
-    }),
+    sidecar_port_bindings = [
+        "ECHO_SERVER_PORT",
+        "HELLO_SERVER_PORT",
+    ],
+)
+
+js_binary(
+    name = "sidecar-supervisor",
+    entry_point = "tests/sidecar-supervisor.mjs",
+    visibility = ["//visibility:public"],
 )

src/workerd/api/node/tests/net-nodejs-tcp-server.js

@@ -7,13 +7,17 @@
 // We execute this command using Node.js, which makes net.createServer available.
 const net = require('node:net');
 
+function reportPort(server) {
+  console.info(`Listening on port ${server.address().port}`);
+}
+
 const server = net.createServer((s) => {
   s.on('error', () => {
     // Do nothing
   });
   s.end();
 });
-server.listen(9999, () => console.info('Listening on port 9999'));
+server.listen(process.env.SERVER_PORT, () => reportPort(server));
 
 const echoServer = net.createServer((s) => {
   s.setTimeout(100);
@@ -22,7 +26,7 @@ const echoServer = net.createServer((s) => {
   });
   s.pipe(s);
 });
-echoServer.listen(9998, () => console.info('Listening on port 9998'));
+echoServer.listen(process.env.ECHO_SERVER_PORT, () => reportPort(echoServer));
 
 const timeoutServer = net.createServer((s) => {
   s.setTimeout(100);
@@ -39,4 +43,6 @@ const timeoutServer = net.createServer((s) => {
     // Do nothing
   });
 });
-timeoutServer.listen(9997, () => console.info('Listening on port 9997'));
+timeoutServer.listen(process.env.TIMEOUT_SERVER_PORT, () =>
+  reportPort(timeoutServer)
+);