]> git.madduck.net Git - etc/vim.git/commitdiff

madduck's git repository

Every one of the projects in this repository is available at the canonical URL git://git.madduck.net/madduck/pub/<projectpath> — see each project's metadata for the exact URL.

All patches and comments are welcome. Please squash your changes to logical commits before using git-format-patch and git-send-email to patches@git.madduck.net. If you'd read over the Git project's submission guidelines and adhered to them, I'd be especially grateful.

SSH access, as well as push access can be individually arranged.

If you use my repositories frequently, consider adding the following snippet to ~/.gitconfig and using the third clone URL listed for each project:

[url "git://git.madduck.net/madduck/"]
  insteadOf = madduck:

Add cpython Lib/ repository config into primer config - Disabled (#2429)
authorCooper Lees <me@cooperlees.com>
Tue, 24 Aug 2021 21:29:49 +0000 (14:29 -0700)
committerGitHub <noreply@github.com>
Tue, 24 Aug 2021 21:29:49 +0000 (17:29 -0400)
* 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
src/black_primer/primer.json
tests/test_primer.py

index 784134bf6523900690a616f54424749b2e00cd6d..7494ae6dc7dcbf8743bd5365bdbf819c5dfa45a5 100644 (file)
@@ -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
index edbed3f33dd3a353ae5d8555e4150f66db7a8359..0d1018fc50ebba776a485957d1a79372898bc4a5 100644 (file)
@@ -1,5 +1,5 @@
 {
-  "configuration_format_version": 20200509,
+  "configuration_format_version": 20210815,
   "projects": {
     "STDIN": {
       "cli_arguments": ["--experimental-string-processing"],
       "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",
index 8dd1212313e09a081a3d9d494acf74f2b6993fc0..e7f99fdb0c2ec97df80dce5fd4bd7bb129b5c175 100644 (file)
@@ -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):