From 45e377566c6fbcbcfe19c2ed324136fef9a58666 Mon Sep 17 00:00:00 2001
From: Matt Clay <matt@mystile.com>
Date: Thu, 6 Jul 2017 16:14:44 -0700
Subject: [PATCH] Refactor ansible-test config classes. (#26505)

* Move Config classes from executor.py to config.py.
* Move Environment and Test config to config.py.
* Move Coverage/CoverageReport Config to config.py.
* Clean up type hints.
---
 test/runner/lib/config.py      | 226 +++++++++++++++++++++++++++++++++
 test/runner/lib/core_ci.py     |   5 +-
 test/runner/lib/cover.py       |  30 +----
 test/runner/lib/delegation.py  |  12 +-
 test/runner/lib/docker_util.py |   5 +-
 test/runner/lib/executor.py    | 125 ++----------------
 test/runner/lib/sanity.py      |   3 +
 test/runner/lib/test.py        |  37 +-----
 test/runner/lib/util.py        |  47 -------
 test/runner/test.py            |  11 +-
 10 files changed, 270 insertions(+), 231 deletions(-)
 create mode 100644 test/runner/lib/config.py

diff --git a/test/runner/lib/config.py b/test/runner/lib/config.py
new file mode 100644
index 00000000000..b64288349f8
--- /dev/null
+++ b/test/runner/lib/config.py
@@ -0,0 +1,226 @@
+"""Configuration classes."""
+
+from __future__ import absolute_import, print_function
+
+import os
+import sys
+
+from lib.util import (
+    CommonConfig,
+    is_shippable,
+    docker_qualify_image,
+)
+
+from lib.metadata import (
+    Metadata,
+)
+
+
+class EnvironmentConfig(CommonConfig):
+    """Configuration common to all commands which execute in an environment."""
+    def __init__(self, args, command):
+        """
+        :type args: any
+        """
+        super(EnvironmentConfig, self).__init__(args)
+
+        self.command = command
+
+        self.local = args.local is True
+
+        if args.tox is True or args.tox is False or args.tox is None:
+            self.tox = args.tox is True
+            self.tox_args = 0
+            self.python = args.python if 'python' in args else None  # type: str
+        else:
+            self.tox = True
+            self.tox_args = 1
+            self.python = args.tox  # type: str
+
+        self.docker = docker_qualify_image(args.docker)  # type: str
+        self.remote = args.remote  # type: str
+
+        self.docker_privileged = args.docker_privileged if 'docker_privileged' in args else False  # type: bool
+        self.docker_util = docker_qualify_image(args.docker_util if 'docker_util' in args else '')  # type: str
+        self.docker_pull = args.docker_pull if 'docker_pull' in args else False  # type: bool
+
+        self.tox_sitepackages = args.tox_sitepackages  # type: bool
+
+        self.remote_stage = args.remote_stage  # type: str
+        self.remote_aws_region = args.remote_aws_region  # type: str
+        self.remote_terminate = args.remote_terminate  # type: str
+
+        self.requirements = args.requirements  # type: bool
+
+        if self.python == 'default':
+            self.python = '.'.join(str(i) for i in sys.version_info[:2])
+
+        self.python_version = self.python or '.'.join(str(i) for i in sys.version_info[:2])
+
+        self.delegate = self.tox or self.docker or self.remote
+
+        if self.delegate:
+            self.requirements = True
+
+
+class TestConfig(EnvironmentConfig):
+    """Configuration common to all test commands."""
+    def __init__(self, args, command):
+        """
+        :type args: any
+        :type command: str
+        """
+        super(TestConfig, self).__init__(args, command)
+
+        self.coverage = args.coverage  # type: bool
+        self.coverage_label = args.coverage_label  # type: str
+        self.include = args.include  # type: list [str]
+        self.exclude = args.exclude  # type: list [str]
+        self.require = args.require  # type: list [str]
+
+        self.changed = args.changed  # type: bool
+        self.tracked = args.tracked  # type: bool
+        self.untracked = args.untracked  # type: bool
+        self.committed = args.committed  # type: bool
+        self.staged = args.staged  # type: bool
+        self.unstaged = args.unstaged  # type: bool
+        self.changed_from = args.changed_from  # type: str
+        self.changed_path = args.changed_path  # type: list [str]
+
+        self.lint = args.lint if 'lint' in args else False  # type: bool
+        self.junit = args.junit if 'junit' in args else False  # type: bool
+        self.failure_ok = args.failure_ok if 'failure_ok' in args else False  # type: bool
+
+        self.metadata = Metadata.from_file(args.metadata) if args.metadata else Metadata()
+        self.metadata_path = None
+
+
+class ShellConfig(EnvironmentConfig):
+    """Configuration for the shell command."""
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(ShellConfig, self).__init__(args, 'shell')
+
+
+class SanityConfig(TestConfig):
+    """Configuration for the sanity command."""
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(SanityConfig, self).__init__(args, 'sanity')
+
+        self.test = args.test  # type: list [str]
+        self.skip_test = args.skip_test  # type: list [str]
+        self.list_tests = args.list_tests  # type: bool
+
+        if args.base_branch:
+            self.base_branch = args.base_branch  # str
+        elif is_shippable():
+            self.base_branch = os.environ.get('BASE_BRANCH', '')  # str
+
+            if self.base_branch:
+                self.base_branch = 'origin/%s' % self.base_branch
+        else:
+            self.base_branch = ''
+
+
+class IntegrationConfig(TestConfig):
+    """Configuration for the integration command."""
+    def __init__(self, args, command):
+        """
+        :type args: any
+        :type command: str
+        """
+        super(IntegrationConfig, self).__init__(args, command)
+
+        self.start_at = args.start_at  # type: str
+        self.start_at_task = args.start_at_task  # type: str
+        self.allow_destructive = args.allow_destructive if 'allow_destructive' in args else False  # type: bool
+        self.retry_on_error = args.retry_on_error  # type: bool
+        self.tags = args.tags
+        self.skip_tags = args.skip_tags
+        self.diff = args.diff
+
+
+class PosixIntegrationConfig(IntegrationConfig):
+    """Configuration for the posix integration command."""
+
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(PosixIntegrationConfig, self).__init__(args, 'integration')
+
+
+class WindowsIntegrationConfig(IntegrationConfig):
+    """Configuration for the windows integration command."""
+
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(WindowsIntegrationConfig, self).__init__(args, 'windows-integration')
+
+        self.windows = args.windows  # type: list [str]
+
+        if self.windows:
+            self.allow_destructive = True
+
+
+class NetworkIntegrationConfig(IntegrationConfig):
+    """Configuration for the network integration command."""
+
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(NetworkIntegrationConfig, self).__init__(args, 'network-integration')
+
+        self.platform = args.platform  # type: list [str]
+
+
+class UnitsConfig(TestConfig):
+    """Configuration for the units command."""
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(UnitsConfig, self).__init__(args, 'units')
+
+        self.collect_only = args.collect_only  # type: bool
+
+
+class CompileConfig(TestConfig):
+    """Configuration for the compile command."""
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(CompileConfig, self).__init__(args, 'compile')
+
+
+class CoverageConfig(EnvironmentConfig):
+    """Configuration for the coverage command."""
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(CoverageConfig, self).__init__(args, 'coverage')
+
+        self.group_by = frozenset(args.group_by) if 'group_by' in args and args.group_by else set()  # type: frozenset [str]
+        self.all = args.all if 'all' in args else False  # type: bool
+        self.stub = args.stub if 'stub' in args else False  # type: bool
+
+
+class CoverageReportConfig(CoverageConfig):
+    """Configuration for the coverage report command."""
+    def __init__(self, args):
+        """
+        :type args: any
+        """
+        super(CoverageReportConfig, self).__init__(args)
+
+        self.show_missing = args.show_missing  # type: bool
diff --git a/test/runner/lib/core_ci.py b/test/runner/lib/core_ci.py
index f32619cdfd5..43487b3eca6 100644
--- a/test/runner/lib/core_ci.py
+++ b/test/runner/lib/core_ci.py
@@ -19,11 +19,14 @@ from lib.util import (
     ApplicationError,
     run_command,
     make_dirs,
-    EnvironmentConfig,
     display,
     is_shippable,
 )
 
+from lib.config import (
+    EnvironmentConfig,
+)
+
 AWS_ENDPOINTS = {
     'us-east-1': 'https://14blg63h2i.execute-api.us-east-1.amazonaws.com',
     'us-east-2': 'https://g5xynwbk96.execute-api.us-east-2.amazonaws.com',
diff --git a/test/runner/lib/cover.py b/test/runner/lib/cover.py
index c34a5ac3d84..432a9406cb9 100644
--- a/test/runner/lib/cover.py
+++ b/test/runner/lib/cover.py
@@ -13,11 +13,15 @@ from lib.target import (
 from lib.util import (
     display,
     ApplicationError,
-    EnvironmentConfig,
     run_command,
     common_environment,
 )
 
+from lib.config import (
+    CoverageConfig,
+    CoverageReportConfig,
+)
+
 from lib.executor import (
     Delegate,
     install_command_requirements,
@@ -244,27 +248,3 @@ def get_coverage_group(args, coverage_file):
             group += '=%s' % names[part]
 
     return group
-
-
-class CoverageConfig(EnvironmentConfig):
-    """Configuration for the coverage command."""
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(CoverageConfig, self).__init__(args, 'coverage')
-
-        self.group_by = frozenset(args.group_by) if 'group_by' in args and args.group_by else set()  # type: frozenset[str]
-        self.all = args.all if 'all' in args else False  # type: bool
-        self.stub = args.stub if 'stub' in args else False  # type: bool
-
-
-class CoverageReportConfig(CoverageConfig):
-    """Configuration for the coverage report command."""
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(CoverageReportConfig, self).__init__(args)
-
-        self.show_missing = args.show_missing  # type: bool
diff --git a/test/runner/lib/delegation.py b/test/runner/lib/delegation.py
index d194fe55ec8..81106aed866 100644
--- a/test/runner/lib/delegation.py
+++ b/test/runner/lib/delegation.py
@@ -12,15 +12,16 @@ import lib.thread
 
 from lib.executor import (
     SUPPORTED_PYTHON_VERSIONS,
+    create_shell_command,
+)
+
+from lib.config import (
+    TestConfig,
+    EnvironmentConfig,
     IntegrationConfig,
     ShellConfig,
     SanityConfig,
     UnitsConfig,
-    create_shell_command,
-)
-
-from lib.test import (
-    TestConfig,
 )
 
 from lib.core_ci import (
@@ -33,7 +34,6 @@ from lib.manage_ci import (
 
 from lib.util import (
     ApplicationError,
-    EnvironmentConfig,
     run_command,
     common_environment,
     pass_vars,
diff --git a/test/runner/lib/docker_util.py b/test/runner/lib/docker_util.py
index 4bd2609b3db..4d2fdeed0d4 100644
--- a/test/runner/lib/docker_util.py
+++ b/test/runner/lib/docker_util.py
@@ -12,12 +12,15 @@ from lib.executor import (
 
 from lib.util import (
     ApplicationError,
-    EnvironmentConfig,
     run_command,
     common_environment,
     display,
 )
 
+from lib.config import (
+    EnvironmentConfig,
+)
+
 BUFFER_SIZE = 256 * 256
 
 
diff --git a/test/runner/lib/executor.py b/test/runner/lib/executor.py
index 308c8e61856..f4b58a8cc8c 100644
--- a/test/runner/lib/executor.py
+++ b/test/runner/lib/executor.py
@@ -37,7 +37,6 @@ from lib.cloud import (
 )
 
 from lib.util import (
-    EnvironmentConfig,
     ApplicationWarning,
     ApplicationError,
     SubprocessError,
@@ -52,10 +51,6 @@ from lib.util import (
     raw_command,
 )
 
-from lib.test import (
-    TestConfig,
-)
-
 from lib.ansible_util import (
     ansible_environment,
 )
@@ -84,6 +79,18 @@ from lib.classification import (
     categorize_changes,
 )
 
+from lib.config import (
+    TestConfig,
+    EnvironmentConfig,
+    CompileConfig,
+    IntegrationConfig,
+    NetworkIntegrationConfig,
+    PosixIntegrationConfig,
+    ShellConfig,
+    UnitsConfig,
+    WindowsIntegrationConfig,
+)
+
 from lib.test import (
     TestMessage,
     TestSuccess,
@@ -857,6 +864,7 @@ def intercept_command(args, cmd, target_name, capture=False, env=None, data=None
     """
     :type args: TestConfig
     :type cmd: collections.Iterable[str]
+    :type target_name: str
     :type capture: bool
     :type env: dict[str, str] | None
     :type data: str | None
@@ -1292,113 +1300,6 @@ class NoTestsForChanges(ApplicationWarning):
         super(NoTestsForChanges, self).__init__('No tests found for detected changes.')
 
 
-class ShellConfig(EnvironmentConfig):
-    """Configuration for the shell command."""
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(ShellConfig, self).__init__(args, 'shell')
-
-
-class SanityConfig(TestConfig):
-    """Configuration for the sanity command."""
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(SanityConfig, self).__init__(args, 'sanity')
-
-        self.test = args.test  # type: list [str]
-        self.skip_test = args.skip_test  # type: list [str]
-        self.list_tests = args.list_tests  # type: bool
-
-        if args.base_branch:
-            self.base_branch = args.base_branch  # str
-        elif is_shippable():
-            self.base_branch = os.environ.get('BASE_BRANCH', '')  # str
-
-            if self.base_branch:
-                self.base_branch = 'origin/%s' % self.base_branch
-        else:
-            self.base_branch = ''
-
-
-class IntegrationConfig(TestConfig):
-    """Configuration for the integration command."""
-    def __init__(self, args, command):
-        """
-        :type args: any
-        :type command: str
-        """
-        super(IntegrationConfig, self).__init__(args, command)
-
-        self.start_at = args.start_at  # type: str
-        self.start_at_task = args.start_at_task  # type: str
-        self.allow_destructive = args.allow_destructive if 'allow_destructive' in args else False  # type: bool
-        self.retry_on_error = args.retry_on_error  # type: bool
-        self.tags = args.tags
-        self.skip_tags = args.skip_tags
-        self.diff = args.diff
-
-
-class PosixIntegrationConfig(IntegrationConfig):
-    """Configuration for the posix integration command."""
-
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(PosixIntegrationConfig, self).__init__(args, 'integration')
-
-
-class WindowsIntegrationConfig(IntegrationConfig):
-    """Configuration for the windows integration command."""
-
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(WindowsIntegrationConfig, self).__init__(args, 'windows-integration')
-
-        self.windows = args.windows  # type: list [str]
-
-        if self.windows:
-            self.allow_destructive = True
-
-
-class NetworkIntegrationConfig(IntegrationConfig):
-    """Configuration for the network integration command."""
-
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(NetworkIntegrationConfig, self).__init__(args, 'network-integration')
-
-        self.platform = args.platform  # type list [str]
-
-
-class UnitsConfig(TestConfig):
-    """Configuration for the units command."""
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(UnitsConfig, self).__init__(args, 'units')
-
-        self.collect_only = args.collect_only  # type: bool
-
-
-class CompileConfig(TestConfig):
-    """Configuration for the compile command."""
-    def __init__(self, args):
-        """
-        :type args: any
-        """
-        super(CompileConfig, self).__init__(args, 'compile')
-
-
 class Delegate(Exception):
     """Trigger command delegation."""
     def __init__(self, exclude=None, require=None):
diff --git a/test/runner/lib/sanity.py b/test/runner/lib/sanity.py
index ee619cc16ad..473c9b0f857 100644
--- a/test/runner/lib/sanity.py
+++ b/test/runner/lib/sanity.py
@@ -38,6 +38,9 @@ from lib.executor import (
     install_command_requirements,
     SUPPORTED_PYTHON_VERSIONS,
     intercept_command,
+)
+
+from lib.config import (
     SanityConfig,
 )
 
diff --git a/test/runner/lib/test.py b/test/runner/lib/test.py
index e3b89217650..70edb703eb5 100644
--- a/test/runner/lib/test.py
+++ b/test/runner/lib/test.py
@@ -7,11 +7,10 @@ import json
 
 from lib.util import (
     display,
-    EnvironmentConfig,
 )
 
-from lib.metadata import (
-    Metadata,
+from lib.config import (
+    TestConfig,
 )
 
 
@@ -55,38 +54,6 @@ def calculate_confidence(path, line, metadata):
     return 50
 
 
-class TestConfig(EnvironmentConfig):
-    """Configuration common to all test commands."""
-    def __init__(self, args, command):
-        """
-        :type args: any
-        :type command: str
-        """
-        super(TestConfig, self).__init__(args, command)
-
-        self.coverage = args.coverage  # type: bool
-        self.coverage_label = args.coverage_label  # type: str
-        self.include = args.include  # type: list [str]
-        self.exclude = args.exclude  # type: list [str]
-        self.require = args.require  # type: list [str]
-
-        self.changed = args.changed  # type: bool
-        self.tracked = args.tracked  # type: bool
-        self.untracked = args.untracked  # type: bool
-        self.committed = args.committed  # type: bool
-        self.staged = args.staged  # type: bool
-        self.unstaged = args.unstaged  # type: bool
-        self.changed_from = args.changed_from  # type: str
-        self.changed_path = args.changed_path  # type: list [str]
-
-        self.lint = args.lint if 'lint' in args else False  # type: bool
-        self.junit = args.junit if 'junit' in args else False  # type: bool
-        self.failure_ok = args.failure_ok if 'failure_ok' in args else False  # type: bool
-
-        self.metadata = Metadata.from_file(args.metadata) if args.metadata else Metadata()
-        self.metadata_path = None
-
-
 class TestResult(object):
     """Base class for test results."""
     def __init__(self, command, test, python_version=None):
diff --git a/test/runner/lib/util.py b/test/runner/lib/util.py
index 9e7218aa5c4..4abfdd36d18 100644
--- a/test/runner/lib/util.py
+++ b/test/runner/lib/util.py
@@ -435,53 +435,6 @@ class CommonConfig(object):
         self.debug = args.debug  # type: bool
 
 
-class EnvironmentConfig(CommonConfig):
-    """Configuration common to all commands which execute in an environment."""
-    def __init__(self, args, command):
-        """
-        :type args: any
-        """
-        super(EnvironmentConfig, self).__init__(args)
-
-        self.command = command
-
-        self.local = args.local is True
-
-        if args.tox is True or args.tox is False or args.tox is None:
-            self.tox = args.tox is True
-            self.tox_args = 0
-            self.python = args.python if 'python' in args else None  # type: str
-        else:
-            self.tox = True
-            self.tox_args = 1
-            self.python = args.tox  # type: str
-
-        self.docker = docker_qualify_image(args.docker)  # type: str
-        self.remote = args.remote  # type: str
-
-        self.docker_privileged = args.docker_privileged if 'docker_privileged' in args else False  # type: bool
-        self.docker_util = docker_qualify_image(args.docker_util if 'docker_util' in args else '')  # type: str
-        self.docker_pull = args.docker_pull if 'docker_pull' in args else False  # type: bool
-
-        self.tox_sitepackages = args.tox_sitepackages  # type: bool
-
-        self.remote_stage = args.remote_stage  # type: str
-        self.remote_aws_region = args.remote_aws_region  # type: str
-        self.remote_terminate = args.remote_terminate  # type: str
-
-        self.requirements = args.requirements  # type: bool
-
-        if self.python == 'default':
-            self.python = '.'.join(str(i) for i in sys.version_info[:2])
-
-        self.python_version = self.python or '.'.join(str(i) for i in sys.version_info[:2])
-
-        self.delegate = self.tox or self.docker or self.remote
-
-        if self.delegate:
-            self.requirements = True
-
-
 def docker_qualify_image(name):
     """
     :type name: str
diff --git a/test/runner/test.py b/test/runner/test.py
index e3cec222e96..f3a286810fa 100755
--- a/test/runner/test.py
+++ b/test/runner/test.py
@@ -27,6 +27,13 @@ from lib.executor import (
     command_shell,
     SUPPORTED_PYTHON_VERSIONS,
     COMPILE_PYTHON_VERSIONS,
+    ApplicationWarning,
+    Delegate,
+    generate_pip_install,
+    check_startup,
+)
+
+from lib.config import (
     PosixIntegrationConfig,
     WindowsIntegrationConfig,
     NetworkIntegrationConfig,
@@ -34,10 +41,6 @@ from lib.executor import (
     UnitsConfig,
     CompileConfig,
     ShellConfig,
-    ApplicationWarning,
-    Delegate,
-    generate_pip_install,
-    check_startup,
 )
 
 from lib.sanity import (