Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: connector-builder-test-read CLI with custom components support (do not merge) #329

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions airbyte_cdk/cli/connector_builder_test_read/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright (c) 2025 Airbyte, Inc., all rights reserved.
"""CLI for running test reads from the Airbyte Connector Builder.

This CLI accepts a config file and an action to perform on the connector.

Usage:
connector-builder-test-read [--config CONFIG] [--action ACTION]

Options:
--action ACTION The action to perform (e.g., test, validate).
--config CONFIG Path to the config file.
"""
from airbyte_cdk.cli.connector_builder_test_read.run import run

__all__ = [
"run",
]
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
from airbyte_cdk.models import Type as MessageType
from airbyte_cdk.sources.declarative.declarative_source import DeclarativeSource
from airbyte_cdk.sources.declarative.manifest_declarative_source import ManifestDeclarativeSource
from airbyte_cdk.sources.declarative.parsers.custom_code_compiler import (
INJECTED_MANIFEST,
)
from airbyte_cdk.sources.declarative.parsers.model_to_component_factory import (
ModelToComponentFactory,
)
Expand Down Expand Up @@ -50,7 +53,7 @@ def get_limits(config: Mapping[str, Any]) -> TestReadLimits:


def create_source(config: Mapping[str, Any], limits: TestReadLimits) -> ManifestDeclarativeSource:
manifest = config["__injected_declarative_manifest"]
manifest = config[INJECTED_MANIFEST]
return ManifestDeclarativeSource(
config=config,
emit_connector_builder_messages=True,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,17 @@ def handle_request(args: List[str]) -> str:
).decode() # type: ignore[no-any-return] # Serializer.dump() always returns AirbyteMessage


if __name__ == "__main__":
def run(args: List[str] | None) -> None:
args = args or sys.argv[1:]
try:
print(handle_request(sys.argv[1:]))
print(handle_request(args))
except Exception as exc:
error = AirbyteTracedException.from_exception(
exc, message=f"Error handling request: {str(exc)}"
)
m = error.as_airbyte_message()
print(orjson.dumps(AirbyteMessageSerializer.dump(m)).decode())


if __name__ == "__main__":
run()
Comment on lines +111 to +112
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix function call to match signature

The run() function is called without the required args parameter. While the function has a default value, it's better to be explicit.

Would this be clearer?

if __name__ == "__main__":
-    run()
+    run(None)  # Explicitly pass None to use default sys.argv[1:]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if __name__ == "__main__":
run()
if __name__ == "__main__":
run(None) # Explicitly pass None to use default sys.argv[1:]
🧰 Tools
🪛 GitHub Actions: Linters

[error] 112-112: Missing positional argument 'args' in call to 'run'

3 changes: 0 additions & 3 deletions airbyte_cdk/connector_builder/__init__.py

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@


class ManifestDeclarativeSource(DeclarativeSource):
"""Declarative source defined by a manifest of low-code components that define source connector behavior"""
"""Declarative source defined by a manifest of low-code components that define source connector behavior.

This class can also handle custom components if they are provided in the config using the
`__injected_components_py` config key.
"""

def __init__(
self,
Expand Down
18 changes: 18 additions & 0 deletions airbyte_cdk/test/declarative/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Copyright (c) 2024 Airbyte, Inc., all rights reserved.
"""Declarative tests framework.

This module provides fixtures and utilities for testing Airbyte sources and destinations
in a declarative way.
"""

from airbyte_cdk.test.declarative.test_suites import (
ConnectorTestSuiteBase,
DestinationTestSuiteBase,
SourceTestSuiteBase,
)

__all__ = [
"ConnectorTestSuiteBase",
"DestinationTestSuiteBase",
"SourceTestSuiteBase",
]
16 changes: 16 additions & 0 deletions airbyte_cdk/test/declarative/connector_interfaces.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from dataclasses import dataclass
from typing import Protocol, Type


class ConnectorInterface(Protocol):
"""Protocol for Airbyte connectors."""

@classmethod
def launch(cls, args: list[str] | None): ...


@dataclass
class PythonWrapper:
"""Wrapper for Python source and destination connectors."""

connector_class: Type["ConnectorInterface"]
7 changes: 7 additions & 0 deletions airbyte_cdk/test/declarative/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from scenario import (
AcceptanceTestScenario,
)

__all__ = [
"AcceptanceTestScenario",
]
48 changes: 48 additions & 0 deletions airbyte_cdk/test/declarative/models/scenario.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Copyright (c) 2024 Airbyte, Inc., all rights reserved.
"""Run acceptance tests in PyTest.

These tests leverage the same `acceptance-test-config.yml` configuration files as the
acceptance tests in CAT, but they run in PyTest instead of CAT. This allows us to run
the acceptance tests in the same local environment as we are developing in, speeding
up iteration cycles.
"""

from __future__ import annotations

from pathlib import Path
from typing import Literal

import yaml
from pydantic import BaseModel


class AcceptanceTestScenario(BaseModel):
"""Acceptance test instance, as a Pydantic model.

This class represents an acceptance test instance, 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.
"""

class AcceptanceTestExpectRecords(BaseModel):
path: Path
exact_order: bool = False

class AcceptanceTestFileTypes(BaseModel):
skip_test: bool
bypass_reason: str

config_path: Path
configured_catalog_path: Path | None = None
timeout_seconds: int | None = None
expect_records: AcceptanceTestExpectRecords | None = None
file_types: AcceptanceTestFileTypes | None = None
status: Literal["succeed", "failed"] | None = None

@property
def expect_exception(self) -> bool:
return self.status and self.status == "failed"
Comment on lines +42 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix type safety in expect_exception property.

The property could return None when self.status is None, which doesn't match the declared bool return type. Wdyt about this fix?

@property
def expect_exception(self) -> bool:
-    return self.status and self.status == "failed"
+    return bool(self.status and self.status == "failed")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@property
def expect_exception(self) -> bool:
return self.status and self.status == "failed"
@property
def expect_exception(self) -> bool:
- return self.status and self.status == "failed"
+ return bool(self.status and self.status == "failed")
🧰 Tools
🪛 GitHub Actions: Linters

[error] 44-44: Incompatible return value type (got 'bool | None', expected 'bool')


@property
def instance_name(self) -> str:
return self.config_path.stem
15 changes: 15 additions & 0 deletions airbyte_cdk/test/declarative/test_suites/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright (c) 2024 Airbyte, Inc., all rights reserved.
"""Declarative test suites.

Here we have base classes for a robust set of declarative connector test suites.
"""

from airbyte_cdk.test.declarative.test_suites.connector_base import ConnectorTestSuiteBase
from airbyte_cdk.test.declarative.test_suites.destination_base import DestinationTestSuiteBase
from airbyte_cdk.test.declarative.test_suites.source_base import SourceTestSuiteBase

__all__ = [
"ConnectorTestSuiteBase",
"DestinationTestSuiteBase",
"SourceTestSuiteBase",
]
110 changes: 110 additions & 0 deletions airbyte_cdk/test/declarative/test_suites/connector_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# Copyright (c) 2024 Airbyte, Inc., all rights reserved.
"""Base class for connector test suites."""

from __future__ import annotations

import abc
from pathlib import Path
from typing import Any, Literal

import pytest
import yaml
from airbyte_connector_tester.job_runner import run_test_job
from pydantic import BaseModel

from airbyte_cdk import Connector
from airbyte_cdk.models import (
AirbyteMessage,
Type,
)
from airbyte_cdk.test import entrypoint_wrapper
from airbyte_cdk.test.declarative.models import (
AcceptanceTestScenario,
)

ACCEPTANCE_TEST_CONFIG_PATH = Path("acceptance-test-config.yml")


class ConnectorTestSuiteBase(abc.ABC):
"""Base class for connector test suites."""

acceptance_test_file_path = Path("./acceptance-test-config.json")
"""The path to the acceptance test config file.

By default, this is set to the `acceptance-test-config.json` file in
the root of the connector source directory.
"""

connector_class: type[Connector]
"""The connector class to test."""

# Public Methods - Subclasses may override these

@abc.abstractmethod
def new_connector(self, **kwargs: dict[str, Any]) -> Connector:
"""Create a new connector instance.

By default, this returns a new instance of the connector class. Subclasses
may override this method to generate a dynamic connector instance.
"""
return self.connector_factory()
Comment on lines +43 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix implementation of abstract method new_connector

The current implementation has a few issues:

  1. The method is marked as abstract but provides an implementation
  2. It uses undefined connector_factory
  3. The return type annotation is missing in the implementation

What do you think about this implementation instead?

@abc.abstractmethod
def new_connector(self, **kwargs: dict[str, Any]) -> Connector:
    """Create a new connector instance.

    By default, this returns a new instance of the connector class. Subclasses
    may override this method to generate a dynamic connector instance.
    """
-    return self.connector_factory()
+    raise NotImplementedError("Subclasses must implement new_connector")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@abc.abstractmethod
def new_connector(self, **kwargs: dict[str, Any]) -> Connector:
"""Create a new connector instance.
By default, this returns a new instance of the connector class. Subclasses
may override this method to generate a dynamic connector instance.
"""
return self.connector_factory()
@abc.abstractmethod
def new_connector(self, **kwargs: dict[str, Any]) -> Connector:
"""Create a new connector instance.
By default, this returns a new instance of the connector class. Subclasses
may override this method to generate a dynamic connector instance.
"""
- return self.connector_factory()
+ raise NotImplementedError("Subclasses must implement new_connector")
🧰 Tools
🪛 GitHub Actions: Linters

[error] 50-50: Returning Any from function declared to return 'Connector'


[error] 50-50: 'ConnectorTestSuiteBase' has no attribute 'connector_factory'


# Internal Methods - We don't expect subclasses to override these

@classmethod
def _get_acceptance_tests(
category: str,
accept_test_config_path: Path = ACCEPTANCE_TEST_CONFIG_PATH,
) -> list[AcceptanceTestScenario]:
all_tests_config = yaml.safe_load(accept_test_config_path.read_text())
if "acceptance_tests" not in all_tests_config:
raise ValueError(f"Acceptance tests config not found in {accept_test_config_path}")
if category not in all_tests_config["acceptance_tests"]:
return []
if "tests" not in all_tests_config["acceptance_tests"][category]:
raise ValueError(f"No tests found for category {category}")

return [
AcceptanceTestScenario.model_validate(test)
for test in all_tests_config["acceptance_tests"][category]["tests"]
if "iam_role" not in test["config_path"]
]
Comment on lines +54 to +71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix classmethod signature

The _get_acceptance_tests classmethod is missing the cls parameter. This will cause issues when accessing class attributes.

How about this fix?

@classmethod
def _get_acceptance_tests(
+    cls,
    category: str,
    accept_test_config_path: Path = ACCEPTANCE_TEST_CONFIG_PATH,
) -> list[AcceptanceTestScenario]:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@classmethod
def _get_acceptance_tests(
category: str,
accept_test_config_path: Path = ACCEPTANCE_TEST_CONFIG_PATH,
) -> list[AcceptanceTestScenario]:
all_tests_config = yaml.safe_load(accept_test_config_path.read_text())
if "acceptance_tests" not in all_tests_config:
raise ValueError(f"Acceptance tests config not found in {accept_test_config_path}")
if category not in all_tests_config["acceptance_tests"]:
return []
if "tests" not in all_tests_config["acceptance_tests"][category]:
raise ValueError(f"No tests found for category {category}")
return [
AcceptanceTestScenario.model_validate(test)
for test in all_tests_config["acceptance_tests"][category]["tests"]
if "iam_role" not in test["config_path"]
]
@classmethod
def _get_acceptance_tests(
cls,
category: str,
accept_test_config_path: Path = ACCEPTANCE_TEST_CONFIG_PATH,
) -> list[AcceptanceTestScenario]:
all_tests_config = yaml.safe_load(accept_test_config_path.read_text())
if "acceptance_tests" not in all_tests_config:
raise ValueError(f"Acceptance tests config not found in {accept_test_config_path}")
if category not in all_tests_config["acceptance_tests"]:
return []
if "tests" not in all_tests_config["acceptance_tests"][category]:
raise ValueError(f"No tests found for category {category}")
return [
AcceptanceTestScenario.model_validate(test)
for test in all_tests_config["acceptance_tests"][category]["tests"]
if "iam_role" not in test["config_path"]
]
🧰 Tools
🪛 GitHub Actions: Linters

[error] 55-55: Self argument missing for a non-static method (or an invalid type for self)


# Test Definitions

@pytest.mark.parametrize(
"test_input,expected",
[
("3+5", 8),
("2+4", 6),
("6*9", 54),
],
)
def test_use_plugin_parametrized_test(
self,
test_input,
expected,
):
assert eval(test_input) == expected
Comment on lines +83 to +88
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add type annotations to test method

The test method is missing type annotations for its parameters and return type.

Would this implementation be better?

def test_use_plugin_parametrized_test(
    self,
-    test_input,
-    expected,
+    test_input: str,
+    expected: int,
-):
+) -> None:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_use_plugin_parametrized_test(
self,
test_input,
expected,
):
assert eval(test_input) == expected
def test_use_plugin_parametrized_test(
self,
test_input: str,
expected: int,
) -> None:
assert eval(test_input) == expected
🧰 Tools
🪛 GitHub Actions: Linters

[error] 83-83: Function is missing a type annotation


@pytest.mark.parametrize(
"instance",
self._get_acceptance_tests("connection"),
ids=lambda instance: instance.instance_name,
Comment on lines +90 to +93
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix parametrize decorator usage

The decorator is using undefined self when calling _get_acceptance_tests.

What about using cls instead?

@pytest.mark.parametrize(
    "instance",
-    self._get_acceptance_tests("connection"),
+    _get_acceptance_tests("connection"),
    ids=lambda instance: instance.instance_name,
)

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Actions: Linters

[error] 92-92: Name 'self' is not defined

)
def test_check(
self,
instance: AcceptanceTestScenario,
) -> None:
"""Run `connection` acceptance tests."""
result: entrypoint_wrapper.EntrypointOutput = run_test_job(
self.new_connector(),
"check",
test_instance=instance,
)
conn_status_messages: list[AirbyteMessage] = [
msg for msg in result._messages if msg.type == Type.CONNECTION_STATUS
] # noqa: SLF001 # Non-public API
assert len(conn_status_messages) == 1, (
"Expected exactly one CONNECTION_STATUS message. Got: \n" + "\n".join(result._messages)
)
Comment on lines +105 to +110
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix string joining of AirbyteMessages

The code attempts to join AirbyteMessage objects directly, which will fail as they need to be converted to strings first.

How about this fix?

assert len(conn_status_messages) == 1, (
-    "Expected exactly one CONNECTION_STATUS message. Got: \n" + "\n".join(result._messages)
+    "Expected exactly one CONNECTION_STATUS message. Got: \n" + "\n".join(str(msg) for msg in result._messages)
)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
conn_status_messages: list[AirbyteMessage] = [
msg for msg in result._messages if msg.type == Type.CONNECTION_STATUS
] # noqa: SLF001 # Non-public API
assert len(conn_status_messages) == 1, (
"Expected exactly one CONNECTION_STATUS message. Got: \n" + "\n".join(result._messages)
)
conn_status_messages: list[AirbyteMessage] = [
msg for msg in result._messages if msg.type == Type.CONNECTION_STATUS
] # noqa: SLF001 # Non-public API
assert len(conn_status_messages) == 1, (
"Expected exactly one CONNECTION_STATUS message. Got: \n" + "\n".join(str(msg) for msg in result._messages)
)
🧰 Tools
🪛 GitHub Actions: Linters

[error] 109-109: Argument 1 to 'join' of 'str' has incompatible type 'list[AirbyteMessage]'; expected 'Iterable[str]'

12 changes: 12 additions & 0 deletions airbyte_cdk/test/declarative/test_suites/destination_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (c) 2024 Airbyte, Inc., all rights reserved.
"""Base class for destination test suites."""

from airbyte_connector_tester.connector_tests import ConnectorTestSuiteBase


class DestinationTestSuiteBase(ConnectorTestSuiteBase):
"""Base class for destination test suites.

This class provides a base set of functionality for testing destination connectors, and it
inherits all generic connector tests from the `ConnectorTestSuiteBase` class.
"""
Loading
Loading