Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion .github/workflows/on-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,21 @@ jobs:
pip install uv
uv pip install invoke robotframework-ghareports --python 3.14 --system
invoke run-test-app-no-build --asynchronous
docker run -v ./atest/:/home/pwuser/test -t tidii:latest bash -c "robot -v SERVER:172.17.0.1:7272 --exclude no-docker-pr -L debug --outputdir /home/pwuser/output /home/pwuser/test"
set +e
docker run -v ./atest/:/home/pwuser/test -t tidii:latest bash -c "robot -v SERVER:172.17.0.1:7272 --exclude no-docker-pr -L debug --outputdir /home/pwuser/output /home/pwuser/test"
docker_status=$?
inv docker-copy-output
copy_status=$?
if [ "$docker_status" -ne 0 ]; then
exit "$docker_status"
fi
exit "$copy_status"

- uses: actions/upload-artifact@v7
if: ${{ always() }}
with:
name: docker_results
path: output_docker
- name: Github Job Summary
if: ${{ always() }}
run: |
Expand Down
68 changes: 57 additions & 11 deletions Browser/playwright.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class Playwright(LibraryComponent):
"""A wrapper for communicating with nodejs Playwright process."""

port: str | None
_node_dependencies_checked = False

def __init__(
self,
Expand All @@ -66,19 +67,38 @@ def __init__(
):
LibraryComponent.__init__(self, library)
self.enable_playwright_debug = enable_playwright_debug
self.ensure_node_dependencies()
self.host = str(host) if host else None
self.port = str(port) if port else None
self.ensure_node_dependencies()
self.playwright_log = playwright_log

@cached_property
def _playwright_process(self) -> Popen | None:
process = self.start_playwright()
atexit.register(self.close)
self.wait_until_server_up()
if platform.system() == "Darwin":
time.sleep(1) # To overcome problem with macOS Sonoma and hanging process
return process
max_attempts = 2 if platform.system() == "Darwin" else 1
last_error: RuntimeError | None = None
for attempt in range(1, max_attempts + 1):
process = self.start_playwright()
try:
self.wait_until_server_up()
atexit.register(self.close)
if platform.system() == "Darwin":
time.sleep(
1
) # To overcome problem with macOS Sonoma and hanging process
return process
except RuntimeError as err:
last_error = err
if process:
close_process_tree(process)
# Reset host/port so next attempt starts a fresh process on a fresh port.
self.host = None
self.port = None
if attempt < max_attempts:
logger.info(
"Retrying Playwright startup on macOS after initial startup failure"
)
time.sleep(0.25)
raise last_error if last_error else RuntimeError("Failed to start Playwright")

@cached_property
def _rfbrowser_dir(self) -> Path:
Expand All @@ -93,18 +113,43 @@ def ensure_node_dependencies(self):

If BrowserBatteries is installed, does nothing.
"""
if self.__class__._node_dependencies_checked:
return
if start_grpc_server is not None:
logger.trace(
"Running gRPC server from BrowserBatteries, no need to check node"
)
self.__class__._node_dependencies_checked = True
return
try:
run(["node", "-v"], stdout=DEVNULL, check=True)
except (CalledProcessError, FileNotFoundError, PermissionError) as err:

# If an external Playwright process is configured, this Python process
# acts only as a gRPC client and does not need to execute local `node`.
configured_port = getattr(self, "port", None)
if configured_port or os.environ.get("ROBOT_FRAMEWORK_BROWSER_NODE_PORT"):
logger.trace(
"Using external Playwright process, skipping local node dependency check"
)
return

node_check_error: (
CalledProcessError | FileNotFoundError | PermissionError | None
) = None
node_check_attempts = 3 if platform.system() == "Darwin" else 1
for attempt in range(node_check_attempts):
try:
run(["node", "-v"], stdout=DEVNULL, check=True)
node_check_error = None
break
except (CalledProcessError, FileNotFoundError, PermissionError) as err:
node_check_error = err
if attempt < node_check_attempts - 1:
time.sleep(0.2)

if node_check_error is not None:
raise RuntimeError(
"Couldn't execute node. Please ensure you have node.js installed and in PATH. "
"See https://nodejs.org/ for instructions. "
f"Original error is {err}"
f"Original error is {node_check_error}"
)

# This second application of .parent is necessary to find out that a developer setup has node_modules correctly
Expand All @@ -115,6 +160,7 @@ def ensure_node_dependencies(self):
(self._browser_wrapper_dir / "node_modules").is_dir(),
]
):
self.__class__._node_dependencies_checked = True
return

raise RuntimeError(
Expand Down
78 changes: 51 additions & 27 deletions atest/library/common.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os
from pathlib import Path
from subprocess import PIPE, STDOUT, Popen
from typing import Dict, NamedTuple
from subprocess import STDOUT, Popen
from typing import IO, Dict, NamedTuple
from urllib.parse import urlparse

from robot.api import logger
Expand All @@ -10,27 +10,40 @@
from Browser.utils import FormatterKeywords, close_process_tree, find_free_port

SERVERS: Dict = {}
LOG_FILES: Dict[str, IO] = {}


def _open_test_app_log(root_dir: Path, port: str) -> IO:
log_dir = root_dir / "atest" / "output" / "test-app"
log_dir.mkdir(parents=True, exist_ok=True)
return open(log_dir / f"test-app-{port}.log", "w", encoding="utf-8")


def parse_url(url: str) -> NamedTuple:
return urlparse(url)


def start_test_server():
global SERVERS
global SERVERS, LOG_FILES
port = str(find_free_port())
# For some reason, we need to have cwd at project root for the server to run properly.
root_dir = Path(os.path.dirname(__file__)) / ".." / ".."
root_dir = root_dir.resolve()
test_app_path = root_dir / "node" / "dynamic-test-app" / "dist" / "server.js"
print(test_app_path)
process = Popen(
["node", test_app_path, "-p", port],
stdout=PIPE,
stderr=STDOUT,
cwd=str(root_dir),
)
log_file = _open_test_app_log(root_dir, port)
try:
process = Popen(
["node", test_app_path, "-p", port],
stdout=log_file,
stderr=STDOUT,
cwd=str(root_dir),
)
except Exception:
log_file.close()
raise
SERVERS[port] = process
LOG_FILES[port] = log_file
return port


Expand Down Expand Up @@ -59,35 +72,46 @@ def start_test_https_server(
ca_cert_path = os.path.relpath(os.path.abspath(ca_cert_path), start=test_app_dir)

print(test_app_path)
process = Popen(
[
"node",
test_app_path,
"-p",
port,
"-c",
server_cert_path,
"-k",
server_key_path,
"-C",
ca_cert_path,
"-M" if mutual_tls else "-T",
],
text=True,
cwd=str(root_dir),
)
log_file = _open_test_app_log(root_dir, port)
try:
process = Popen(
[
"node",
test_app_path,
"-p",
port,
"-c",
server_cert_path,
"-k",
server_key_path,
"-C",
ca_cert_path,
"-M" if mutual_tls else "-T",
],
stdout=log_file,
stderr=STDOUT,
cwd=str(root_dir),
)
except Exception:
log_file.close()
raise
SERVERS[port] = process
LOG_FILES[port] = log_file
return port


def stop_test_server(port: str):
global SERVERS
global SERVERS, LOG_FILES
if port in SERVERS:
p: Popen = SERVERS[port]
close_process_tree(p)
del SERVERS[port]
else:
logger.warn(f"Server with port {port} not found")
if port in LOG_FILES:
LOG_FILES[port].flush()
LOG_FILES[port].close()
del LOG_FILES[port]
Comment on lines 105 to +114
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

In stop_test_server, if close_process_tree(p) raises (e.g., psutil AccessDenied/other unexpected errors), the corresponding log file handle in LOG_FILES will not be flushed/closed because that cleanup happens after the process shutdown block. Consider wrapping the process shutdown in a try/finally (or otherwise guaranteeing log-file cleanup) so log handles don’t leak on teardown failures.

Suggested change
if port in SERVERS:
p: Popen = SERVERS[port]
close_process_tree(p)
del SERVERS[port]
else:
logger.warn(f"Server with port {port} not found")
if port in LOG_FILES:
LOG_FILES[port].flush()
LOG_FILES[port].close()
del LOG_FILES[port]
try:
if port in SERVERS:
p: Popen = SERVERS[port]
close_process_tree(p)
del SERVERS[port]
else:
logger.warn(f"Server with port {port} not found")
finally:
if port in LOG_FILES:
LOG_FILES[port].flush()
LOG_FILES[port].close()
del LOG_FILES[port]

Copilot uses AI. Check for mistakes.


def get_current_scope_from_lib(keyword: FormatterKeywords) -> list:
Expand Down
6 changes: 4 additions & 2 deletions node/dynamic-test-app/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ import { Command } from 'commander';
import * as express from 'express';
import * as fs from 'fs';
import * as https from 'https';
import morgan from 'morgan';
import * as path from 'path';

const app = express.default();

// Configure morgan to write to a log file to avoid stdout blocking in Docker
const logStream = fs.createWriteStream(path.join(__dirname, '..', 'test-app.log'), { flags: 'a' });
app.use(morgan(':date[iso] :method :url :status :res[content-length] - :response-time ms', { stream: logStream }));
app.use(express.json());

const program = new Command();
Expand All @@ -31,9 +35,7 @@ const mutualTls: boolean = options.mutualTls;

app.set('etag', false);

// @ts-expect-error
app.get('/favicon.ico', (req, res) => res.status(204).send());
// @ts-expect-error
app.get('/dist/favicon.ico', (req, res) => res.status(204).send());

app.head('/api/get/json', (req, res) => {
Expand Down
Loading