Skip to content

Commit 477d7ce

Browse files
committed
gh-109276: libregrtest: use separated file for JSON
libregrtest now uses a separated file descriptor to write test result as JSON. Previously, if a test wrote debug messages late around the JSON, the main test process failed to parse JSON. Rename TestResult.write_json() to TestResult.write_json_into(). worker_process() no longer writes an empty line at the end. There is no need to separate test process output from the JSON output anymore, since JSON is now written into a separated file descriptor.
1 parent 57b6205 commit 477d7ce

File tree

6 files changed

+54
-29
lines changed

6 files changed

+54
-29
lines changed

Lib/test/libregrtest/result.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def get_rerun_match_tests(self) -> FilterTuple | None:
156156
return None
157157
return tuple(match_tests)
158158

159-
def write_json(self, file) -> None:
159+
def write_json_into(self, file) -> None:
160160
json.dump(self, file, cls=_EncodeTestResult)
161161

162162
@staticmethod

Lib/test/libregrtest/run_workers.py

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from .runtests import RunTests
2121
from .single import PROGRESS_MIN_TIME
2222
from .utils import (
23-
StrPath, TestName,
23+
StrPath, StrJSON, TestName,
2424
format_duration, print_warning)
2525
from .worker import create_worker_process, USE_PROCESS_GROUP
2626

@@ -153,10 +153,11 @@ def mp_result_error(
153153
) -> MultiprocessResult:
154154
return MultiprocessResult(test_result, stdout, err_msg)
155155

156-
def _run_process(self, runtests: RunTests, output_file: TextIO,
156+
def _run_process(self, runtests: RunTests, output_fd: int, json_fd: int,
157157
tmp_dir: StrPath | None = None) -> int:
158158
try:
159-
popen = create_worker_process(runtests, output_file, tmp_dir)
159+
popen = create_worker_process(runtests, output_fd, json_fd,
160+
tmp_dir)
160161

161162
self._killed = False
162163
self._popen = popen
@@ -221,14 +222,23 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
221222
match_tests = self.runtests.get_match_tests(test_name)
222223
else:
223224
match_tests = None
224-
kwargs = {}
225-
if match_tests:
226-
kwargs['match_tests'] = match_tests
227-
worker_runtests = self.runtests.copy(tests=tests, **kwargs)
225+
err_msg = None
228226

229227
# gh-94026: Write stdout+stderr to a tempfile as workaround for
230228
# non-blocking pipes on Emscripten with NodeJS.
231-
with tempfile.TemporaryFile('w+', encoding=encoding) as stdout_file:
229+
with (tempfile.TemporaryFile('w+', encoding=encoding) as stdout_file,
230+
tempfile.TemporaryFile('w+', encoding='utf8') as json_file):
231+
stdout_fd = stdout_file.fileno()
232+
json_fd = json_file.fileno()
233+
234+
kwargs = {}
235+
if match_tests:
236+
kwargs['match_tests'] = match_tests
237+
worker_runtests = self.runtests.copy(
238+
tests=tests,
239+
json_fd=json_fd,
240+
**kwargs)
241+
232242
# gh-93353: Check for leaked temporary files in the parent process,
233243
# since the deletion of temporary files can happen late during
234244
# Python finalization: too late for libregrtest.
@@ -239,12 +249,14 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
239249
tmp_dir = tempfile.mkdtemp(prefix="test_python_")
240250
tmp_dir = os.path.abspath(tmp_dir)
241251
try:
242-
retcode = self._run_process(worker_runtests, stdout_file, tmp_dir)
252+
retcode = self._run_process(worker_runtests,
253+
stdout_fd, json_fd, tmp_dir)
243254
finally:
244255
tmp_files = os.listdir(tmp_dir)
245256
os_helper.rmtree(tmp_dir)
246257
else:
247-
retcode = self._run_process(worker_runtests, stdout_file)
258+
retcode = self._run_process(worker_runtests,
259+
stdout_fd, json_fd)
248260
tmp_files = ()
249261
stdout_file.seek(0)
250262

@@ -257,23 +269,27 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
257269
result = TestResult(test_name, state=State.MULTIPROCESSING_ERROR)
258270
return self.mp_result_error(result, err_msg=err_msg)
259271

272+
try:
273+
# deserialize run_tests_worker() output
274+
json_file.seek(0)
275+
worker_json: StrJSON = json_file.read()
276+
if worker_json:
277+
result = TestResult.from_json(worker_json)
278+
else:
279+
err_msg = f"empty JSON"
280+
except Exception as exc:
281+
# gh-101634: Catch UnicodeDecodeError if stdout cannot be
282+
# decoded from encoding
283+
err_msg = f"Fail to read or parser worker process JSON: {exc}"
284+
result = TestResult(test_name, state=State.MULTIPROCESSING_ERROR)
285+
return self.mp_result_error(result, stdout, err_msg=err_msg)
286+
260287
if retcode is None:
261288
result = TestResult(test_name, state=State.TIMEOUT)
262289
return self.mp_result_error(result, stdout)
263290

264-
err_msg = None
265291
if retcode != 0:
266292
err_msg = "Exit code %s" % retcode
267-
else:
268-
stdout, _, worker_json = stdout.rpartition("\n")
269-
stdout = stdout.rstrip()
270-
if not worker_json:
271-
err_msg = "Failed to parse worker stdout"
272-
else:
273-
try:
274-
result = TestResult.from_json(worker_json)
275-
except Exception as exc:
276-
err_msg = "Failed to parse worker JSON: %s" % exc
277293

278294
if err_msg:
279295
result = TestResult(test_name, state=State.MULTIPROCESSING_ERROR)

Lib/test/libregrtest/runtests.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class RunTests:
3636
gc_threshold: int | None = None
3737
use_resources: list[str] = dataclasses.field(default_factory=list)
3838
python_cmd: list[str] | None = None
39+
json_fd: int | None = None
3940

4041
def copy(self, **override):
4142
state = dataclasses.asdict(self)

Lib/test/libregrtest/single.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,5 +271,9 @@ def run_single_test(test_name: TestName, runtests: RunTests) -> TestResult:
271271
print(f"test {test_name} crashed -- {msg}",
272272
file=sys.stderr, flush=True)
273273
result.state = State.UNCAUGHT_EXC
274+
275+
sys.stdout.flush()
276+
sys.stderr.flush()
277+
274278
result.duration = time.perf_counter() - start_time
275279
return result

Lib/test/libregrtest/worker.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919

2020
def create_worker_process(runtests: RunTests,
21-
output_file: TextIO,
21+
output_fd: int, json_fd: int,
2222
tmp_dir: StrPath | None = None) -> subprocess.Popen:
2323
python_cmd = runtests.python_cmd
2424
worker_json = runtests.as_json()
@@ -43,11 +43,12 @@ def create_worker_process(runtests: RunTests,
4343
# sysconfig.is_python_build() is true. See issue 15300.
4444
kw = dict(
4545
env=env,
46-
stdout=output_file,
46+
stdout=output_fd,
4747
# bpo-45410: Write stderr into stdout to keep messages order
48-
stderr=output_file,
48+
stderr=output_fd,
4949
text=True,
5050
close_fds=(os.name != 'nt'),
51+
pass_fds=[json_fd],
5152
)
5253
if USE_PROCESS_GROUP:
5354
kw['start_new_session'] = True
@@ -58,6 +59,7 @@ def worker_process(worker_json: StrJSON) -> NoReturn:
5859
runtests = RunTests.from_json(worker_json)
5960
test_name = runtests.tests[0]
6061
match_tests: FilterTuple | None = runtests.match_tests
62+
json_fd: int = runtests.json_fd
6163

6264
setup_test_dir(runtests.test_dir)
6365
setup_process()
@@ -70,11 +72,10 @@ def worker_process(worker_json: StrJSON) -> NoReturn:
7072
print(f"Re-running {test_name} in verbose mode", flush=True)
7173

7274
result = run_single_test(test_name, runtests)
73-
print() # Force a newline (just in case)
7475

75-
# Serialize TestResult as dict in JSON
76-
result.write_json(sys.stdout)
77-
sys.stdout.flush()
76+
with open(json_fd, 'w', encoding='utf-8') as json_file:
77+
result.write_json_into(json_file)
78+
7879
sys.exit(0)
7980

8081

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
libregrtest now uses a separated file descriptor to write test result as JSON.
2+
Previously, if a test wrote debug messages late around the JSON, the main test
3+
process failed to parse JSON. Patch by Victor Stinner.

0 commit comments

Comments
 (0)