refactor install and test (#44482)

* refactor install and test

* refactored optional and i think it's working

* minor clean

* optional actually working properly

* bug fix

* bug fix

* minor exception handling revisions

Author: JennyPng

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

@@ -59,31 +59,11 @@ def run(self, args: argparse.Namespace) -> int:
             executable, staging_directory = self.get_executable(args.isolate, args.command, sys.executable, package_dir)
             logger.info(f"Processing {package_name} using interpreter {executable}")
 
-            try:
-                self._install_common_requirements(executable, package_dir)
-                if self.should_install_dev_requirements():
-                    self.install_dev_reqs(executable, args, package_dir)
-                self.after_dependencies_installed(executable, package_dir, staging_directory, args)
-            except CalledProcessError as exc:
-                logger.error(f"Failed to prepare dependencies for {package_name}: {exc}")
-                results.append(exc.returncode)
-                continue
-
-            try:
-                create_package_and_install(
-                    distribution_directory=staging_directory,
-                    target_setup=package_dir,
-                    skip_install=False,
-                    cache_dir=None,
-                    work_dir=staging_directory,
-                    force_create=False,
-                    package_type=self.package_type,
-                    pre_download_disabled=False,
-                    python_executable=executable,
-                )
-            except CalledProcessError as exc:
-                logger.error(f"Failed to build/install {self.package_type} for {package_name}: {exc}")
-                results.append(1)
+            install_result = self.install_all_requiremenmts(
+                executable, staging_directory, package_name, package_dir, args
+            )
+            if install_result != 0:
+                results.append(install_result)
                 continue
 
             try:
@@ -94,47 +74,93 @@ def run(self, args: argparse.Namespace) -> int:
                 continue
 
             pytest_args = self._build_pytest_args(package_dir, args)
-            pytest_command = ["-m", "pytest", *pytest_args]
-            pytest_result = self.run_venv_command(
-                executable, pytest_command, cwd=staging_directory, immediately_dump=True
-            )
-
-            if pytest_result.returncode != 0:
-                if pytest_result.returncode == 5 and is_error_code_5_allowed(package_dir, package_name):
-                    logger.info(
-                        "pytest exited with code 5 for %s, which is allowed for management or opt-out packages.",
-                        package_name,
-                    )
-                    # Align with tox: skip coverage when tests are skipped entirely
-                    continue
-                else:
-                    results.append(pytest_result.returncode)
-                    logger.error(f"pytest failed for {package_name} with exit code {pytest_result.returncode}.")
-                    continue
+            pytest_result = self.run_pytest(executable, staging_directory, package_dir, package_name, pytest_args)
+            if pytest_result != 0:
+                results.append(pytest_result)
+                continue
 
             if not self.coverage_enabled:
                 continue
 
-            coverage_command = [
-                os.path.join(REPO_ROOT, "eng/tox/run_coverage.py"),
-                "-t",
-                package_dir,
-                "-r",
-                REPO_ROOT,
-            ]
-            coverage_result = self.run_venv_command(executable, coverage_command, cwd=package_dir)
-            if coverage_result.returncode != 0:
-                logger.error(
-                    f"Coverage generation failed for {package_name} with exit code {coverage_result.returncode}."
-                )
-                if coverage_result.stdout:
-                    logger.error(coverage_result.stdout)
-                if coverage_result.stderr:
-                    logger.error(coverage_result.stderr)
-                results.append(coverage_result.returncode)
+            coverage_result = self.check_coverage(executable, package_dir, package_name)
+            if coverage_result != 0:
+                results.append(coverage_result)
 
         return max(results) if results else 0
 
+    def check_coverage(self, executable: str, package_dir: str, package_name: str) -> int:
+        coverage_command = [
+            os.path.join(REPO_ROOT, "eng/tox/run_coverage.py"),
+            "-t",
+            package_dir,
+            "-r",
+            REPO_ROOT,
+        ]
+        coverage_result = self.run_venv_command(executable, coverage_command, cwd=package_dir)
+        if coverage_result.returncode != 0:
+            logger.error(f"Coverage generation failed for {package_name} with exit code {coverage_result.returncode}.")
+            if coverage_result.stdout:
+                logger.error(coverage_result.stdout)
+            if coverage_result.stderr:
+                logger.error(coverage_result.stderr)
+            return coverage_result.returncode
+        return 0
+
+    def run_pytest(
+        self,
+        executable: str,
+        staging_directory: str,
+        package_dir: str,
+        package_name: str,
+        pytest_args: List[str],
+        cwd: Optional[str] = None,
+    ) -> int:
+        pytest_command = ["-m", "pytest", *pytest_args]
+        pytest_result = self.run_venv_command(
+            executable, pytest_command, cwd=(cwd or staging_directory), immediately_dump=True
+        )
+        if pytest_result.returncode != 0:
+            if pytest_result.returncode == 5 and is_error_code_5_allowed(package_dir, package_name):
+                logger.info(
+                    "pytest exited with code 5 for %s, which is allowed for management or opt-out packages.",
+                    package_name,
+                )
+                # Align with tox: skip coverage when tests are skipped entirely
+                return 0
+            else:
+                logger.error(f"pytest failed for {package_name} with exit code {pytest_result.returncode}.")
+                return pytest_result.returncode
+        return 0
+
+    def install_all_requiremenmts(
+        self, executable: str, staging_directory: str, package_name: str, package_dir: str, args: argparse.Namespace
+    ) -> int:
+        try:
+            self._install_common_requirements(executable, package_dir)
+            if self.should_install_dev_requirements():
+                self.install_dev_reqs(executable, args, package_dir)
+            self.after_dependencies_installed(executable, package_dir, staging_directory, args)
+        except CalledProcessError as exc:
+            logger.error(f"Failed to prepare dependencies for {package_name}: {exc}")
+            return exc.returncode or 1
+
+        try:
+            create_package_and_install(
+                distribution_directory=staging_directory,
+                target_setup=package_dir,
+                skip_install=False,
+                cache_dir=None,
+                work_dir=staging_directory,
+                force_create=False,
+                package_type=self.package_type,
+                pre_download_disabled=False,
+                python_executable=executable,
+            )
+        except CalledProcessError as exc:
+            logger.error(f"Failed to build/install {self.package_type} for {package_name}: {exc}")
+            exit(1)
+        return 0
+
     def get_env_defaults(self) -> Dict[str, str]:
         defaults: Dict[str, str] = {}
         if self.proxy_url:

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

@@ -5,24 +5,22 @@
 
 from typing import Optional, List
 
-from .Check import Check
+from .install_and_test import InstallAndTest
 from ci_tools.functions import (
     install_into_venv,
     uninstall_from_venv,
-    is_error_code_5_allowed,
 )
-from ci_tools.scenario.generation import create_package_and_install, prepare_environment
-from ci_tools.variables import discover_repo_root, in_ci, set_envvar_defaults
-from ci_tools.environment_exclusions import is_check_enabled
+from ci_tools.scenario.generation import prepare_environment
+from ci_tools.variables import discover_repo_root, set_envvar_defaults
 from ci_tools.parsing import get_config_setting
 from ci_tools.logging import logger
 
 REPO_ROOT = discover_repo_root()
 
 
-class optional(Check):
+class optional(InstallAndTest):
     def __init__(self) -> None:
-        super().__init__()
+        super().__init__(package_type="sdist", proxy_url="http://localhost:5004", display_name="optional")
 
     def register(
         self, subparsers: "argparse._SubParsersAction", parent_parsers: Optional[List[argparse.ArgumentParser]] = None
@@ -48,31 +46,29 @@ def run(self, args: argparse.Namespace) -> int:
         """Run the optional check command."""
         logger.info("Running optional check...")
 
-        set_envvar_defaults({"PROXY_URL": "http://localhost:5004"})
+        env_defaults = self.get_env_defaults()
+        if env_defaults:
+            set_envvar_defaults(env_defaults)
+
         targeted = self.get_targeted_directories(args)
+        if not targeted:
+            logger.warning("No target packages discovered for optional check.")
+            return 0
 
         results: List[int] = []
 
         for parsed in targeted:
             package_dir = parsed.folder
             package_name = parsed.name
             executable, staging_directory = self.get_executable(args.isolate, args.command, sys.executable, package_dir)
-            logger.info(f"Processing {package_name} for optional check")
-
-            if in_ci():
-                if not is_check_enabled(package_dir, "optional", False):
-                    logger.info(f"Package {package_name} opts-out of optional check.")
-                    continue
-
-            try:
-                self.install_dev_reqs(executable, args, package_dir)
-            except CalledProcessError as exc:
-                logger.error(f"Failed to install dependencies for {package_name}: {exc}")
-                results.append(exc.returncode)
-                continue
+            logger.info(f"Processing {package_name} using interpreter {executable}")
 
             try:
-                self.prepare_and_test_optional(package_name, package_dir, staging_directory, args.optional)
+                result = self.prepare_and_test_optional(
+                    package_name, package_dir, staging_directory, args.optional, args
+                )
+                if result != 0:
+                    results.append(result)
             except Exception as e:
                 logger.error(f"Optional check for package {package_name} failed with exception: {e}")
                 results.append(1)
@@ -83,16 +79,19 @@ def run(self, args: argparse.Namespace) -> int:
     # TODO copying from generation.py, remove old code later
     # TODO remove pytest() function from ci_tools.functions as it was only used in the old version of this logic
     def prepare_and_test_optional(
-        self, package_name: str, package_dir: str, temp_dir: str, target_env_name: str
-    ) -> None:
+        self, package_name: str, package_dir: str, temp_dir: str, target_env_name: str, args: argparse.Namespace
+    ) -> int:
         """
         Prepare and test the optional environment for the given package.
         """
         optional_configs = get_config_setting(package_dir, "optional")
 
+        if not isinstance(optional_configs, list):
+            optional_configs = []
+
         if len(optional_configs) == 0:
             logger.info(f"No optional environments detected in pyproject.toml within {package_dir}.")
-            exit(0)
+            return 0
 
         config_results = []
 
@@ -109,26 +108,18 @@ def prepare_and_test_optional(
 
             environment_exe = prepare_environment(package_dir, temp_dir, env_name)
 
-            create_package_and_install(
-                distribution_directory=temp_dir,
-                target_setup=package_dir,
-                skip_install=False,
-                cache_dir=None,
-                work_dir=temp_dir,
-                force_create=False,
-                package_type="sdist",
-                pre_download_disabled=False,
-                python_executable=environment_exe,
-            )
-            dev_reqs = os.path.join(package_dir, "dev_requirements.txt")
-            test_tools = os.path.join(REPO_ROOT, "eng", "test_tools.txt")
-
-            # install the dev requirements and test_tools requirements files to ensure tests can run
+            # install package and testing requirements
             try:
-                install_into_venv(environment_exe, ["-r", dev_reqs, "-r", test_tools], package_dir)
+                install_result = self.install_all_requiremenmts(
+                    environment_exe, temp_dir, package_name, package_dir, args
+                )
+                if install_result != 0:
+                    logger.error(f"Failed to install base requirements for {package_name} in optional env {env_name}.")
+                    config_results.append(False)
+                    break
             except CalledProcessError as exc:
                 logger.error(
-                    f"Unable to complete installation of dev_requirements.txt and/or test_tools.txt for {package_name}, check command output above."
+                    f"Failed to install base requirements for {package_name} in optional env {env_name}: {exc}"
                 )
                 config_results.append(False)
                 break
@@ -181,30 +172,16 @@ def prepare_and_test_optional(
 
             logger.info(f"Invoking tests for package {package_name} and optional environment {env_name}")
 
-            pytest_command = ["-m", "pytest", *pytest_args]
-            pytest_result = self.run_venv_command(
-                environment_exe, pytest_command, cwd=package_dir, immediately_dump=True
-            )
-
-            if pytest_result.returncode != 0:
-                if pytest_result.returncode == 5 and is_error_code_5_allowed(package_dir, package_name):
-                    logger.info(
-                        "pytest exited with code 5 for %s, which is allowed for management or opt-out packages.",
-                        package_name,
-                    )
-                    # Align with tox: skip coverage when tests are skipped entirely
-                    continue
-                logger.error(
-                    f"pytest failed for {package_name} and optional environment {env_name} with exit code {pytest_result.returncode}."
+            try:
+                pytest_result = self.run_pytest(
+                    environment_exe, temp_dir, package_dir, package_name, pytest_args, cwd=package_dir
                 )
+                config_results.append(True if pytest_result == 0 else False)
+            except CalledProcessError as exc:
                 config_results.append(False)
-            else:
-                logger.info(f"pytest succeeded for {package_name} and optional environment {env_name}.")
-                config_results.append(True)
 
         if all(config_results):
             logger.info(f"All optional environment(s) for {package_name} completed successfully.")
-            exit(0)
         else:
             for i, config in enumerate(optional_configs):
                 if i >= len(config_results):
@@ -214,4 +191,5 @@ def prepare_and_test_optional(
                     logger.error(
                         f"Optional environment {config_name} for {package_name} completed with non-zero exit-code. Check test results above."
                     )
-            exit(1)
+            return 1
+        return 0