Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-109413: Enable strict_optional = true for libregrtest/run_workers #126855

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
4 changes: 0 additions & 4 deletions Lib/test/libregrtest/mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ disallow_untyped_defs = False
check_untyped_defs = False
warn_return_any = False

# Enable --strict-optional for these ASAP:
[mypy-Lib.test.libregrtest.run_workers.*]
strict_optional = False

# Various internal modules that typeshed deliberately doesn't have stubs for:
[mypy-_abc.*,_opcode.*,_overlapped.*,_testcapi.*,_testinternalcapi.*,test.*]
ignore_missing_imports = True
36 changes: 27 additions & 9 deletions Lib/test/libregrtest/run_workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,28 @@ def __init__(self, worker_id: int, runner: "RunWorkers") -> None:
self.output = runner.output
self.timeout = runner.worker_timeout
self.log = runner.log
self.test_name: TestName | None = None
self.start_time: float | None = None
self._test_name: TestName | None = None
self._start_time: float | None = None
self._popen: subprocess.Popen[str] | None = None
self._killed = False
self._stopped = False

@property
def test_name(self) -> TestName:
if self._test_name is None:
raise ValueError(
'Should never call `.test_name` before calling `.run()`'
sobolevn marked this conversation as resolved.
Show resolved Hide resolved
)
return self._test_name

@property
def start_time(self) -> float:
if self._start_time is None:
raise ValueError(
'Should never call `.start_time` before calling `.run()`'
)
return self._start_time

def __repr__(self) -> str:
info = [f'WorkerThread #{self.worker_id}']
if self.is_alive():
Expand All @@ -129,7 +145,7 @@ def __repr__(self) -> str:
popen = self._popen
if popen is not None:
dt = time.monotonic() - self.start_time
info.extend((f'pid={self._popen.pid}',
info.extend((f'pid={popen.pid}',
f'time={format_duration(dt)}'))
return '<%s>' % ' '.join(info)

Expand Down Expand Up @@ -394,14 +410,14 @@ def run(self) -> None:
except StopIteration:
break

self.start_time = time.monotonic()
self.test_name = test_name
self._start_time = time.monotonic()
self._test_name = test_name
try:
mp_result = self._runtest(test_name)
except WorkerError as exc:
mp_result = exc.mp_result
finally:
self.test_name = None
self._test_name = None
mp_result.result.duration = time.monotonic() - self.start_time
self.output.put((False, mp_result))

Expand All @@ -416,6 +432,8 @@ def run(self) -> None:

def _wait_completed(self) -> None:
popen = self._popen
# only needed for mypy:
assert popen is not None, "Should never call `._popen` before `.run()`"
sobolevn marked this conversation as resolved.
Show resolved Hide resolved

try:
popen.wait(WAIT_COMPLETED_TIMEOUT)
Expand Down Expand Up @@ -483,7 +501,7 @@ def __init__(self, num_workers: int, runtests: RunTests,
self.worker_timeout: float | None = min(self.timeout * 1.5, self.timeout + 5 * 60)
else:
self.worker_timeout = None
self.workers: list[WorkerThread] | None = None
self.workers: list[WorkerThread] = []

sobolevn marked this conversation as resolved.
Show resolved Hide resolved
jobs = self.runtests.get_jobs()
if jobs is not None:
Expand All @@ -503,7 +521,7 @@ def start_workers(self) -> None:
processes = plural(nworkers, "process", "processes")
msg = (f"Run {tests} in parallel using "
f"{nworkers} worker {processes}")
if self.timeout:
if self.timeout and self.worker_timeout is not None:
msg += (" (timeout: %s, worker timeout: %s)"
% (format_duration(self.timeout),
format_duration(self.worker_timeout)))
Expand Down Expand Up @@ -555,7 +573,7 @@ def display_result(self, mp_result: MultiprocessResult) -> None:
if mp_result.err_msg:
# WORKER_BUG
text += ' (%s)' % mp_result.err_msg
elif (result.duration >= PROGRESS_MIN_TIME and not pgo):
elif (result.duration and result.duration >= PROGRESS_MIN_TIME and not pgo):
text += ' (%s)' % format_duration(result.duration)
if not pgo:
running = get_running(self.workers)
Expand Down
Loading