From d3ae744049a040b3aa8567c57318a24ba5503f7d Mon Sep 17 00:00:00 2001 From: Aaron Steers Date: Fri, 2 May 2025 14:35:21 -0700 Subject: [PATCH 1/9] handle case of declared category with no 'tests' (usually 'bypass_reason' exists) --- airbyte_cdk/test/standard_tests/connector_base.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/connector_base.py b/airbyte_cdk/test/standard_tests/connector_base.py index 35bcdbe8f..bfe4f1440 100644 --- a/airbyte_cdk/test/standard_tests/connector_base.py +++ b/airbyte_cdk/test/standard_tests/connector_base.py @@ -154,10 +154,12 @@ def get_scenarios( f"Acceptance tests config not found in {cls.acceptance_test_config_path}." f" Found only: {str(all_tests_config)}." ) - if category not in all_tests_config["acceptance_tests"]: + + if ( + category not in all_tests_config["acceptance_tests"] + or "tests" not in all_tests_config["acceptance_tests"][category] + ): return [] - if "tests" not in all_tests_config["acceptance_tests"][category]: - raise ValueError(f"No tests found for category {category}") tests_scenarios = [ ConnectorTestScenario.model_validate(test) From cdbe49c576f4cbe8abe5e8ba407222743f0cc4ca Mon Sep 17 00:00:00 2001 From: Aaron Steers Date: Tue, 6 May 2025 16:40:21 -0700 Subject: [PATCH 2/9] add support for 'spec' and tests without a scenario --- airbyte_cdk/test/entrypoint_wrapper.py | 4 +++ .../test/standard_tests/_job_runner.py | 24 +++++++++----- .../test/standard_tests/connector_base.py | 33 ++++++++++--------- .../standard_tests/declarative_sources.py | 10 ++++-- .../test/standard_tests/source_base.py | 20 +++++++++++ 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/airbyte_cdk/test/entrypoint_wrapper.py b/airbyte_cdk/test/entrypoint_wrapper.py index 43c84204a..79c328203 100644 --- a/airbyte_cdk/test/entrypoint_wrapper.py +++ b/airbyte_cdk/test/entrypoint_wrapper.py @@ -82,6 +82,10 @@ def records(self) -> List[AirbyteMessage]: def state_messages(self) -> List[AirbyteMessage]: return self._get_message_by_types([Type.STATE]) + @property + def spec_messages(self) -> List[AirbyteMessage]: + return self._get_message_by_types([Type.SPEC]) + @property def connection_status_messages(self) -> List[AirbyteMessage]: return self._get_message_by_types([Type.CONNECTION_STATUS]) diff --git a/airbyte_cdk/test/standard_tests/_job_runner.py b/airbyte_cdk/test/standard_tests/_job_runner.py index bab170361..7fc93e415 100644 --- a/airbyte_cdk/test/standard_tests/_job_runner.py +++ b/airbyte_cdk/test/standard_tests/_job_runner.py @@ -56,9 +56,9 @@ def spec(self, logger: logging.Logger) -> Any: def run_test_job( connector: IConnector | type[IConnector] | Callable[[], IConnector], - verb: Literal["read", "check", "discover"], - test_scenario: ConnectorTestScenario, + verb: Literal["spec", "read", "check", "discover"], *, + test_scenario: ConnectorTestScenario | None = None, catalog: ConfiguredAirbyteCatalog | dict[str, Any] | None = None, ) -> entrypoint_wrapper.EntrypointOutput: """Run a test scenario from provided CLI args and return the result.""" @@ -81,9 +81,9 @@ def run_test_job( ) args: list[str] = [verb] - if test_scenario.config_path: + if test_scenario and test_scenario.config_path: args += ["--config", str(test_scenario.config_path)] - elif test_scenario.config_dict: + elif test_scenario and test_scenario.config_dict: config_path = ( Path(tempfile.gettempdir()) / "airbyte-test" / f"temp_config_{uuid.uuid4().hex}.json" ) @@ -103,7 +103,7 @@ def run_test_job( ) catalog_path.parent.mkdir(parents=True, exist_ok=True) catalog_path.write_text(orjson.dumps(catalog).decode()) - elif test_scenario.configured_catalog_path: + elif test_scenario and test_scenario.configured_catalog_path: catalog_path = Path(test_scenario.configured_catalog_path) if catalog_path: @@ -112,12 +112,18 @@ def run_test_job( # This is a bit of a hack because the source needs the catalog early. # Because it *also* can fail, we have to redundantly wrap it in a try/except block. + expect_exception = False + if test_scenario and test_scenario.expect_exception: + # If the test scenario expects an exception, we need to set the + # `expect_exception` flag to True. + expect_exception = True + 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_scenario.expect_exception, + expecting_exception=False if not test_scenario else expect_exception, ) - if result.errors and not test_scenario.expect_exception: + if result.errors and not expect_exception: raise AssertionError( f"Expected no errors but got {len(result.errors)}: \n" + _errors_to_str(result) ) @@ -132,7 +138,7 @@ def run_test_job( + "\n".join([str(msg) for msg in result.connection_status_messages]) + _errors_to_str(result) ) - if test_scenario.expect_exception: + if expect_exception: conn_status = result.connection_status_messages[0].connectionStatus assert conn_status, ( "Expected CONNECTION_STATUS message to be present. Got: \n" @@ -146,7 +152,7 @@ def run_test_job( return result # For all other verbs, we assert check that an exception is raised (or not). - if test_scenario.expect_exception: + if expect_exception: if not result.errors: raise AssertionError("Expected exception but got none.") diff --git a/airbyte_cdk/test/standard_tests/connector_base.py b/airbyte_cdk/test/standard_tests/connector_base.py index bfe4f1440..4355d22fc 100644 --- a/airbyte_cdk/test/standard_tests/connector_base.py +++ b/airbyte_cdk/test/standard_tests/connector_base.py @@ -89,7 +89,7 @@ def get_test_class_dir(cls) -> Path: @classmethod def create_connector( cls, - scenario: ConnectorTestScenario, + scenario: ConnectorTestScenario | None, ) -> IConnector: """Instantiate the connector class.""" connector = cls.connector # type: ignore @@ -147,7 +147,7 @@ def get_scenarios( This has to be a separate function because pytest does not allow parametrization of fixtures with arguments from the test class itself. """ - category = "connection" + categories = ["connection", "spec"] all_tests_config = yaml.safe_load(cls.acceptance_test_config_path.read_text()) if "acceptance_tests" not in all_tests_config: raise ValueError( @@ -155,22 +155,25 @@ def get_scenarios( f" Found only: {str(all_tests_config)}." ) - if ( - category not in all_tests_config["acceptance_tests"] - or "tests" not in all_tests_config["acceptance_tests"][category] - ): - return [] - - tests_scenarios = [ - ConnectorTestScenario.model_validate(test) - for test in all_tests_config["acceptance_tests"][category]["tests"] - if "iam_role" not in test["config_path"] - ] + test_scenarios: list[ConnectorTestScenario] = [] + for category in categories: + if ( + category not in all_tests_config["acceptance_tests"] + or "tests" not in all_tests_config["acceptance_tests"][category] + ): + continue + + test_scenarios.extend([ + ConnectorTestScenario.model_validate(test) + for test in all_tests_config["acceptance_tests"][category]["tests"] + if "config_path" in test and "iam_role" not in test["config_path"] + ]) + connector_root = cls.get_connector_root_dir().absolute() - for test in tests_scenarios: + for test in test_scenarios: if test.config_path: test.config_path = connector_root / test.config_path if test.configured_catalog_path: test.configured_catalog_path = connector_root / test.configured_catalog_path - return tests_scenarios + return test_scenarios diff --git a/airbyte_cdk/test/standard_tests/declarative_sources.py b/airbyte_cdk/test/standard_tests/declarative_sources.py index e1954246f..70b814d79 100644 --- a/airbyte_cdk/test/standard_tests/declarative_sources.py +++ b/airbyte_cdk/test/standard_tests/declarative_sources.py @@ -64,7 +64,7 @@ def components_py_path(cls) -> Path | None: @classmethod def create_connector( cls, - scenario: ConnectorTestScenario, + scenario: ConnectorTestScenario | None, ) -> IConnector: """Create a connector scenario for the test suite. @@ -73,9 +73,13 @@ def create_connector( Subclasses should not need to override this method. """ - config: dict[str, Any] = scenario.get_config_dict() - manifest_dict = yaml.safe_load(cls.manifest_yaml_path.read_text()) + config = { + "__injected_manifest": manifest_dict, + } + if scenario: + config.update(scenario.get_config_dict()) + if cls.components_py_path and cls.components_py_path.exists(): os.environ["AIRBYTE_ENABLE_UNSAFE_CODE"] = "true" config["__injected_components_py"] = cls.components_py_path.read_text() diff --git a/airbyte_cdk/test/standard_tests/source_base.py b/airbyte_cdk/test/standard_tests/source_base.py index 83cc7326f..9c16002fa 100644 --- a/airbyte_cdk/test/standard_tests/source_base.py +++ b/airbyte_cdk/test/standard_tests/source_base.py @@ -64,6 +64,26 @@ def test_discover( test_scenario=scenario, ) + def test_spec( + self, + # scenario: ConnectorTestScenario, # No inputs needed for spec test + ) -> None: + """Standard test for `spec`.""" + result = run_test_job( + verb="spec", + test_scenario=None, + connector=self.create_connector(scenario=None), + ) + if result.errors: + raise AssertionError( + f"Expected no errors but got {len(result.errors)}: \n" + + "\n".join([str(e) for e in result.errors]) + ) + assert len(result.spec_messages) == 1, ( + "Expected exactly 1 spec message but got {len(result.spec_messages)}", + result.errors, + ) + def test_basic_read( self, scenario: ConnectorTestScenario, From c663ee01ab4f4c966e4f6ef78a671f024c6c2684 Mon Sep 17 00:00:00 2001 From: octavia-squidington-iii Date: Tue, 6 May 2025 23:52:39 +0000 Subject: [PATCH 3/9] Auto-fix lint and format issues --- airbyte_cdk/test/standard_tests/connector_base.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/connector_base.py b/airbyte_cdk/test/standard_tests/connector_base.py index 4355d22fc..7d4c35132 100644 --- a/airbyte_cdk/test/standard_tests/connector_base.py +++ b/airbyte_cdk/test/standard_tests/connector_base.py @@ -163,11 +163,13 @@ def get_scenarios( ): continue - test_scenarios.extend([ - ConnectorTestScenario.model_validate(test) - for test in all_tests_config["acceptance_tests"][category]["tests"] - if "config_path" in test and "iam_role" not in test["config_path"] - ]) + test_scenarios.extend( + [ + ConnectorTestScenario.model_validate(test) + for test in all_tests_config["acceptance_tests"][category]["tests"] + if "config_path" in test and "iam_role" not in test["config_path"] + ] + ) connector_root = cls.get_connector_root_dir().absolute() for test in test_scenarios: From a257eb2c7308426f899d2e2bc26822deb9f2f326 Mon Sep 17 00:00:00 2001 From: "Aaron (\"AJ\") Steers" Date: Tue, 6 May 2025 16:57:06 -0700 Subject: [PATCH 4/9] Update airbyte_cdk/test/standard_tests/_job_runner.py --- airbyte_cdk/test/standard_tests/_job_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte_cdk/test/standard_tests/_job_runner.py b/airbyte_cdk/test/standard_tests/_job_runner.py index 7fc93e415..2d0b15244 100644 --- a/airbyte_cdk/test/standard_tests/_job_runner.py +++ b/airbyte_cdk/test/standard_tests/_job_runner.py @@ -121,7 +121,7 @@ def run_test_job( result: entrypoint_wrapper.EntrypointOutput = entrypoint_wrapper._run_command( # noqa: SLF001 # Non-public API source=connector_obj, # type: ignore [arg-type] args=args, - expecting_exception=False if not test_scenario else expect_exception, + expecting_exception=expect_exception, ) if result.errors and not expect_exception: raise AssertionError( From 2d308e83323a8d3017136f3c0ede6292db6cb63c Mon Sep 17 00:00:00 2001 From: "Aaron (\"AJ\") Steers" Date: Tue, 6 May 2025 17:01:04 -0700 Subject: [PATCH 5/9] Update airbyte_cdk/test/standard_tests/source_base.py --- airbyte_cdk/test/standard_tests/source_base.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/source_base.py b/airbyte_cdk/test/standard_tests/source_base.py index 9c16002fa..f10853dc2 100644 --- a/airbyte_cdk/test/standard_tests/source_base.py +++ b/airbyte_cdk/test/standard_tests/source_base.py @@ -64,11 +64,18 @@ def test_discover( test_scenario=scenario, ) - def test_spec( - self, - # scenario: ConnectorTestScenario, # No inputs needed for spec test - ) -> None: - """Standard test for `spec`.""" + def test_spec(self) -> None: + """Standard test for `spec`. + + This test does not require a `scenario` input, since `spec` + does not require any inputs. + + We assume `spec` should always succeed and it should always generate + a valid `SPEC` message. + + Note: the parsing of messages by type also implicitly validates that + the generated `SPEC` message is valid JSON. + """ result = run_test_job( verb="spec", test_scenario=None, From b883a556f5fe3a85a4fec67e34fd2ad10cc36031 Mon Sep 17 00:00:00 2001 From: octavia-squidington-iii Date: Wed, 7 May 2025 00:03:43 +0000 Subject: [PATCH 6/9] Auto-fix lint and format issues --- airbyte_cdk/test/standard_tests/source_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/source_base.py b/airbyte_cdk/test/standard_tests/source_base.py index f10853dc2..05dfd69ca 100644 --- a/airbyte_cdk/test/standard_tests/source_base.py +++ b/airbyte_cdk/test/standard_tests/source_base.py @@ -69,8 +69,8 @@ def test_spec(self) -> None: This test does not require a `scenario` input, since `spec` does not require any inputs. - - We assume `spec` should always succeed and it should always generate + + We assume `spec` should always succeed and it should always generate a valid `SPEC` message. Note: the parsing of messages by type also implicitly validates that From 345b3d2b6cf4df5b9d47a86f77f144c61337f7d2 Mon Sep 17 00:00:00 2001 From: Aaron Steers Date: Tue, 6 May 2025 17:20:02 -0700 Subject: [PATCH 7/9] add handling for empty config within a scenario --- .../test/standard_tests/_job_runner.py | 27 +++++++++---------- .../standard_tests/declarative_sources.py | 4 +-- .../test/standard_tests/models/scenario.py | 18 ++++++++++--- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/_job_runner.py b/airbyte_cdk/test/standard_tests/_job_runner.py index 2d0b15244..ad8316d78 100644 --- a/airbyte_cdk/test/standard_tests/_job_runner.py +++ b/airbyte_cdk/test/standard_tests/_job_runner.py @@ -62,6 +62,9 @@ def run_test_job( catalog: ConfiguredAirbyteCatalog | dict[str, Any] | None = None, ) -> entrypoint_wrapper.EntrypointOutput: """Run a test scenario from provided CLI args and return the result.""" + # Use default (empty) scenario if not provided: + test_scenario = test_scenario or ConnectorTestScenario() + if not connector: raise ValueError("Connector is required") @@ -81,14 +84,14 @@ def run_test_job( ) args: list[str] = [verb] - if test_scenario and test_scenario.config_path: - args += ["--config", str(test_scenario.config_path)] - elif test_scenario and test_scenario.config_dict: + config_dict = test_scenario.get_config_dict(empty_if_missing=True) + if config_dict and verb != "spec": + # Write the config to a temp json file and pass the path to the file as an argument. 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_scenario.config_dict).decode()) + config_path.write_text(orjson.dumps(config_dict).decode()) args += ["--config", str(config_path)] catalog_path: Path | None = None @@ -103,7 +106,7 @@ def run_test_job( ) catalog_path.parent.mkdir(parents=True, exist_ok=True) catalog_path.write_text(orjson.dumps(catalog).decode()) - elif test_scenario and test_scenario.configured_catalog_path: + elif test_scenario.configured_catalog_path: catalog_path = Path(test_scenario.configured_catalog_path) if catalog_path: @@ -112,18 +115,12 @@ def run_test_job( # This is a bit of a hack because the source needs the catalog early. # Because it *also* can fail, we have to redundantly wrap it in a try/except block. - expect_exception = False - if test_scenario and test_scenario.expect_exception: - # If the test scenario expects an exception, we need to set the - # `expect_exception` flag to True. - expect_exception = True - result: entrypoint_wrapper.EntrypointOutput = entrypoint_wrapper._run_command( # noqa: SLF001 # Non-public API source=connector_obj, # type: ignore [arg-type] args=args, - expecting_exception=expect_exception, + expecting_exception=test_scenario.expect_exception, ) - if result.errors and not expect_exception: + if result.errors and not test_scenario.expect_exception: raise AssertionError( f"Expected no errors but got {len(result.errors)}: \n" + _errors_to_str(result) ) @@ -138,7 +135,7 @@ def run_test_job( + "\n".join([str(msg) for msg in result.connection_status_messages]) + _errors_to_str(result) ) - if 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" @@ -152,7 +149,7 @@ def run_test_job( return result # For all other verbs, we assert check that an exception is raised (or not). - if expect_exception: + if test_scenario.expect_exception: if not result.errors: raise AssertionError("Expected exception but got none.") diff --git a/airbyte_cdk/test/standard_tests/declarative_sources.py b/airbyte_cdk/test/standard_tests/declarative_sources.py index 70b814d79..a105e26a4 100644 --- a/airbyte_cdk/test/standard_tests/declarative_sources.py +++ b/airbyte_cdk/test/standard_tests/declarative_sources.py @@ -73,12 +73,12 @@ def create_connector( Subclasses should not need to override this method. """ + scenario = scenario or ConnectorTestScenario() # Use default (empty) scenario if None manifest_dict = yaml.safe_load(cls.manifest_yaml_path.read_text()) config = { "__injected_manifest": manifest_dict, } - if scenario: - config.update(scenario.get_config_dict()) + config.update(scenario.get_config_dict(empty_if_missing=True)) if cls.components_py_path and cls.components_py_path.exists(): os.environ["AIRBYTE_ENABLE_UNSAFE_CODE"] = "true" diff --git a/airbyte_cdk/test/standard_tests/models/scenario.py b/airbyte_cdk/test/standard_tests/models/scenario.py index 944b60921..2aa8b027e 100644 --- a/airbyte_cdk/test/standard_tests/models/scenario.py +++ b/airbyte_cdk/test/standard_tests/models/scenario.py @@ -13,6 +13,7 @@ from typing import Any, Literal, cast import yaml +from numpy import empty from pydantic import BaseModel @@ -43,18 +44,29 @@ class AcceptanceTestFileTypes(BaseModel): file_types: AcceptanceTestFileTypes | None = None status: Literal["succeed", "failed"] | None = None - def get_config_dict(self) -> dict[str, Any]: + def get_config_dict( + self, + *, + empty_if_missing: bool, + ) -> dict[str, Any]: """Return the config dictionary. If a config dictionary has already been loaded, return it. Otherwise, load the config file and return the dictionary. + + If `self.config_dict` and `self.config_path` are both `None`: + - return an empty dictionary if `empty_if_missing` is True + - raise a ValueError if `empty_if_missing` is False """ - if self.config_dict: + if self.config_dict is not None: return self.config_dict - if self.config_path: + if self.config_path is not None: return cast(dict[str, Any], yaml.safe_load(self.config_path.read_text())) + if empty_if_missing: + return {} + raise ValueError("No config dictionary or path provided.") @property From 72e73ecc3e04aae6513c47ef5f6f56675266b803 Mon Sep 17 00:00:00 2001 From: Aaron Steers Date: Tue, 6 May 2025 17:32:03 -0700 Subject: [PATCH 8/9] remove redundant errors check --- airbyte_cdk/test/standard_tests/source_base.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/source_base.py b/airbyte_cdk/test/standard_tests/source_base.py index 05dfd69ca..a256fa04c 100644 --- a/airbyte_cdk/test/standard_tests/source_base.py +++ b/airbyte_cdk/test/standard_tests/source_base.py @@ -81,11 +81,8 @@ def test_spec(self) -> None: test_scenario=None, connector=self.create_connector(scenario=None), ) - if result.errors: - raise AssertionError( - f"Expected no errors but got {len(result.errors)}: \n" - + "\n".join([str(e) for e in result.errors]) - ) + # If an error occurs, it will be raised above. + assert len(result.spec_messages) == 1, ( "Expected exactly 1 spec message but got {len(result.spec_messages)}", result.errors, From e168342bbf221a9bb163d626a713276f40f21d92 Mon Sep 17 00:00:00 2001 From: "Aaron (\"AJ\") Steers" Date: Wed, 7 May 2025 12:05:11 -0700 Subject: [PATCH 9/9] Apply suggestions from code review --- airbyte_cdk/test/standard_tests/models/scenario.py | 1 - 1 file changed, 1 deletion(-) diff --git a/airbyte_cdk/test/standard_tests/models/scenario.py b/airbyte_cdk/test/standard_tests/models/scenario.py index 2aa8b027e..0ace85d33 100644 --- a/airbyte_cdk/test/standard_tests/models/scenario.py +++ b/airbyte_cdk/test/standard_tests/models/scenario.py @@ -13,7 +13,6 @@ from typing import Any, Literal, cast import yaml -from numpy import empty from pydantic import BaseModel