Use new Black check in CI (#43303)

* cut

* debug: log

* missing black in pyproject should default to false...dunno why this worked for tox without this change tho

* check exclusions in dispatch

* Update eng/tools/azure-sdk-tools/azpysdk/black.py

Co-authored-by: Scott Beddall <[email protected]>

---------

Co-authored-by: Scott Beddall <[email protected]>

Author: JennyPng

eng/pipelines/templates/steps/run_black.yml

@@ -9,13 +9,15 @@ steps:
   - task: PythonScript@0
     displayName: 'Run Black'
     inputs:
-      scriptPath: 'scripts/devops_tasks/dispatch_tox.py'
+      scriptPath: 'eng/scripts/dispatch_checks.py'
       arguments: >-
         "$(TargetingString)"
-        --mark_arg="${{ parameters.TestMarkArgument }}"
         --service="${{ parameters.ServiceDirectory }}"
-        --toxenv="black"
+        --checks="black"
         --filter-type="Omit_management"
         ${{ parameters.AdditionalTestArgs }}
-    env: ${{ parameters.EnvVars }}
+    env:
+      TOX_PIP_IMPL: "uv"
+      VIRTUAL_ENV: ""
+      PYTHONHOME: ""
     condition: and(succeededOrFailed(), ne(variables['Skip.Black'],'true'))

eng/scripts/dispatch_checks.py

@@ -12,10 +12,11 @@
 from ci_tools.variables import in_ci
 from ci_tools.scenario.generation import build_whl_for_req
 from ci_tools.logging import configure_logging, logger
-from ci_tools.environment_exclusions import is_check_enabled
+from ci_tools.environment_exclusions import is_check_enabled, CHECK_DEFAULTS
 
 root_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
 
+
 @dataclass
 class CheckResult:
     package: str
@@ -26,7 +27,14 @@ class CheckResult:
     stderr: str
 
 
-async def run_check(semaphore: asyncio.Semaphore, package: str, check: str, base_args: List[str], idx: int, total: int) -> CheckResult:
+async def run_check(
+    semaphore: asyncio.Semaphore,
+    package: str,
+    check: str,
+    base_args: List[str],
+    idx: int,
+    total: int,
+) -> CheckResult:
     """Run a single check (subprocess) within a concurrency semaphore, capturing output and timing.
 
     :param semaphore: Concurrency limiter used to bound concurrent checks.
@@ -65,7 +73,9 @@ async def run_check(semaphore: asyncio.Semaphore, package: str, check: str, base
         stderr = stderr_b.decode(errors="replace")
         exit_code = proc.returncode or 0
         status = "OK" if exit_code == 0 else f"FAIL({exit_code})"
-        logger.info(f"[END   {idx}/{total}] {check} :: {package} -> {status} in {duration:.2f}s")
+        logger.info(
+            f"[END   {idx}/{total}] {check} :: {package} -> {status} in {duration:.2f}s"
+        )
         # Print captured output after completion to avoid interleaving
         header = f"===== OUTPUT: {check} :: {package} (exit {exit_code}) ====="
         trailer = "=" * len(header)
@@ -74,7 +84,7 @@ async def run_check(semaphore: asyncio.Semaphore, package: str, check: str, base
             print(stdout.rstrip())
             print(trailer)
         if stderr:
-            print(header.replace('OUTPUT', 'STDERR'))
+            print(header.replace("OUTPUT", "STDERR"))
             print(stderr.rstrip())
             print(trailer)
 
@@ -86,7 +96,9 @@ async def run_check(semaphore: asyncio.Semaphore, package: str, check: str, base
             try:
                 shutil.rmtree(isolate_dir)
             except:
-                logger.warning(f"Failed to remove isolate dir {isolate_dir} for {package} / {check}")
+                logger.warning(
+                    f"Failed to remove isolate dir {isolate_dir} for {package} / {check}"
+                )
         return CheckResult(package, check, exit_code, duration, stdout, stderr)
 
 
@@ -110,10 +122,14 @@ def summarize(results: List[CheckResult]) -> int:
     print("-" * len(header))
     for r in sorted(results, key=lambda x: (x.exit_code != 0, x.package, x.check)):
         status = "OK" if r.exit_code == 0 else f"FAIL({r.exit_code})"
-        print(f"{r.package.ljust(pkg_w)}  {r.check.ljust(chk_w)}  {status.ljust(8)}  {r.duration:>10.2f}")
+        print(
+            f"{r.package.ljust(pkg_w)}  {r.check.ljust(chk_w)}  {status.ljust(8)}  {r.duration:>10.2f}"
+        )
     worst = max((r.exit_code for r in results), default=0)
     failed = [r for r in results if r.exit_code != 0]
-    print(f"\nTotal checks: {len(results)} | Failed: {len(failed)} | Worst exit code: {worst}")
+    print(
+        f"\nTotal checks: {len(results)} | Failed: {len(failed)} | Worst exit code: {worst}"
+    )
     return worst
 
 
@@ -135,10 +151,16 @@ async def run_all_checks(packages, checks, max_parallel):
     combos = [(p, c) for p in packages for c in checks]
     total = len(combos)
     for idx, (package, check) in enumerate(combos, start=1):
-        if not is_check_enabled(package, check):
-            logger.warning(f"Skipping disabled check {check} ({idx}/{total}) for package {package}")
+        if not is_check_enabled(package, check, CHECK_DEFAULTS.get(check, True)):
+            logger.warning(
+                f"Skipping disabled check {check} ({idx}/{total}) for package {package}"
+            )
             continue
-        tasks.append(asyncio.create_task(run_check(semaphore, package, check, base_args, idx, total)))
+        tasks.append(
+            asyncio.create_task(
+                run_check(semaphore, package, check, base_args, idx, total)
+            )
+        )
 
     # Handle Ctrl+C gracefully
     pending = set(tasks)
@@ -157,7 +179,9 @@ async def run_all_checks(packages, checks, max_parallel):
         elif isinstance(res, Exception):
             norm_results.append(CheckResult(package, check, 99, 0.0, "", str(res)))
         else:
-            norm_results.append(CheckResult(package, check, 98, 0.0, "", f"Unknown result type: {res}"))
+            norm_results.append(
+                CheckResult(package, check, 98, 0.0, "", f"Unknown result type: {res}")
+            )
     return summarize(norm_results)
 
 
@@ -233,11 +257,15 @@ def handler(signum, frame):
         ),
     )
 
-    parser.add_argument("--disablecov", help=("Flag. Disables code coverage."), action="store_true")
+    parser.add_argument(
+        "--disablecov", help=("Flag. Disables code coverage."), action="store_true"
+    )
 
     parser.add_argument(
         "--service",
-        help=("Name of service directory (under sdk/) to test. Example: --service applicationinsights"),
+        help=(
+            "Name of service directory (under sdk/) to test. Example: --service applicationinsights"
+        ),
     )
 
     parser.add_argument(
@@ -297,7 +325,9 @@ def handler(signum, frame):
     else:
         target_dir = root_dir
 
-    logger.info(f"Beginning discovery for {args.service} and root dir {root_dir}. Resolving to {target_dir}.")
+    logger.info(
+        f"Beginning discovery for {args.service} and root dir {root_dir}. Resolving to {target_dir}."
+    )
 
     # ensure that recursive virtual envs aren't messed with by this call
     os.environ.pop("VIRTUAL_ENV", None)
@@ -314,7 +344,9 @@ def handler(signum, frame):
     )
 
     if len(targeted_packages) == 0:
-        logger.info(f"No packages collected for targeting string {args.glob_string} and root dir {root_dir}. Exit 0.")
+        logger.info(
+            f"No packages collected for targeting string {args.glob_string} and root dir {root_dir}. Exit 0."
+        )
         exit(0)
 
     logger.info(f"Executing checks with the executable {sys.executable}.")
@@ -329,7 +361,9 @@ def handler(signum, frame):
     if in_ci():
         # prepare a build of eng/tools/azure-sdk-tools
         # todo: ensure that we honor this .wheels directory when replacing for dev reqs
-        build_whl_for_req("eng/tools/azure-sdk-tools", root_dir, os.path.join(root_dir, ".wheels"))
+        build_whl_for_req(
+            "eng/tools/azure-sdk-tools", root_dir, os.path.join(root_dir, ".wheels")
+        )
 
     # so if we have checks whl,import_all and selected package paths `sdk/core/azure-core`, `sdk/storage/azure-storage-blob` we should
     # shell out to `azypysdk <checkname>` with cwd of the package directory, which is what is in `targeted_packages` array
@@ -342,11 +376,15 @@ def handler(signum, frame):
         logger.error("No valid checks provided via -c/--checks.")
         sys.exit(2)
 
-    logger.info(f"Running {len(checks)} check(s) across {len(targeted_packages)} packages (max_parallel={args.max_parallel}).")
+    logger.info(
+        f"Running {len(checks)} check(s) across {len(targeted_packages)} packages (max_parallel={args.max_parallel})."
+    )
 
     configure_interrupt_handling()
     try:
-        exit_code = asyncio.run(run_all_checks(targeted_packages, checks, args.max_parallel))
+        exit_code = asyncio.run(
+            run_all_checks(targeted_packages, checks, args.max_parallel)
+        )
     except KeyboardInterrupt:
         logger.error("Aborted by user.")
         exit_code = 130

eng/tools/azure-sdk-tools/azpysdk/black.py

@@ -60,7 +60,7 @@ def run(self, args: argparse.Namespace) -> int:
             config_file_location = os.path.join(REPO_ROOT, "eng/black-pyproject.toml")
 
             if in_ci():
-                if not is_check_enabled(package_dir, "black"):
+                if not is_check_enabled(package_dir, "black", default=False):
                     logger.info(f"Package {package_name} opts-out of black check.")
                     continue
             try:
@@ -76,6 +76,7 @@ def run(self, args: argparse.Namespace) -> int:
                     stderr=subprocess.PIPE,
                     check=True,
                 )
+
                 if run_result.stderr and "reformatted" in run_result.stderr.decode("utf-8"):
                     if in_ci():
                         logger.info(f"The package {package_name} needs reformat. Run `black` locally to reformat.")