clean up naming, remove unused

Author: aaronsteers

airbyte_cdk/test/declarative/models/scenario.py

@@ -17,9 +17,9 @@
 
 
 class ConnectorTestScenario(BaseModel):
-    """Acceptance test instance, as a Pydantic model.
+    """Acceptance test scenario, as a Pydantic model.
 
-    This class represents an acceptance test instance, which is a single test case
+    This class represents an acceptance test scenario, which is a single test case
     that can be run against a connector. It is used to deserialize and validate the
     acceptance test configuration file.
     """

airbyte_cdk/test/declarative/test_suites/connector_base.py

@@ -29,27 +29,12 @@
 MANIFEST_YAML = "manifest.yaml"
 
 
-class JavaClass(str):
-    """A string that represents a Java class."""
-
-
-class DockerImage(str):
-    """A string that represents a Docker image."""
-
-
-class RunnableConnector(abc.ABC):
-    """A connector that can be run in a test scenario."""
-
-    @abc.abstractmethod
-    def launch(cls, args: list[str] | None) -> None: ...
-
-
 def generate_tests(metafunc: pytest.Metafunc) -> None:
     """
     A helper for pytest_generate_tests hook.
 
     If a test method (in a class subclassed from our base class)
-    declares an argument 'instance', this function retrieves the
+    declares an argument 'scenario', this function retrieves the
     'scenarios' attribute from the test class and parametrizes that
     test with the values from 'scenarios'.
 
@@ -69,8 +54,8 @@ class TestMyConnector(ConnectorTestSuiteBase):
 
     ```
     """
-    # Check if the test function requires an 'instance' argument
-    if "instance" in metafunc.fixturenames:
+    # Check if the test function requires an 'scenario' argument
+    if "scenario" in metafunc.fixturenames:
         # Retrieve the test class
         test_class = metafunc.cls
         if test_class is None:
@@ -85,14 +70,14 @@ class TestMyConnector(ConnectorTestSuiteBase):
 
         scenarios = test_class.get_scenarios()
         ids = [str(scenario) for scenario in scenarios]
-        metafunc.parametrize("instance", scenarios, ids=ids)
+        metafunc.parametrize("scenario", scenarios, ids=ids)
 
 
 class ConnectorTestSuiteBase(abc.ABC):
     """Base class for connector test suites."""
 
     connector: type[IConnector] | Callable[[], IConnector] | None = None
-    """The connector class or a factory function that returns an instance of IConnector."""
+    """The connector class or a factory function that returns an scenario of IConnector."""
 
     @classmethod
     def get_test_class_dir(cls) -> Path:
@@ -120,31 +105,17 @@ def create_connector(
             "override `cls.create_connector()` to define a custom initialization process."
         )
 
-    def run_test_scenario(
-        self,
-        verb: Literal["read", "check", "discover"],
-        test_scenario: ConnectorTestScenario,
-        *,
-        catalog: dict[str, Any] | None = None,
-    ) -> entrypoint_wrapper.EntrypointOutput:
-        """Run a test job from provided CLI args and return the result."""
-        return run_test_job(
-            self.create_connector(test_scenario),
-            verb,
-            test_instance=test_scenario,
-            catalog=catalog,
-        )
-
     # Test Definitions
 
     def test_check(
         self,
-        instance: ConnectorTestScenario,
+        scenario: ConnectorTestScenario,
     ) -> None:
         """Run `connection` acceptance tests."""
-        result = self.run_test_scenario(
+        result: entrypoint_wrapper.EntrypointOutput = run_test_job(
+            self.create_connector(scenario),
             "check",
-            test_scenario=instance,
+            test_scenario=scenario,
         )
         conn_status_messages: list[AirbyteMessage] = [
             msg for msg in result._messages if msg.type == Type.CONNECTION_STATUS

airbyte_cdk/test/declarative/test_suites/declarative_sources.py

@@ -18,11 +18,25 @@
 
 
 def md5_checksum(file_path: Path) -> str:
+    """Helper function to calculate the MD5 checksum of a file.
+
+    This is used to calculate the checksum of the `components.py` file, if it exists.
+    """
     with open(file_path, "rb") as file:
         return md5(file.read()).hexdigest()
 
 
 class DeclarativeSourceTestSuite(SourceTestSuiteBase):
+    """Declarative source test suite.
+
+    This inherits from the Python-based source test suite and implements the
+    `create_connector` method to create a declarative source object instead of
+    requiring a custom Python source object.
+
+    The class also automatically locates the `manifest.yaml` file and the
+    `components.py` file (if it exists) in the connector's directory.
+    """
+
     @classproperty
     def manifest_yaml_path(cls) -> Path:
         """Get the path to the manifest.yaml file."""
@@ -37,7 +51,10 @@ def manifest_yaml_path(cls) -> Path:
 
     @classproperty
     def components_py_path(cls) -> Path | None:
-        """Get the path to the components.py file."""
+        """Get the path to the `components.py` file, if one exists.
+
+        If not `components.py` file exists, return None.
+        """
         result = cls.get_connector_root_dir() / "components.py"
         if result.exists():
             return result
@@ -49,11 +66,14 @@ def create_connector(
         cls,
         scenario: ConnectorTestScenario,
     ) -> IConnector:
-        """Create a connector instance for the test suite."""
+        """Create a connector scenario for the test suite.
+
+        This overrides `create_connector` from the create a declarative source object
+        instead of requiring a custom python source object.
+
+        Subclasses should not need to override this method.
+        """
         config: dict[str, Any] = scenario.get_config_dict()
-        # catalog = scenario.get_catalog()
-        # state = scenario.get_state()
-        # source_config = scenario.get_source_config()
 
         manifest_dict = yaml.safe_load(cls.manifest_yaml_path.read_text())
         if cls.components_py_path and cls.components_py_path.exists():

airbyte_cdk/test/declarative/test_suites/destination_base.py

@@ -9,4 +9,8 @@ class DestinationTestSuiteBase(ConnectorTestSuiteBase):
 
     This class provides a base set of functionality for testing destination connectors, and it
     inherits all generic connector tests from the `ConnectorTestSuiteBase` class.
+
+    TODO: As of now, this class does not add any additional functionality or tests specific to
+    destination connectors. However, it serves as a placeholder for future enhancements and
+    customizations that may be needed for destination connectors.
     """

airbyte_cdk/test/declarative/test_suites/source_base.py

@@ -31,13 +31,18 @@ class SourceTestSuiteBase(ConnectorTestSuiteBase):
 
     def test_check(
         self,
-        instance: ConnectorTestScenario,
+        scenario: ConnectorTestScenario,
     ) -> None:
-        """Run `connection` acceptance tests."""
+        """Run standard `check` tests on the connector.
+
+        Assert that the connector returns a single CONNECTION_STATUS message.
+        This test is designed to validate the connector's ability to establish a connection
+        and return its status with the expected message type.
+        """
         result: entrypoint_wrapper.EntrypointOutput = run_test_job(
-            self.create_connector(instance),
+            self.create_connector(scenario),
             "check",
-            test_instance=instance,
+            test_scenario=scenario,
         )
         conn_status_messages: list[AirbyteMessage] = [
             msg for msg in result._messages if msg.type == Type.CONNECTION_STATUS
@@ -50,15 +55,21 @@ def test_check(
 
     def test_basic_read(
         self,
-        instance: ConnectorTestScenario,
+        scenario: ConnectorTestScenario,
     ) -> None:
-        """Run acceptance tests."""
+        """Run standard `read` test on the connector.
+
+        This test is designed to validate the connector's ability to read data
+        from the source and return records. It first runs a `discover` job to
+        obtain the catalog of streams, and then it runs a `read` job to fetch
+        records from those streams.
+        """
         discover_result = run_test_job(
-            self.create_connector(instance),
+            self.create_connector(scenario),
             "discover",
-            test_instance=instance,
+            test_scenario=scenario,
         )
-        if instance.expect_exception:
+        if scenario.expect_exception:
             assert discover_result.errors, "Expected exception but got none."
             return
 
@@ -73,20 +84,20 @@ def test_basic_read(
             ]
         )
         result = run_test_job(
-            self.create_connector(instance),
+            self.create_connector(scenario),
             "read",
-            test_instance=instance,
+            test_scenario=scenario,
             catalog=configured_catalog,
         )
 
         if not result.records:
             raise AssertionError("Expected records but got none.")  # noqa: TRY003
 
-    def test_fail_with_bad_catalog(
+    def test_fail_read_with_bad_catalog(
         self,
-        instance: ConnectorTestScenario,
+        scenario: ConnectorTestScenario,
     ) -> None:
-        """Test that a bad catalog fails."""
+        """Standard test for `read` when passed a bad catalog file."""
         invalid_configured_catalog = ConfiguredAirbyteCatalog(
             streams=[
                 # Create ConfiguredAirbyteStream which is deliberately invalid
@@ -107,22 +118,23 @@ def test_fail_with_bad_catalog(
             ]
         )
         # Set expected status to "failed" to ensure the test fails if the connector.
-        instance.status = "failed"
-        result = self.run_test_scenario(
+        scenario.status = "failed"
+        result: entrypoint_wrapper.EntrypointOutput = run_test_job(
+            self.create_connector(scenario),
             "read",
-            test_scenario=instance,
+            test_scenario=scenario,
             catalog=asdict(invalid_configured_catalog),
         )
         assert result.errors, "Expected errors but got none."
         assert result.trace_messages, "Expected trace messages but got none."
 
     def test_discover(
         self,
-        instance: ConnectorTestScenario,
+        scenario: ConnectorTestScenario,
     ) -> None:
-        """Run acceptance tests."""
+        """Standard test for `discover`."""
         run_test_job(
-            self.create_connector(instance),
-            "check",
-            test_instance=instance,
+            self.create_connector(scenario),
+            "discover",
+            test_scenario=scenario,
         )

airbyte_cdk/test/declarative/utils/job_runner.py

@@ -29,11 +29,11 @@ def launch(self, args: list[str] | None) -> None:
 def run_test_job(
     connector: IConnector | type[IConnector] | Callable[[], IConnector],
     verb: Literal["read", "check", "discover"],
-    test_instance: ConnectorTestScenario,
+    test_scenario: ConnectorTestScenario,
     *,
     catalog: ConfiguredAirbyteCatalog | dict[str, Any] | None = None,
 ) -> entrypoint_wrapper.EntrypointOutput:
-    """Run a test job from provided CLI args and return the result."""
+    """Run a test scenario from provided CLI args and return the result."""
     if not connector:
         raise ValueError("Connector is required")
 
@@ -59,14 +59,14 @@ def run_test_job(
         )
 
     args: list[str] = [verb]
-    if test_instance.config_path:
-        args += ["--config", str(test_instance.config_path)]
-    elif test_instance.config_dict:
+    if test_scenario.config_path:
+        args += ["--config", str(test_scenario.config_path)]
+    elif test_scenario.config_dict:
         config_path = (
             Path(tempfile.gettempdir()) / "airbyte-test" / f"temp_config_{uuid.uuid4().hex}.json"
         )
         config_path.parent.mkdir(parents=True, exist_ok=True)
-        config_path.write_text(orjson.dumps(test_instance.config_dict).decode())
+        config_path.write_text(orjson.dumps(test_scenario.config_dict).decode())
         args += ["--config", str(config_path)]
 
     catalog_path: Path | None = None
@@ -81,21 +81,21 @@ def run_test_job(
             )
             catalog_path.parent.mkdir(parents=True, exist_ok=True)
             catalog_path.write_text(orjson.dumps(catalog).decode())
-        elif test_instance.configured_catalog_path:
-            catalog_path = Path(test_instance.configured_catalog_path)
+        elif test_scenario.configured_catalog_path:
+            catalog_path = Path(test_scenario.configured_catalog_path)
 
         if catalog_path:
             args += ["--catalog", str(catalog_path)]
 
     # This is a bit of a hack because the source needs the catalog early.
-    # Because it *also* can fail, we have ot redundantly wrap it in a try/except block.
+    # Because it *also* can fail, we have to redundantly wrap it in a try/except block.
 
     result: entrypoint_wrapper.EntrypointOutput = entrypoint_wrapper._run_command(  # noqa: SLF001  # Non-public API
         source=connector_obj,  # type: ignore [arg-type]
         args=args,
-        expecting_exception=test_instance.expect_exception,
+        expecting_exception=test_scenario.expect_exception,
     )
-    if result.errors and not test_instance.expect_exception:
+    if result.errors and not test_scenario.expect_exception:
         raise AssertionError(
             "\n\n".join(
                 [str(err.trace.error).replace("\\n", "\n") for err in result.errors if err.trace],
@@ -114,7 +114,7 @@ def run_test_job(
             f"{len(result.connection_status_messages)}:\n"
             + "\n".join([str(msg) for msg in result.connection_status_messages])
         )
-        if test_instance.expect_exception:
+        if test_scenario.expect_exception:
             conn_status = result.connection_status_messages[0].connectionStatus
             assert conn_status, (
                 "Expected CONNECTION_STATUS message to be present. Got: \n"
@@ -128,7 +128,7 @@ def run_test_job(
         return result
 
     # For all other verbs, we assert check that an exception is raised (or not).
-    if test_instance.expect_exception:
+    if test_scenario.expect_exception:
         if not result.errors:
             raise AssertionError("Expected exception but got none.")