From ba8748f683acc3263e6988372c4109c1788e0b7b Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 10 Feb 2023 17:13:53 -0500 Subject: [PATCH 01/11] Ignore build dir when typing --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 5965db50..c7d7fea8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ disallow_untyped_defs = true # "unused-awaitable", # exclude = [ - "integration/", "tests/", "setup.py", "sites/www/conf.py" + "integration/", "tests/", "setup.py", "tasks.py", "sites/www/conf.py", "build/", ] ignore_missing_imports = true # implicit_reexport = False From 3d30ba56b614a3ab5413d804f5fe06821f1ca5e1 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 10 Feb 2023 17:27:30 -0500 Subject: [PATCH 02/11] Add type checks to CI --- .circleci/config.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index f2f4e2a2..ed30259a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -35,6 +35,15 @@ jobs: - run: inv www.doctest - orb/debug + typecheck: + executor: + name: orb/default + version: "3.6" + steps: + - orb/setup + - run: mypy . + - orb/debug + workflows: main: @@ -43,6 +52,8 @@ workflows: name: Lint - orb/format: name: Style check + - typecheck: + name: Types check - coverage: name: Test - regression: From d9c8f68a2d2ac6f7576cc70f4bc553849b893f7d Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 10 Feb 2023 18:08:10 -0500 Subject: [PATCH 03/11] Mark runner stream processors as Optional[bytes] I'm not sure why these were marked as bytes/str/None, os.read and friends ought to be 100% bytes-emitting. Maybe this was a Py2+3 hangover? --- invoke/runners.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/invoke/runners.py b/invoke/runners.py index f24bb5f3..ae422173 100644 --- a/invoke/runners.py +++ b/invoke/runners.py @@ -1075,7 +1075,7 @@ def start_timer(self, timeout: int) -> None: self._timer = threading.Timer(timeout, self.kill) self._timer.start() - def read_proc_stdout(self, num_bytes: int) -> Union[bytes, str, None]: + def read_proc_stdout(self, num_bytes: int) -> Optional[bytes]: """ Read ``num_bytes`` from the running process' stdout stream. @@ -1087,7 +1087,7 @@ def read_proc_stdout(self, num_bytes: int) -> Union[bytes, str, None]: """ raise NotImplementedError - def read_proc_stderr(self, num_bytes: int) -> Union[bytes, str, None]: + def read_proc_stderr(self, num_bytes: int) -> Optional[bytes]: """ Read ``num_bytes`` from the running process' stderr stream. @@ -1238,7 +1238,7 @@ def should_use_pty(self, pty: bool = False, fallback: bool = True) -> bool: use_pty = False return use_pty - def read_proc_stdout(self, num_bytes: int) -> Union[bytes, str, None]: + def read_proc_stdout(self, num_bytes: int) -> Optional[bytes]: # Obtain useful read-some-bytes function if self.using_pty: # Need to handle spurious OSErrors on some Linux platforms. @@ -1265,7 +1265,7 @@ def read_proc_stdout(self, num_bytes: int) -> Union[bytes, str, None]: data = None return data - def read_proc_stderr(self, num_bytes: int) -> Union[bytes, str, None]: + def read_proc_stderr(self, num_bytes: int) -> Optional[bytes]: # NOTE: when using a pty, this will never be called. # TODO: do we ever get those OSErrors on stderr? Feels like we could? if self.process and self.process.stderr: From 1b3ad79c17e246468ef55649c35d6aedb0f955e8 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 10 Feb 2023 18:11:09 -0500 Subject: [PATCH 04/11] Twiddle newly raised exception language --- invoke/runners.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/invoke/runners.py b/invoke/runners.py index ae422173..8ba3713c 100644 --- a/invoke/runners.py +++ b/invoke/runners.py @@ -1280,7 +1280,7 @@ def _write_proc_stdin(self, data: bytes) -> None: elif self.process and self.process.stdin: fd = self.process.stdin.fileno() else: - raise SubprocessPipeError("No stdin process exists") + raise SubprocessPipeError("Unable to write to missing subprocess or stdin!") # Try to write, ignoring broken pipes if encountered (implies child # process exited before the process piping stdin to us finished; # there's nothing we can do about that!) @@ -1298,7 +1298,7 @@ def close_proc_stdin(self) -> None: elif self.process and self.process.stdin: self.process.stdin.close() else: - raise SubprocessPipeError("No stdin process exists") + raise SubprocessPipeError("Unable to close missing subprocess or stdin!") def start(self, command: str, shell: str, env: Dict[str, Any]) -> None: if self.using_pty: From 0c7b802b8a1588dc41de52c1023b3323b00adf17 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 10 Feb 2023 18:17:12 -0500 Subject: [PATCH 05/11] Make Runner.returncode return type Optional[int] and reinstate former behavior of returning None by default instead of 0. I wasn't a big fan of changing this even though it should only occur in corner cases. It's a very important corner case! Also updated the superclass' docstring to make this clearer, as it was not entirely accurate. --- invoke/runners.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/invoke/runners.py b/invoke/runners.py index 8ba3713c..64bc8d22 100644 --- a/invoke/runners.py +++ b/invoke/runners.py @@ -1154,11 +1154,13 @@ def send_interrupt(self, interrupt: "KeyboardInterrupt") -> None: """ self.write_proc_stdin("\x03") - def returncode(self) -> int: + def returncode(self) -> Optional[int]: """ Return the numeric return/exit code resulting from command execution. - :returns: `int` + :returns: + `int`, if any reasonable return code could be determined, or + ``None`` in corner cases where that was not possible. .. versionadded:: 1.0 """ @@ -1358,7 +1360,7 @@ def process_is_finished(self) -> bool: else: return self.process.poll() is not None - def returncode(self) -> int: + def returncode(self) -> Optional[int]: if self.using_pty: # No subprocess.returncode available; use WIFEXITED/WIFSIGNALED to # determine whch of WEXITSTATUS / WTERMSIG to use. @@ -1366,7 +1368,7 @@ def returncode(self) -> int: # return whichever one of them is nondefault"? Probably not? # NOTE: doing this in an arbitrary order should be safe since only # one of the WIF* methods ought to ever return True. - code = 0 + code = None if os.WIFEXITED(self.status): code = os.WEXITSTATUS(self.status) elif os.WIFSIGNALED(self.status): From 4cce0bef50fda902f61d2b1995430399696590fd Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 10 Feb 2023 19:06:29 -0500 Subject: [PATCH 06/11] Pull in slightly newer typing reqs per 7a9a9dd --- dev-requirements.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 4b56b30f..c3f04f9f 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -20,6 +20,5 @@ setuptools>56 # Debuggery icecream>=2.1 # typing -mypy==0.971 -typed-ast==1.5.4 +mypy==0.991 types-PyYAML==6.0.12.4 From 896fc9a7526755d797d71fc49e02a179ff438918 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 11 Feb 2023 15:27:40 -0500 Subject: [PATCH 07/11] Remove some Python 3.8-isms ('/' in sigs) --- invoke/tasks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/invoke/tasks.py b/invoke/tasks.py index 22ce5952..cd3075e9 100644 --- a/invoke/tasks.py +++ b/invoke/tasks.py @@ -59,7 +59,6 @@ class Task(Generic[T]): def __init__( self, body: Callable, - /, name: Optional[str] = None, aliases: Iterable[str] = (), positional: Optional[Iterable[str]] = None, @@ -484,7 +483,7 @@ def clone( return klass(**data) -def call(task: "Task", /, *args: Any, **kwargs: Any) -> "Call": +def call(task: "Task", *args: Any, **kwargs: Any) -> "Call": """ Describes execution of a `.Task`, typically with pre-supplied arguments. From cfc692d71261cb34d23d72699a651a6fe7a70355 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 11 Feb 2023 15:29:09 -0500 Subject: [PATCH 08/11] Clean up some test prints, blacken, flake8 --- invoke/runners.py | 9 ++++++--- tasks.py | 3 --- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/invoke/runners.py b/invoke/runners.py index 64bc8d22..423cb797 100644 --- a/invoke/runners.py +++ b/invoke/runners.py @@ -19,7 +19,6 @@ Optional, Tuple, Type, - Union, ) # Import some platform-specific things at top level so they can be mocked for @@ -1282,7 +1281,9 @@ def _write_proc_stdin(self, data: bytes) -> None: elif self.process and self.process.stdin: fd = self.process.stdin.fileno() else: - raise SubprocessPipeError("Unable to write to missing subprocess or stdin!") + raise SubprocessPipeError( + "Unable to write to missing subprocess or stdin!" + ) # Try to write, ignoring broken pipes if encountered (implies child # process exited before the process piping stdin to us finished; # there's nothing we can do about that!) @@ -1300,7 +1301,9 @@ def close_proc_stdin(self) -> None: elif self.process and self.process.stdin: self.process.stdin.close() else: - raise SubprocessPipeError("Unable to close missing subprocess or stdin!") + raise SubprocessPipeError( + "Unable to close missing subprocess or stdin!" + ) def start(self, command: str, shell: str, env: Dict[str, Any]) -> None: if self.using_pty: diff --git a/tasks.py b/tasks.py index 228b2931..37cb4568 100644 --- a/tasks.py +++ b/tasks.py @@ -51,9 +51,6 @@ def test( ) -print('test', vars(test), type(test)) - - # TODO: replace with invocations' once the "call truly local tester" problem is # solved (see other TODOs). For now this is just a copy/paste/modify. @task(help=test.help) # type: ignore From 5fb494e0fe64e7ef683159084dc9a872e5148b5b Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 11 Feb 2023 15:57:25 -0500 Subject: [PATCH 09/11] Ah, mypy 0.991 must be for >Py36 --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index c3f04f9f..800afa4e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -20,5 +20,5 @@ setuptools>56 # Debuggery icecream>=2.1 # typing -mypy==0.991 +mypy==0.971 types-PyYAML==6.0.12.4 From fff76e015329355f2e48ade2784486b75e3b4d0a Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 11 Feb 2023 16:12:52 -0500 Subject: [PATCH 10/11] Not sure I agree with typing a tasks.py but may as well check it for now Given that the work's been done to annotate it! --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c7d7fea8..8f10b685 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ disallow_untyped_defs = true # "unused-awaitable", # exclude = [ - "integration/", "tests/", "setup.py", "tasks.py", "sites/www/conf.py", "build/", + "integration/", "tests/", "setup.py", "sites/www/conf.py", "build/", ] ignore_missing_imports = true # implicit_reexport = False From e01da1af48f030648e5e87919f595d6e420eed67 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 11 Feb 2023 18:18:05 -0500 Subject: [PATCH 11/11] Fix up integration suite??? Cannot for life of me figure out why this decided to break considering none of the changes for #906 appear relevant. But smells like something that incidentally worked-by-mistake in the past (pytest trying to load up the tasks.py is something I'd fixed in other suites recently). --- integration/{ => _support}/tasks.py | 0 integration/main.py | 19 ++++++------------- 2 files changed, 6 insertions(+), 13 deletions(-) rename integration/{ => _support}/tasks.py (100%) diff --git a/integration/tasks.py b/integration/_support/tasks.py similarity index 100% rename from integration/tasks.py rename to integration/_support/tasks.py diff --git a/integration/main.py b/integration/main.py index 7a86a80f..253aae15 100644 --- a/integration/main.py +++ b/integration/main.py @@ -1,4 +1,5 @@ import os +from pathlib import Path import sys import pytest @@ -15,18 +16,13 @@ def _output_eq(cmd, expected): assert run(cmd, hide=True).stdout == expected -def _setup(self): - self.cwd = os.getcwd() - # Enter integration/ so Invoke loads its local tasks.py - os.chdir(os.path.dirname(__file__)) - - class Main: - def setup(self): - # MEH - _setup(self) + def setup_method(self): + self.cwd = os.getcwd() + # Enter integration/_support as all support files are in there now + os.chdir(Path(__file__).parent / "_support") - def teardown(self): + def teardown_method(self): os.chdir(self.cwd) class basics: @@ -86,7 +82,6 @@ def invocable_via_python_dash_m(self): class funky_characters_in_stdout: @only_utf8 def basic_nonstandard_characters(self): - os.chdir("_support") # Crummy "doesn't explode with decode errors" test cmd = ("type" if WINDOWS else "cat") + " tree.out" run(cmd, hide="stderr") @@ -117,7 +112,6 @@ def complex_nesting_under_ptys_doesnt_break(self): def pty_puts_both_streams_in_stdout(self): if WINDOWS: return - os.chdir("_support") err_echo = "{} err.py".format(sys.executable) command = "echo foo && {} bar".format(err_echo) r = run(command, hide="both", pty=True) @@ -152,7 +146,6 @@ def false_as_optional_arg_default_value_works_okay(self): # (Dis)proves #416. When bug present, parser gets very confused, # asks "what the hell is 'whee'?". See also a unit test for # Task.get_arguments. - os.chdir("_support") for argstr, expected in ( ("", "False"), ("--meh", "True"),