From a8bd0c016afb98195504ff388e41f0147f0f59bf Mon Sep 17 00:00:00 2001 From: Joris Roovers Date: Mon, 19 Dec 2022 09:42:27 +0000 Subject: [PATCH 1/3] WIP: remove dependency on sh for integration tests Mostly involves fixing the hook tests. --- .github/workflows/checks.yml | 4 ++-- qa/test_hooks.py | 18 ++++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 86b5d940..d710273c 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -52,7 +52,7 @@ jobs: - name: Integration tests (GITLINT_QA_USE_SH_LIB=0) run: | - hatch run qa:integration-tests --ignore qa/test_hooks.py qa + hatch run qa:integration-tests -k "not(test_commit_hook_continue or test_commit_hook_abort or test_commit_hook_edit)" qa env: GITLINT_QA_USE_SH_LIB: 0 @@ -134,7 +134,7 @@ jobs: - name: Integration tests run: | hatch run qa:install-local - hatch run qa:integration-tests -k "not (HookTests or test_lint_staged_stdin or test_stdin_file or test_stdin_pipe_empty)" qa + hatch run qa:integration-tests -k "not (test_commit_hook_continue or test_commit_hook_abort or test_commit_hook_edit or test_lint_staged_stdin or test_stdin_file or test_stdin_pipe_empty)" qa - name: Build test (gitlint) run: | diff --git a/qa/test_hooks.py b/qa/test_hooks.py index 19edeb25..6867a35c 100644 --- a/qa/test_hooks.py +++ b/qa/test_hooks.py @@ -30,18 +30,16 @@ def setUp(self): # install git commit-msg hook and assert output output_installed = gitlint("install-hook", _cwd=self.tmp_git_repo) - expected_installed = ( - f"Successfully installed gitlint commit-msg hook in {self.tmp_git_repo}/.git/hooks/commit-msg\n" - ) + commit_msg_hook_path = os.path.join(self.tmp_git_repo, ".git", "hooks", "commit-msg") + expected_installed = f"Successfully installed gitlint commit-msg hook in {commit_msg_hook_path}\n" self.assertEqualStdout(output_installed, expected_installed) def tearDown(self): # uninstall git commit-msg hook and assert output output_uninstalled = gitlint("uninstall-hook", _cwd=self.tmp_git_repo) - expected_uninstalled = ( - f"Successfully uninstalled gitlint commit-msg hook from {self.tmp_git_repo}/.git/hooks/commit-msg\n" - ) + commit_msg_hook_path = os.path.join(self.tmp_git_repo, ".git", "hooks", "commit-msg") + expected_uninstalled = f"Successfully uninstalled gitlint commit-msg hook from {commit_msg_hook_path}\n" self.assertEqualStdout(output_uninstalled, expected_uninstalled) super().tearDown() @@ -171,10 +169,10 @@ def test_commit_hook_worktree(self): output_installed = gitlint("install-hook", _cwd=worktree_dir) expected_hook_path = os.path.join(tmp_git_repo, ".git", "hooks", "commit-msg") - expected_msg = f"Successfully installed gitlint commit-msg hook in {expected_hook_path}\r\n" - self.assertEqual(output_installed, expected_msg) + expected_msg = f"Successfully installed gitlint commit-msg hook in {expected_hook_path}\n" + self.assertEqualStdout(output_installed, expected_msg) output_uninstalled = gitlint("uninstall-hook", _cwd=worktree_dir) expected_hook_path = os.path.join(tmp_git_repo, ".git", "hooks", "commit-msg") - expected_msg = f"Successfully uninstalled gitlint commit-msg hook from {expected_hook_path}\r\n" - self.assertEqual(output_uninstalled, expected_msg) + expected_msg = f"Successfully uninstalled gitlint commit-msg hook from {expected_hook_path}\n" + self.assertEqualStdout(output_uninstalled, expected_msg) From cd44563e5e0dcbcb02e631ea76bbff0706d523dc Mon Sep 17 00:00:00 2001 From: Joris Roovers Date: Tue, 20 Dec 2022 10:20:31 +0000 Subject: [PATCH 2/3] Trying some things out to to fix the hook tests without sh --- qa/shell.py | 31 ++++++++++++++++++++++++++----- qa/test_hooks.py | 1 + 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/qa/shell.py b/qa/shell.py index 7cddf3bc..64b6750c 100644 --- a/qa/shell.py +++ b/qa/shell.py @@ -1,6 +1,7 @@ # This code is mostly duplicated from the `gitlint.shell` module. We consciously duplicate this code as to not depend # on gitlint internals for our integration testing framework. +import queue import subprocess from qa.utils import USE_SH_LIB, DEFAULT_ENCODING @@ -83,18 +84,38 @@ def _exec(*args, **kwargs): "env": kwargs.get("_env", None), } - stdin_input = None + stdin = None if len(args) > 1 and isinstance(args[1], ShResult): - stdin_input = args[1].stdout + stdin = args[1].stdout # pop args[1] from the array and use it as stdin args = list(args) args.pop(1) - popen_kwargs["stdin"] = subprocess.PIPE + elif kwargs.get("_out", None): + popen_kwargs["bufsize"] = 1 + # stdin, stdout_err = os.pipe() + # popen_kwargs["stdout"] = stdout_err + # popen_kwargs["stderr"] = stdout_err + # popen_kwargs["stdin"] = subprocess.PIPE try: with subprocess.Popen(args, **popen_kwargs) as p: - if stdin_input: - result = p.communicate(stdin_input) + if kwargs.get("_out", None): + q = queue.Queue() + iofunc = kwargs["_out"] + while True: + if not q.empty(): + inputstr = q.get().encode(DEFAULT_ENCODING) + p.stdin.write(inputstr) + p.stdin.flush() + + else: + line = p.stderr.readline() + if line: + iofunc(line.decode(DEFAULT_ENCODING), q) + else: + break + elif stdin: + result = p.communicate(stdin) else: result = p.communicate() diff --git a/qa/test_hooks.py b/qa/test_hooks.py index 6867a35c..7e134cb8 100644 --- a/qa/test_hooks.py +++ b/qa/test_hooks.py @@ -52,6 +52,7 @@ def _violations(self): def _interact(self, line, stdin): self.githook_output.append(line) + print(line) # Answer 'yes' to question to keep violating commit-msg if "Your commit message contains violations" in line: response = self.responses[self.response_index] From 5dc56b551bd98ea92d5ab344f9828b38183aeabe Mon Sep 17 00:00:00 2001 From: Joris Roovers Date: Wed, 21 Dec 2022 09:54:03 +0000 Subject: [PATCH 3/3] More attempts at replacing sh for hook tests Not succesful yet, can't get the stdin to work :/ --- qa/shell.py | 83 +++++++++++++++++++++++++++++++++--------------- qa/test_hooks.py | 8 ++++- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/qa/shell.py b/qa/shell.py index 64b6750c..ed0f043d 100644 --- a/qa/shell.py +++ b/qa/shell.py @@ -1,6 +1,7 @@ # This code is mostly duplicated from the `gitlint.shell` module. We consciously duplicate this code as to not depend # on gitlint internals for our integration testing framework. +import asyncio import queue import subprocess from qa.utils import USE_SH_LIB, DEFAULT_ENCODING @@ -76,9 +77,9 @@ def run_command(command, *args, **kwargs): def _exec(*args, **kwargs): popen_kwargs = { - "stdout": subprocess.PIPE, - "stderr": subprocess.PIPE, - "stdin": subprocess.PIPE, + "stdout": asyncio.subprocess.PIPE, + "stderr": asyncio.subprocess.PIPE, + "stdin": asyncio.subprocess.PIPE, "shell": kwargs.get("_tty_out", False), "cwd": kwargs.get("_cwd", None), "env": kwargs.get("_env", None), @@ -90,33 +91,63 @@ def _exec(*args, **kwargs): # pop args[1] from the array and use it as stdin args = list(args) args.pop(1) - elif kwargs.get("_out", None): - popen_kwargs["bufsize"] = 1 - # stdin, stdout_err = os.pipe() - # popen_kwargs["stdout"] = stdout_err - # popen_kwargs["stderr"] = stdout_err - # popen_kwargs["stdin"] = subprocess.PIPE try: - with subprocess.Popen(args, **popen_kwargs) as p: - if kwargs.get("_out", None): - q = queue.Queue() - iofunc = kwargs["_out"] + + async def read_stream(p, stream, iofunc, q, timeout=0.3): + line = bytearray() + try: while True: - if not q.empty(): - inputstr = q.get().encode(DEFAULT_ENCODING) - p.stdin.write(inputstr) - p.stdin.flush() - - else: - line = p.stderr.readline() - if line: - iofunc(line.decode(DEFAULT_ENCODING), q) - else: - break - elif stdin: + char = await asyncio.wait_for(stream.read(1), timeout) + line += bytearray(char) + if char == b"\n": + decoded = line.decode(DEFAULT_ENCODING) + iofunc(decoded, q) + line = bytearray() + except asyncio.TimeoutError: + decoded = line.decode(DEFAULT_ENCODING) + iofunc(decoded, q) + + async def write_stdin(p, q): + # inputstr = await q.get() + inputstr = await asyncio.wait_for(q.get(), 0.25) + p.stdin.write(inputstr.encode(DEFAULT_ENCODING)) + await p.stdin.drain() + + if kwargs.get("_out", None): + # redirect stderr to stdout (this will ensure we capture the last git output lines which are printed to stdout, not stderr) + popen_kwargs["stderr"] = asyncio.subprocess.STDOUT + + async def start_process(): + p = await asyncio.create_subprocess_exec(*args, **popen_kwargs) + + q = asyncio.Queue() + + await asyncio.gather( + # read_stream(p, p.stderr, kwargs["_out"], q), + read_stream(p, p.stdout, kwargs["_out"], q), + ) + print("after gather1") + await asyncio.gather(write_stdin(p, q)) + print("after gather2") + + # Manually answer the prompt here, for some reason I can't get this to work via stdin + await asyncio.sleep(2) + + await asyncio.gather( + read_stream(p, p.stdout, kwargs["_out"], q), + ) + print("after gather 3") + await p.wait() + print("process finished") + + asyncio.run(start_process()) + return + elif stdin: + with subprocess.Popen(args, **popen_kwargs) as p: result = p.communicate(stdin) - else: + else: + with subprocess.Popen(args, **popen_kwargs) as p: result = p.communicate() except FileNotFoundError as exc: diff --git a/qa/test_hooks.py b/qa/test_hooks.py index 7e134cb8..a9fbe5d7 100644 --- a/qa/test_hooks.py +++ b/qa/test_hooks.py @@ -55,8 +55,10 @@ def _interact(self, line, stdin): print(line) # Answer 'yes' to question to keep violating commit-msg if "Your commit message contains violations" in line: + print("VIOLATIONS FOUND!") response = self.responses[self.response_index] - stdin.put(f"{response}\n") + stdin.put_nowait(f"{response}\r\n") + print("AFTER PUT") self.response_index = (self.response_index + 1) % len(self.responses) def test_commit_hook_no_violations(self): @@ -92,6 +94,10 @@ def test_commit_hook_continue(self): " 1 file changed, 0 insertions(+), 0 deletions(-)\n", f" create mode 100644 {test_filename}\n", ] + print("EXPECTED OUTPUT") + print(expected_output) + print("ACTUAL OUTPUT") + print(self.githook_output) for output, expected in zip(self.githook_output, expected_output): self.assertMultiLineEqual(output.replace("\r", ""), expected.replace("\r", ""))