8000 gh-109413: Fix some trivial mypy nitpicks in libregrtest by AlexWaygood · Pull Request #109454 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-109413: Fix some trivial mypy nitpicks in libregrtest #109454

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 15, 2023
Merged

gh-109413: Fix some trivial mypy nitpicks in libregrtest #109454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95f2e95
gh-109413: Fix some trivial mypy nitpicks in libregrtest
AlexWaygood Sep 15, 2023
24ec358
`test_time` -> `log_time`
AlexWaygood Sep 15, 2023
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
8 changes: 4 additions & 4 deletions Lib/test/libregrtest/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ def log(self, line: str = '') -> None:
line = f"load avg: {load_avg:.2f} {line}"

# add the timestamp prefix: "0:01:05 "
test_time = time.perf_counter() - self.start_time
log_time = time.perf_counter() - self.start_time

mins, secs = divmod(int(test_time), 60)
mins, secs = divmod(int(log_time), 60)
hours, mins = divmod(mins, 60)
test_time = "%d:%02d:%02d" % (hours, mins, secs)
formatted_log_time = "%d:%02d:%02d" % (hours, mins, secs)

line = f"{test_time} {line}"
line = f"{formatted_log_time} {line}"
if empty:
line = line[:-1]

Expand Down
6 changes: 2 additions & 4 deletions Lib/test/libregrtest/results.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ def display_summary(self, first_runtests: RunTests, filtered: bool):
report.append(f'failures={stats.failures:,}')
if stats.skipped:
report.append(f'skipped={stats.skipped:,}')
report = ' '.join(report)
print(f"Total tests: {report}")
print(f"Total tests: {' '.join(report)}")
Copy link
Member Author

Choose a reason for hiding this comment

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

There's the same issue here: report is a list earlier on in the function, but at this location, it's being assigned to a str. We could use a new variable name, or just do what I suggest here (make it an "anonymous variable")


# Total test files
all_tests = [self.good, self.bad, self.rerun,
Expand All @@ -256,5 +255,4 @@ def display_summary(self, first_runtests: RunTests, filtered: bool):
):
if tests:
report.append(f'{name}={len(tests)}')
report = ' '.join(report)
print(f"Total test files: {report}")
print(f"Total test files: {' '.join(report)}")
8 changes: 4 additions & 4 deletions Lib/test/libregrtest/single.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@ def _runtest_env_changed_exc(result: TestResult, runtests: RunTests,
with saved_test_environment(test_name,
runtests.verbose, quiet, pgo=pgo):
_load_run_test(result, runtests)
except support.ResourceDenied as msg:
except support.ResourceDenied as exc:
if not quiet and not pgo:
print(f"{test_name} skipped -- {msg}", flush=True)
print(f"{test_name} skipped -- {exc}", flush=True)
result.state = State.RESOURCE_DENIED
return
except unittest.SkipTest as msg:
except unittest.SkipTest as exc:
if not quiet and not pgo:
print(f"{test_name} skipped -- {msg}", flush=True)
print(f"{test_name} skipped -- {exc}", flush=True)
Comment on lines -139 to +146
Copy link
Member Author
@AlexWaygood AlexWaygood Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This passage was causing mypy to get very agitated:

libregrtest\single.py:150: error: Assignment to variable "msg" outside
except: block  [misc]
            msg = f"test {test_name} failed"
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
libregrtest\single.py:152: error: Trying to read deleted variable "msg"
[misc]
                msg = f"{msg} -- {exc}"
                      ^~~~~~
libregrtest\single.py:152: error: Assignment to variable "msg" outside
except: block  [misc]
                msg = f"{msg} -- {exc}"
                      ^~~~~~~~~~~~~~~~~
libregrtest\single.py:153: error: Trying to read deleted variable "msg"
[misc]
            print(msg, file=sys.stderr, flush=True)
                  ^~~
libregrtest\single.py:160: error: Assignment to variable "msg" outside
except: block  [misc]
            msg = f"test {test_name} failed"
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
libregrtest\single.py:162: error: Trying to read deleted variable "msg"
[misc]
                msg = f"{msg} -- {exc}"
                      ^~~~~~
libregrtest\single.py:162: error: Assignment to variable "msg" outside
except: block  [misc]
                msg = f"{msg} -- {exc}"
                      ^~~~~~~~~~~~~~~~~
libregrtest\single.py:163: error: Trying to read deleted variable "msg"
[misc]
            print(msg, file=sys.stderr, flush=True)
                  ^~~
libregrtest\single.py:176: error: Assignment to variable "msg" outside
except: block  [misc]
                msg = traceback.format_exc()
                      ^~~~~~~~~~~~~~~~~~~~~~
libregrtest\single.py:177: error: Trying to read deleted variable "msg"
[misc]
                print(f"test {test_name} crashed -- {msg}",
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Mypy was getting confused because the msg variable had type Exception here, but lower down in the same function (on line 150), the same variable name is assigned to a str. By renaming the variable here as exc, the exc variable is now always an Exception instance in this function, and the msg variable is now always a str in this function.

result.state = State.SKIPPED
return
except support.TestFailedWithDetails as exc:
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/libregrtest/worker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def create_worker_process(runtests: RunTests, output_fd: int,
if python_cmd is not None:
executable = python_cmd
else:
executable = [sys.executable]
executable = (sys.executable,)
Copy link
Member Author

Choose a reason for hiding this comment

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

Mypy complained about this line because on line 26, the executable variable has type tuple[str, ...], but here, the same name is being assigned to an object of type list[str]. We can make mypy happy by just making it a tuple in both the branches.

cmd = [*executable, *support.args_from_interpreter_flags(),
'-u', # Unbuffered stdout and stderr
'-m', 'test.libregrtest.worker',
Expand Down
0