10000 gh-109276: libregrtest: use separated file for JSON (#109277) · python/cpython@de5f8f7 · GitHub
[go: up one dir, main page]
Skip to content

Commit de5f8f7

Browse files
vstinnervstinner
authored
gh-109276: libregrtest: use separated file for JSON (#109277)
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. create_worker_process() now always spawn the process with close_fds=True.
1 parent baa6dc8 commit de5f8f7

File tree

6 files changed

+84
-36
lines changed

6 files changed

+84
-36
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: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@
2020
from .runtests import RunTests
2121
from .single import PROGRESS_MIN_TIME
2222
from .utils import (
23-
StrPath, TestName,
23+
StrPath, StrJSON, TestName, MS_WINDOWS,
2424
format_duration, print_warning)
2525
from .worker import create_worker_process, USE_PROCESS_GROUP
2626

27-
if sys.platform == 'win32':
27+
if MS_WINDOWS:
2828
import locale
29+
import msvcrt
30+
2931

3032

3133
# Display the running tests if nothing happened last N seconds
@@ -153,10 +155,11 @@ def mp_result_error(
153155
) -> MultiprocessResult:
154156
return MultiprocessResult(test_result, stdout, err_msg)
155157

156-
def _run_process(self, runtests: RunTests, output_file: TextIO,
158+
def _run_process(self, runtests: RunTests, output_fd: int, json_fd: int,
157159
tmp_dir: StrPath | None = None) -> int:
158160
try:
159-
popen = create_worker_process(runtests, output_file, tmp_dir)
161+
popen = create_worker_process(runtests, output_fd, json_fd,
162+
tmp_dir)
160163

161164
self._killed = False
162165
self._popen = popen
@@ -208,7 +211,7 @@ def _run_process(self, runtests: RunTests, output_file: TextIO,
208211
def _runtest(self, test_name: TestName) -> MultiprocessResult:
209212
self.current_test_name = test_name
210213

211-
if sys.platform == 'win32':
214+
if MS_WINDOWS:
212215
# gh-95027: When stdout is not a TTY, Python uses the ANSI code
213216
# page for the sys.stdout encoding. If the main process runs in a
214217
# terminal, sys.stdout uses WindowsConsoleIO with UTF-8 encoding.
@@ -221,14 +224,25 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
221224
match_tests = self.runtests.get_match_tests(test_name)
222225
else:
223226
match_tests = None
224-
kwargs = {}
225-
if match_tests:
226-
kwargs['match_tests'] = match_tests
227-
worker_runtests = self.runtests.copy(tests=tests, **kwargs)
227+
err_msg = None
228228

229229
# gh-94026: Write stdout+stderr to a tempfile as workaround for
230230
# non-blocking pipes on Emscripten with NodeJS.
231-
with tempfile.TemporaryFile('w+', encoding=encoding) as stdout_file:
231+
with (tempfile.TemporaryFile('w+', encoding=encoding) as stdout_file,
232+
tempfile.TemporaryFile('w+', encoding='utf8') as json_file):
233+
stdout_fd = stdout_file.fileno()
234+
json_fd = json_file.fileno()
235+
if MS_WINDOWS:
236+
json_fd = msvcrt.get_osfhandle(json_fd)
237+
238+
kwargs = {}
239+
if match_tests:
240+
kwargs['match_tests'] = match_tests
241+
worker_runtests = self.runtests.copy(
242+
tests=tests,
243+
json_fd=json_fd,
244+
**kwargs)
245+
232246
# gh-93353: Check for leaked temporary files in the parent process,
233247
# since the deletion of temporary files can happen late during
234248
# Python finalization: too late for libregrtest.
@@ -239,12 +253,14 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
239253
tmp_dir = tempfile.mkdtemp(prefix="test_python_")
240254
tmp_dir = os.path.abspath(tmp_dir)
241255
try:
242-
retcode = self._run_process(worker_runtests, stdout_file, tmp_dir)
256+
retcode = self._run_process(worker_runtests,
257+
stdout_fd, json_fd, tmp_dir)
243258
finally:
244259
tmp_files = os.listdir(tmp_dir)
245260
os_helper.rmtree(tmp_dir)
246261
else:
247-
retcode = self._run_process(worker_runtests, stdout_file)
262+
retcode = self._run_process(worker_runtests,
263+
stdout_fd, json_fd)
248264
tmp_files = ()
249265
stdout_file.seek(0)
250266

@@ -257,23 +273,27 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
257273
result = TestResult(test_name, state=State.MULTIPROCESSING_ERROR)
258274
return self.mp_result_error(result, err_msg=err_msg)
259275

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

264-
err_msg = None
265295
if retcode != 0:
266296
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
277297

278298
if err_msg:
279299
result = TestResult(test_name, state=State.MULTIPROCESSING_ERROR)

‎Lib/test/libregrtest/runtests.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ 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+
# On Unix, it's a file descriptor.
40+
# On Windows, it's a handle.
41+
json_fd: int | None = None
3942

4043
def copy(self, **override):
4144
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: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@
1010
from .runtests import RunTests
1111
from .single import run_single_test
1212
from .utils import (
13-
StrPath, StrJSON, FilterTuple,
13+
StrPath, StrJSON, FilterTuple, MS_WINDOWS,
1414
get_work_dir, exit_timeout)
1515

1616

1717
USE_PROCESS_GROUP = (hasattr(os, "setsid") and hasattr(os, "killpg"))
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()
@@ -41,23 +41,42 @@ def create_worker_process(runtests: RunTests,
4141
# Running the child from the same working directory as regrtest's original
4242
# invocation ensures that TEMPDIR for the child is the same when
4343
# sysconfig.is_python_build() is true. See issue 15300.
44-
kw = dict(
44+
kwargs = 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,
50-
close_fds=(os.name != 'nt'),
50+
close_fds=True,
5151
)
52+
if not MS_WINDOWS:
53+
kwargs['pass_fds'] = [json_fd]
54+
else:
55+
startupinfo = subprocess.STARTUPINFO()
56+
startupinfo.lpAttributeList = {"handle_list": [json_fd]}
57+
kwargs['startupinfo'] = startupinfo
5258
if USE_PROCESS_GROUP:
53-
kw['start_new_session'] = True
54-
return subprocess.Popen(cmd, **kw)
59+
kwargs['start_new_session'] = True
60+
61+
if MS_WINDOWS:
62+
os.set_handle_inheritable(json_fd, True)
63+
try:
64+
return subprocess.Popen(cmd, **kwargs)
65+
finally:
66+
if MS_WINDOWS:
67+
os.set_handle_inheritable(json_fd, False)
5568

5669

5770
def worker_process(worker_json: StrJSON) -> NoReturn:
5871
runtests = RunTests.from_json(worker_json)
5972
test_name = runtests.tests[0]
6073
match_tests: FilterTuple | None = runtests.match_tests
74+
json_fd: int = runtests.json_fd
75+
76+
if MS_WINDOWS:
77+
import msvcrt
78+
json_fd = msvcrt.open_osfhandle(json_fd, os.O_WRONLY)
79+
6180

6281
setup_test_dir(runtests.test_dir)
6382
setup_process()
@@ -70,11 +89,10 @@ def worker_process(worker_json: StrJSON) -> NoReturn:
7089
print(f"Re-running {test_name} in verbose mode", flush=True)
7190

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

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

8098

‎Misc/NEWS.d/next/Tests/2023-09-11-18-19-52.gh-issue-109276.btfFtT.rst

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)
0