[WIP] fix(http-client-python): coerce string boolean args in run_batch.py

.chronus/changes/fix-keep-setup-py-coercion-2026-4-30-5-30-0.md

@@ -0,0 +1,7 @@
+---
+changeKind: internal
+packages:
+  - "@typespec/http-client-python"
+---
+
+Fix `run_batch.py` passing string `"true"`/`"false"` values to pygen, which caused `keep-setup-py="false"` to be treated as truthy and generate `setup.py` instead of `pyproject.toml` for all regenerated test packages.

packages/http-client-python/emitter/src/types.ts

@@ -545,6 +545,22 @@ export const KnownTypes = {
   any: { type: "any" },
 };
 
+/**
+ * Detect whether an endpoint template parameter should be treated as an API
+ * version parameter even when TCGC does not flag it as such.  This is a
+ * compatibility shim for unreleased TCGC changes that gate the name-based
+ * heuristic on `isMetadata`, which excludes server template parameters.
+ */
+function isEndpointApiVersionFallback(
+  param: { name: string; isApiVersionParam: boolean },
+  serviceApiVersions: string[],
+): boolean {
+  if (param.isApiVersionParam) return false;
+  if (serviceApiVersions.length === 0) return false;
+  const lower = param.name.toLowerCase();
+  return lower === "apiversion" || lower === "api-version";
+}
+
 export function emitEndpointType(
   context: PythonSdkContext,
   type: SdkEndpointType,
@@ -554,13 +570,29 @@ export function emitEndpointType(
   for (const param of type.templateArguments) {
     const paramBase = emitParamBase(context, param, undefined, serviceApiVersions);
     paramBase.clientName = context.arm ? "base_url" : paramBase.clientName;
+
+    let effectiveClientDefaultValue = param.clientDefaultValue;
+    // If this endpoint template param looks like an api-version but TCGC
+    // did not flag it, apply fallback: mark as api version and derive defaults.
+    if (isEndpointApiVersionFallback(param, serviceApiVersions)) {
+      paramBase.isApiVersion = true;
+      if (!effectiveClientDefaultValue) {
+        effectiveClientDefaultValue = serviceApiVersions[serviceApiVersions.length - 1];
+      }
+      paramBase.type = getSimpleTypeResult(context, {
+        type: "constant",
+        value: effectiveClientDefaultValue,
+        valueType: paramBase.type,
+      });
+    }
+
     params.push({
       ...paramBase,
-      optional: Boolean(param.clientDefaultValue),
+      optional: Boolean(effectiveClientDefaultValue),
       wireName: param.name,
       location: "endpointPath",
       implementation: getImplementation(context, param),
-      clientDefaultValue: param.clientDefaultValue,
+      clientDefaultValue: effectiveClientDefaultValue,
       skipUrlEncoding: param.allowReserved,
     });
     context.__endpointPathParameters!.push(params.at(-1)!);

packages/http-client-python/eng/scripts/ci/run-tests.ts

@@ -80,6 +80,7 @@ interface ToxResult {
   success: boolean;
   duration: number;
   error?: string;
+  failedTests?: string[];
 }
 
 interface RunningTask {
@@ -88,6 +89,29 @@ interface RunningTask {
   promise: Promise<ToxResult>;
 }
 
+function extractFailedTests(output: string): string[] {
+  const lines = output.split("\n");
+  const failedTests: string[] = [];
+  let inSummary = false;
+
+  for (const line of lines) {
+    if (line.includes("short test summary info")) {
+      inSummary = true;
+      continue;
+    }
+    if (inSummary) {
+      const match = line.match(/^FAILED\s+(.+)/);
+      if (match) {
+        failedTests.push(match[1].trim());
+      } else if (line.match(/^={3,}/) || line.match(/^\d+ failed/)) {
+        break;
+      }
+    }
+  }
+
+  return failedTests;
+}
+
 function startToxEnv(env: string, pythonPath: string, name?: string): RunningTask {
   const startTime = Date.now();
   const toxIniPath = join(testsDir, "tox.ini");
@@ -102,14 +126,28 @@ function startToxEnv(env: string, pythonPath: string, name?: string): RunningTas
 
   const proc: ChildProcess = spawn(pythonPath, args, {
     cwd: testsDir,
-    stdio: !argv.values.quiet ? "inherit" : "pipe",
+    stdio: "pipe",
     env: { ...process.env, FOLDER: env.split("-")[1] || "azure" },
   });
 
   let stderr = "";
-  if (argv.values.quiet && proc.stderr) {
+  let stdout = "";
+  if (proc.stdout) {
+    proc.stdout.on("data", (data) => {
+      const chunk = data.toString();
+      stdout += chunk;
+      if (!argv.values.quiet) {
+        process.stdout.write(chunk);
+      }
+    });
+  }
+  if (proc.stderr) {
     proc.stderr.on("data", (data) => {
-      stderr += data.toString();
+      const chunk = data.toString();
+      stderr += chunk;
+      if (!argv.values.quiet) {
+        process.stderr.write(chunk);
+      }
     });
   }
 
@@ -124,11 +162,14 @@ function startToxEnv(env: string, pythonPath: string, name?: string): RunningTas
         console.log(`${pc.red("[FAIL]")} ${env} (${duration.toFixed(1)}s)`);
       }
 
+      const failedTests = extractFailedTests(stdout);
+
       resolve({
         env,
         success,
         duration,
         error: success ? undefined : stderr || `Exit code: ${code}`,
+        failedTests: failedTests.length > 0 ? failedTests : undefined,
       });
     });
 
@@ -218,7 +259,8 @@ async function _runSequential(
 ): Promise<ToxResult[]> {
   const results: ToxResult[] = [];
   for (const env of envs) {
-    const result = await runToxEnv(env, pythonPath, name);
+    const task = startToxEnv(env, pythonPath, name);
+    const result = await task.promise;
     results.push(result);
   }
   return results;
@@ -251,7 +293,11 @@ function printSummary(results: ToxResult[]): void {
     console.log(pc.red("Failed environments:"));
     for (const result of failed) {
       console.log(`  - ${result.env}`);
-      if (result.error && !argv.values.quiet) {
+      if (result.failedTests && result.failedTests.length > 0) {
+        for (const test of result.failedTests) {
+          console.log(`    ${pc.red("FAILED")} ${test}`);
+        }
+      } else if (result.error && !argv.values.quiet) {
         console.log(`    ${result.error.split("\n").slice(0, 5).join("\n    ")}`);
       }
     }

packages/http-client-python/eng/scripts/setup/run_batch.py

@@ -39,8 +39,21 @@ def process_single_spec(config_path_str: str) -> tuple[str, bool, str]:
         output_dir = config["outputDir"]
 
         # Pass command args directly to pygen - pygen expects hyphenated keys
-        # Remove keys that shouldn't be passed to pygen
-        pygen_args = {k: v for k, v in command_args.items() if k not in ["emit-yaml-only"]}
+        # Remove keys that shouldn't be passed to pygen.
+        # Also coerce the string values "true"/"false" to real booleans, matching the behavior
+        # of pygen.utils.parse_args (the CLI path). Without this, the emitter passes string
+        # "false" for flags like keep-setup-py, which is truthy in Python and causes pygen to
+        # take the wrong branch (e.g. generating setup.py instead of pyproject.toml).
+        def _coerce(value):
+            if value == "true":
+                return True
+            if value == "false":
+                return False
+            return value
+
+        pygen_args = {
+            k: _coerce(v) for k, v in command_args.items() if k not in ["emit-yaml-only"]
+        }
 
         # Run preprocess and codegen (black is batched at the end for performance)
         preprocess.PreProcessPlugin(output_folder=output_dir, tsp_file=yaml_path, **pygen_args).process()

packages/http-client-python/generator/pygen/codegen/models/parameter.py

@@ -377,6 +377,8 @@ def is_host(self) -> bool:
     def method_location(self) -> ParameterMethodLocation:
         if self.constant:
             return ParameterMethodLocation.KWARG
+        if self.is_api_version:
+            return ParameterMethodLocation.KWARG
         if (
             self.is_host
             and (self.code_model.options["version-tolerant"] or self.code_model.options["low-level-client"])
@@ -408,6 +410,6 @@ def is_host(self) -> bool:
 
     @property
     def method_location(self) -> ParameterMethodLocation:
-        if self.constant:
+        if self.constant or self.is_api_version:
             return ParameterMethodLocation.KWARG
         return ParameterMethodLocation.POSITIONAL

packages/http-client-python/generator/pygen/codegen/serializers/__init__.py

@@ -42,15 +42,15 @@
     "JinjaSerializer",
 ]
 
-_PACKAGE_FILES = [
+_DEFAULT_PACKAGE_FILES = (
     "CHANGELOG.md.jinja2",
     "dev_requirements.txt.jinja2",
     "LICENSE.jinja2",
     "MANIFEST.in.jinja2",
     "README.md.jinja2",
-]
+)
 
-_REGENERATE_FILES = {"MANIFEST.in"}
+_DEFAULT_REGENERATE_FILES = frozenset({"MANIFEST.in"})
 AsyncInfo = namedtuple("AsyncInfo", ["async_mode", "async_path"])
 
 
@@ -75,15 +75,17 @@ def __init__(
     ) -> None:
         super().__init__(output_folder=output_folder, **kwargs)
         self.code_model = code_model
+        self._package_files: list[str] = list(_DEFAULT_PACKAGE_FILES)
+        self._regenerate_files: set[str] = set(_DEFAULT_REGENERATE_FILES)
         self._regenerate_setup_py()
 
     def _regenerate_setup_py(self):
         if self.code_model.options["keep-setup-py"] or self.code_model.options["basic-setup-py"]:
-            _PACKAGE_FILES.append("setup.py.jinja2")
-            _REGENERATE_FILES.add("setup.py")
+            self._package_files.append("setup.py.jinja2")
+            self._regenerate_files.add("setup.py")
         else:
-            _PACKAGE_FILES.append("pyproject.toml.jinja2")
-            _REGENERATE_FILES.add("pyproject.toml")
+            self._package_files.append("pyproject.toml.jinja2")
+            self._regenerate_files.add("pyproject.toml")
 
     @property
     def has_aio_folder(self) -> bool:
@@ -253,7 +255,7 @@ def _serialize_and_write_package_files(self) -> None:
                 lstrip_blocks=True,
             )
 
-            package_files = list(_PACKAGE_FILES)  # Copy to avoid modifying global
+            package_files = list(self._package_files)
             if not self.code_model.license_description:
                 package_files.remove("LICENSE.jinja2")
         elif Path(self.code_model.options["package-mode"]).exists():
@@ -272,7 +274,7 @@ def _serialize_and_write_package_files(self) -> None:
                 continue
             file = template_name.replace(".jinja2", "")
             output_file = root_of_sdk / file
-            if not self.read_file(output_file) or file in _REGENERATE_FILES:
+            if not self.read_file(output_file) or file in self._regenerate_files:
                 if self.keep_version_file and file == "setup.py" and not self.code_model.options["azure-arm"]:
                     # don't regenerate setup.py file if the version file is more up to date for data-plane
                     continue

packages/http-client-python/generator/pygen/codegen/serializers/parameter_serializer.py

@@ -202,11 +202,13 @@ def append_pop_kwarg(key: str, pop_type: PopKwargType) -> None:
                 if is_content_type_optional and not type_annotation.startswith("Optional[")
                 else type_annotation
             )
-            if kwarg.client_default_value is not None or kwarg.optional or kwarg.constant:
+            if kwarg.client_default_value is not None or kwarg.optional or kwarg.constant or kwarg.is_api_version:
                 if check_client_input and kwarg.check_client_input:
                     default_value = f"self._config.{kwarg.client_name}"
                 elif kwarg.constant:
                     default_value = kwarg.type.get_declaration(None)
+                elif kwarg.is_api_version and kwarg.client_default_value is None and kwarg.api_versions:
+                    default_value = f'"{kwarg.api_versions[-1]}"'
                 else:
                     default_value = kwarg.client_default_value_declaration
                 if check_kwarg_dict and (kwarg.location in [ParameterLocation.HEADER, ParameterLocation.QUERY]):

packages/http-client-python/tests/unit/test_generated_packaging_files.py

@@ -0,0 +1,65 @@
+# -------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for
+# license information.
+# --------------------------------------------------------------------------
+"""Verify regenerated test packages emit pyproject.toml, not setup.py.
+
+The Python emitter passes options to pygen via JSON config files. Boolean
+flags arrive as the strings "true"/"false". If those strings are not coerced
+to real booleans before being unpacked as kwargs to pygen, a value like
+keep-setup-py="false" is treated as truthy in Python and pygen emits
+setup.py instead of pyproject.toml. This test guards against that
+regression by walking the regenerated test fixtures.
+"""
+from pathlib import Path
+
+import pytest
+
+# Packages that intentionally opt into setup.py via keep-setup-py=true in
+# eng/scripts/ci/regenerate.ts. They should have setup.py and no pyproject.toml.
+KEEP_SETUP_PY_PACKAGES = {"setuppy-authentication-union"}
+
+_GENERATED_ROOT = Path(__file__).resolve().parents[1] / "generated"
+
+
+def _iter_generated_packages():
+    if not _GENERATED_ROOT.is_dir():
+        return
+    for flavor_dir in sorted(_GENERATED_ROOT.iterdir()):
+        if not flavor_dir.is_dir():
+            continue
+        for pkg_dir in sorted(flavor_dir.iterdir()):
+            if not pkg_dir.is_dir():
+                continue
+            yield flavor_dir.name, pkg_dir
+
+
+@pytest.mark.parametrize(
+    "flavor,pkg_dir",
+    list(_iter_generated_packages()),
+    ids=lambda v: v.name if isinstance(v, Path) else v,
+)
+def test_generated_package_uses_pyproject_toml(flavor, pkg_dir):
+    has_setup_py = (pkg_dir / "setup.py").is_file()
+    has_pyproject = (pkg_dir / "pyproject.toml").is_file()
+
+    if pkg_dir.name in KEEP_SETUP_PY_PACKAGES:
+        assert has_setup_py, f"{flavor}/{pkg_dir.name} should have setup.py (keep-setup-py=true)"
+        assert not has_pyproject, (
+            f"{flavor}/{pkg_dir.name} should NOT have pyproject.toml (keep-setup-py=true)"
+        )
+        return
+
+    assert has_pyproject, (
+        f"{flavor}/{pkg_dir.name} is missing pyproject.toml. "
+        f"This usually means string boolean args (e.g. keep-setup-py=\"false\") "
+        f"were not coerced to real booleans before being passed to pygen. "
+        f"See eng/scripts/setup/run_batch.py."
+    )
+    assert not has_setup_py, (
+        f"{flavor}/{pkg_dir.name} has setup.py but should use pyproject.toml. "
+        f"This usually means string boolean args (e.g. keep-setup-py=\"false\") "
+        f"were not coerced to real booleans before being passed to pygen. "
+        f"See eng/scripts/setup/run_batch.py."
+    )