From 5bb4da02c2c8c92d017e8b6be57eb442cc8f04ff Mon Sep 17 00:00:00 2001 From: Cooper Lees Date: Tue, 24 Aug 2021 14:29:49 -0700 Subject: [PATCH] Add cpython Lib/ repository config into primer config - Disabled (#2429) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * Add CPython repository into primer runs - CPython tests is probably the best repo for black to test on as the stdlib's unittests should use all syntax - Limit to running in recent versions of the python runtime - e.g. today >= 3.9 - This allows us to parse more syntax - Exclude all failing files for now - Definitely have bugs to explore there - Refer to #2407 for more details there - Some test files on purpose have syntax errors, so we will never be able to parse them - Add new black command arguments logging in debug mode; very handy for seeing how CLI arguments are formatted CPython now succeeds ignoring 16 files: ``` Oh no! 💥 💔 💥 1859 files would be reformatted, 148 files would be left unchanged. ``` Testing - Ran locally with and without string processing - Very little runtime difference BUT 3 more failed files ``` time /tmp/tb/bin/black --experimental-string-processing --check . 2>&1 | tee /tmp/black_cpython_esp ... Oh no! 💥 💔 💥 1859 files would be reformatted, 148 files would be left unchanged, 16 files would fail to reformat. real 4m8.563s user 16m21.735s sys 0m6.000s ``` - Add unittest for new covienence config file flattening that allows long arguments to be broke up into an array/list of strings Addresses #2407 --- Commit history before merge: * Add new `timeout_seconds` support into primer.json - If present, will set forked process limit to that value in seconds - Otherwise, stay with default 10 minutes (600 seconds) * Add new "base_path" concept to black-primer - Rather than start at the repo root start at a configured path within the repository - e.g. for cpython only run black on `Lib` * Disable by default - It's too much for GitHub Actions. But let's leave config for others to use * Minor tweak to _flatten_cli_args Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> --- src/black_primer/lib.py | 42 ++++++++++++++++++++++++++++++++---- src/black_primer/primer.json | 33 +++++++++++++++++++++++++++- tests/test_primer.py | 7 ++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/black_primer/lib.py b/src/black_primer/lib.py index 784134b..7494ae6 100644 --- a/src/black_primer/lib.py +++ b/src/black_primer/lib.py @@ -12,12 +12,23 @@ from shutil import rmtree, which from subprocess import CalledProcessError from sys import version_info from tempfile import TemporaryDirectory -from typing import Any, Callable, Dict, NamedTuple, Optional, Sequence, Tuple +from typing import ( + Any, + Callable, + Dict, + List, + NamedTuple, + Optional, + Sequence, + Tuple, + Union, +) from urllib.parse import urlparse import click +TEN_MINUTES_SECONDS = 600 WINDOWS = system() == "Windows" BLACK_BINARY = "black.exe" if WINDOWS else "black" GIT_BINARY = "git.exe" if WINDOWS else "git" @@ -39,7 +50,7 @@ class Results(NamedTuple): async def _gen_check_output( cmd: Sequence[str], - timeout: float = 600, + timeout: float = TEN_MINUTES_SECONDS, env: Optional[Dict[str, str]] = None, cwd: Optional[Path] = None, stdin: Optional[bytes] = None, @@ -113,6 +124,21 @@ def analyze_results(project_count: int, results: Results) -> int: return results.stats["failed"] +def _flatten_cli_args(cli_args: List[Union[Sequence[str], str]]) -> List[str]: + """Allow a user to put long arguments into a list of strs + to make the JSON human readable""" + flat_args = [] + for arg in cli_args: + if isinstance(arg, str): + flat_args.append(arg) + continue + + args_as_str = "".join(arg) + flat_args.append(args_as_str) + + return flat_args + + async def black_run( project_name: str, repo_path: Optional[Path], @@ -131,7 +157,7 @@ async def black_run( stdin_test = project_name.upper() == "STDIN" cmd = [str(which(BLACK_BINARY))] if "cli_arguments" in project_config and project_config["cli_arguments"]: - cmd.extend(project_config["cli_arguments"]) + cmd.extend(_flatten_cli_args(project_config["cli_arguments"])) cmd.append("--check") if not no_diff: cmd.append("--diff") @@ -141,9 +167,16 @@ async def black_run( if stdin_test: cmd.append("-") stdin = repo_path.read_bytes() + elif "base_path" in project_config: + cmd.append(project_config["base_path"]) else: cmd.append(".") + timeout = ( + project_config["timeout_seconds"] + if "timeout_seconds" in project_config + else TEN_MINUTES_SECONDS + ) with TemporaryDirectory() as tmp_path: # Prevent reading top-level user configs by manipulating environment variables env = { @@ -154,8 +187,9 @@ async def black_run( cwd_path = repo_path.parent if stdin_test else repo_path try: + LOG.debug(f"Running black for {project_name}: {' '.join(cmd)}") _stdout, _stderr = await _gen_check_output( - cmd, cwd=cwd_path, env=env, stdin=stdin + cmd, cwd=cwd_path, env=env, stdin=stdin, timeout=timeout ) except asyncio.TimeoutError: results.stats["failed"] += 1 diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index edbed3f..0d1018f 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -1,5 +1,5 @@ { - "configuration_format_version": 20200509, + "configuration_format_version": 20210815, "projects": { "STDIN": { "cli_arguments": ["--experimental-string-processing"], @@ -36,6 +36,37 @@ "long_checkout": false, "py_versions": ["all"] }, + "cpython": { + "disabled": true, + "disabled_reason": "To big / slow for GitHub Actions but handy to keep config to use manually or in some other CI in the future", + "base_path": "Lib", + "cli_arguments": [ + "--experimental-string-processing", + "--extend-exclude", + [ + "Lib/lib2to3/tests/data/different_encoding.py", + "|Lib/lib2to3/tests/data/false_encoding.py", + "|Lib/lib2to3/tests/data/py2_test_grammar.py", + "|Lib/test/bad_coding.py", + "|Lib/test/bad_coding2.py", + "|Lib/test/badsyntax_3131.py", + "|Lib/test/badsyntax_pep3120.py", + "|Lib/test/test_base64.py", + "|Lib/test/test_exceptions.py", + "|Lib/test/test_grammar.py", + "|Lib/test/test_named_expressions.py", + "|Lib/test/test_patma.py", + "|Lib/test/test_tokenize.py", + "|Lib/test/test_xml_etree.py", + "|Lib/traceback.py" + ] + ], + "expect_formatting_changes": true, + "git_clone_url": "https://github.com/python/cpython.git", + "long_checkout": false, + "py_versions": ["3.9", "3.10"], + "timeout_seconds": 900 + }, "django": { "cli_arguments": [ "--experimental-string-processing", diff --git a/tests/test_primer.py b/tests/test_primer.py index 8dd1212..e7f99fd 100644 --- a/tests/test_primer.py +++ b/tests/test_primer.py @@ -146,6 +146,11 @@ class PrimerLibTests(unittest.TestCase): ) self.assertEqual(2, results.stats["failed"]) + def test_flatten_cli_args(self) -> None: + fake_long_args = ["--arg", ["really/", "|long", "|regex", "|splitup"], "--done"] + expected = ["--arg", "really/|long|regex|splitup", "--done"] + self.assertEqual(expected, lib._flatten_cli_args(fake_long_args)) + @event_loop() def test_gen_check_output(self) -> None: loop = asyncio.get_event_loop() @@ -184,6 +189,8 @@ class PrimerLibTests(unittest.TestCase): @patch("sys.stdout", new_callable=StringIO) @event_loop() def test_process_queue(self, mock_stdout: Mock) -> None: + """Test the process queue on primer itself + - If you have non black conforming formatting in primer itself this can fail""" loop = asyncio.get_event_loop() config_path = Path(lib.__file__).parent / "primer.json" with patch("black_primer.lib.git_checkout_or_rebase", return_false): -- 2.39.5