diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index 43a344cac6df..4735703751e0 100644 --- a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md +++ b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 0.1.59 +Backward compatibility tests: add syntactic validation of specs [#15194](https://github.com/airbytehq/airbyte/pull/15194/). + ## 0.1.58 Bootstrap spec backward compatibility tests. Add fixtures to retrieve a previous connector version spec [#14954](https://github.com/airbytehq/airbyte/pull/14954/). diff --git a/airbyte-integrations/bases/source-acceptance-test/Dockerfile b/airbyte-integrations/bases/source-acceptance-test/Dockerfile index f1b78978db01..92d1f22eb9bb 100644 --- a/airbyte-integrations/bases/source-acceptance-test/Dockerfile +++ b/airbyte-integrations/bases/source-acceptance-test/Dockerfile @@ -33,7 +33,7 @@ COPY pytest.ini setup.py ./ COPY source_acceptance_test ./source_acceptance_test RUN pip install . -LABEL io.airbyte.version=0.1.58 +LABEL io.airbyte.version=0.1.59 LABEL io.airbyte.name=airbyte/source-acceptance-test ENTRYPOINT ["python", "-m", "pytest", "-p", "source_acceptance_test.plugin", "-r", "fEsx"] diff --git a/airbyte-integrations/bases/source-acceptance-test/pytest.ini b/airbyte-integrations/bases/source-acceptance-test/pytest.ini index e1827862065a..1eff47fef43c 100644 --- a/airbyte-integrations/bases/source-acceptance-test/pytest.ini +++ b/airbyte-integrations/bases/source-acceptance-test/pytest.ini @@ -6,3 +6,4 @@ testpaths = markers = default_timeout + backward_compatibility diff --git a/airbyte-integrations/bases/source-acceptance-test/setup.py b/airbyte-integrations/bases/source-acceptance-test/setup.py index dfffee08a770..6b53663573c2 100644 --- a/airbyte-integrations/bases/source-acceptance-test/setup.py +++ b/airbyte-integrations/bases/source-acceptance-test/setup.py @@ -20,6 +20,7 @@ "dpath~=2.0.1", "jsonschema~=3.2.0", "jsonref==0.2", + "deepdiff~=5.8.0", "requests-mock", "pytest-mock~=3.6.1", ] diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/config.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/config.py index b974d03e0b7f..dfcbc8ba33db 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/config.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/config.py @@ -17,6 +17,8 @@ configured_catalog_path: Optional[str] = Field(default=None, description="Path to configured catalog") timeout_seconds: int = Field(default=None, description="Test execution timeout_seconds", ge=0) +SEMVER_REGEX = r"(0|(?:[1-9]\d*))(?:\.(0|(?:[1-9]\d*))(?:\.(0|(?:[1-9]\d*)))?(?:\-([\w][\w\.\-_]*))?)?" + class BaseConfig(BaseModel): class Config: @@ -25,10 +27,10 @@ class Config: class BackwardCompatibilityTestsConfig(BaseConfig): previous_connector_version: str = Field( - default="latest", description="Previous connector version to use for backward compatibility tests." + regex=SEMVER_REGEX, default="latest", description="Previous connector version to use for backward compatibility tests." ) - disable_backward_compatibility_tests_for_version: Optional[str] = Field( - default=None, description="Disable backward compatibility tests for a specific connector version." + disable_for_version: Optional[str] = Field( + regex=SEMVER_REGEX, default=None, description="Disable backward compatibility tests for a specific connector version." ) diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/conftest.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/conftest.py index 6fc61ff76b70..301267c0329a 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/conftest.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/conftest.py @@ -23,8 +23,9 @@ Type, ) from docker import errors +from source_acceptance_test.base import BaseTest from source_acceptance_test.config import Config -from source_acceptance_test.utils import ConnectorRunner, SecretDict, load_config, load_yaml_or_json_path +from source_acceptance_test.utils import ConnectorRunner, SecretDict, filter_output, load_config, load_yaml_or_json_path @pytest.fixture(name="base_path") @@ -210,6 +211,35 @@ def detailed_logger() -> Logger: return logger +@pytest.fixture(name="actual_connector_spec") +def actual_connector_spec_fixture(request: BaseTest, docker_runner: ConnectorRunner) -> ConnectorSpecification: + if not request.instance.spec_cache: + output = docker_runner.call_spec() + spec_messages = filter_output(output, Type.SPEC) + assert len(spec_messages) == 1, "Spec message should be emitted exactly once" + spec = spec_messages[0].spec + request.instance.spec_cache = spec + return request.instance.spec_cache + + +@pytest.fixture(name="previous_connector_spec") +def previous_connector_spec_fixture( + request: BaseTest, previous_connector_docker_runner: ConnectorRunner +) -> Optional[ConnectorSpecification]: + if previous_connector_docker_runner is None: + logging.warning( + "\n We could not retrieve the previous connector spec as a connector runner for the previous connector version could not be instantiated." + ) + return None + if not request.instance.previous_spec_cache: + output = previous_connector_docker_runner.call_spec() + spec_messages = filter_output(output, Type.SPEC) + assert len(spec_messages) == 1, "Spec message should be emitted exactly once" + spec = spec_messages[0].spec + request.instance.previous_spec_cache = spec + return request.instance.previous_spec_cache + + def pytest_sessionfinish(session, exitstatus): """Called after whole test run finished, right before returning the exit status to the system. https://docs.pytest.org/en/6.2.x/reference.html#pytest.hookspec.pytest_sessionfinish diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/plugin.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/plugin.py index 3875cad4dd29..0145ce88699b 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/plugin.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/plugin.py @@ -16,6 +16,10 @@ def pytest_configure(config): config.addinivalue_line("markers", "default_timeout: mark test to be wrapped by `timeout` decorator with default value") + config.addinivalue_line( + "markers", + "backward_compatibility: mark test to be part of the backward compatibility tests suite (deselect with '-m \"not backward_compatibility\"')", + ) def pytest_load_initial_conftests(early_config: Config, parser: Parser, args: List[str]): diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py index d5986b53561a..068bd614e6c8 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py @@ -8,7 +8,7 @@ from collections import Counter, defaultdict from functools import reduce from logging import Logger -from typing import Any, Dict, List, Mapping, MutableMapping, Optional, Set +from typing import Any, Dict, List, Mapping, MutableMapping, Set import dpath.util import jsonschema @@ -24,6 +24,7 @@ TraceType, Type, ) +from deepdiff import DeepDiff from docker.errors import ContainerError from jsonschema._utils import flatten from source_acceptance_test.base import BaseTest @@ -38,41 +39,38 @@ def connector_spec_dict_fixture(actual_connector_spec): return json.loads(actual_connector_spec.json()) -@pytest.fixture(name="actual_connector_spec") -def actual_connector_spec_fixture(request: BaseTest, docker_runner: ConnectorRunner) -> ConnectorSpecification: - if not request.instance.spec_cache: - output = docker_runner.call_spec() - spec_messages = filter_output(output, Type.SPEC) - assert len(spec_messages) == 1, "Spec message should be emitted exactly once" - spec = spec_messages[0].spec - request.spec_cache = spec - return request.spec_cache - - -@pytest.fixture(name="previous_connector_spec") -def previous_connector_spec_fixture( - request: BaseTest, previous_connector_docker_runner: ConnectorRunner -) -> Optional[ConnectorSpecification]: - if previous_connector_docker_runner is None: - logging.warning( - "\n We could not retrieve the previous connector spec as a connector runner for the previous connector version could not be instantiated." - ) - return None - if not request.instance.previous_spec_cache: - output = previous_connector_docker_runner.call_spec() - spec_messages = filter_output(output, Type.SPEC) - assert len(spec_messages) == 1, "Spec message should be emitted exactly once" - spec = spec_messages[0].spec - request.instance.previous_spec_cache = spec - return request.instance.previous_spec_cache - - @pytest.mark.default_timeout(10) class TestSpec(BaseTest): spec_cache: ConnectorSpecification = None previous_spec_cache: ConnectorSpecification = None + @staticmethod + def compute_spec_diff(actual_connector_spec: ConnectorSpecification, previous_connector_spec: ConnectorSpecification): + return DeepDiff(previous_connector_spec.dict(), actual_connector_spec.dict(), view="tree", ignore_order=True) + + @pytest.fixture(name="skip_backward_compatibility_tests") + def skip_backward_compatibility_tests_fixture(self, inputs: SpecTestConfig, previous_connector_docker_runner: ConnectorRunner) -> bool: + if previous_connector_docker_runner is None: + pytest.skip("The previous connector image could not be retrieved.") + + # Get the real connector version in case 'latest' is used in the config: + previous_connector_version = previous_connector_docker_runner._image.labels.get("io.airbyte.version") + + if previous_connector_version == inputs.backward_compatibility_tests_config.disable_for_version: + pytest.skip(f"Backward compatibility tests are disabled for version {previous_connector_version}.") + return False + + @pytest.fixture(name="spec_diff") + def spec_diff_fixture( + self, + skip_backward_compatibility_tests: bool, + actual_connector_spec: ConnectorSpecification, + previous_connector_spec: ConnectorSpecification, + ) -> DeepDiff: + assert isinstance(actual_connector_spec, ConnectorSpecification) and isinstance(previous_connector_spec, ConnectorSpecification) + return self.compute_spec_diff(actual_connector_spec, previous_connector_spec) + def test_config_match_spec(self, actual_connector_spec: ConnectorSpecification, connector_config: SecretDict): """Check that config matches the actual schema from the spec call""" # Getting rid of technical variables that start with an underscore @@ -182,7 +180,108 @@ def test_oauth_flow_parameters(self, actual_connector_spec: ConnectorSpecificati diff = params - schema_path assert diff == set(), f"Specified oauth fields are missed from spec schema: {diff}" - def test_additional_properties_is_true(self, actual_connector_spec): + @pytest.mark.default_timeout(60) + @pytest.mark.backward_compatibility + def test_new_required_field_declaration(self, spec_diff: DeepDiff): + """Check if a 'required' field was added to the spec.""" + added_required_fields = [ + addition for addition in spec_diff.get("dictionary_item_added", []) if addition.path(output_format="list")[-1] == "required" + ] + assert len(added_required_fields) == 0, f"The current spec has a new required field: {spec_diff.pretty()}" + + @pytest.mark.default_timeout(60) + @pytest.mark.backward_compatibility + def test_new_required_property(self, spec_diff: DeepDiff): + """Check if a a new property was added to the 'required' field.""" + added_required_properties = [ + addition for addition in spec_diff.get("iterable_item_added", []) if addition.up.path(output_format="list")[-1] == "required" + ] + assert len(added_required_properties) == 0, f"The current spec has a new required property: {spec_diff.pretty()}" + + @pytest.mark.default_timeout(60) + @pytest.mark.backward_compatibility + def test_field_type_changed(self, spec_diff: DeepDiff): + """Check if the current spec is changing the types of properties.""" + + common_error_message = f"The current spec changed the value of a 'type' field: {spec_diff.pretty()}" + # Detect type value change in case type field is declared as a string (e.g "str" -> "int"): + type_values_changed = [change for change in spec_diff.get("values_changed", []) if change.path(output_format="list")[-1] == "type"] + + # Detect type value change in case type field is declared as a single item list (e.g ["str"] -> ["int"]): + type_values_changed_in_list = [ + change for change in spec_diff.get("values_changed", []) if change.path(output_format="list")[-2] == "type" + ] + + assert len(type_values_changed_in_list) == 0 and len(type_values_changed) == 0, common_error_message + + # Detect type value added to type list if new type value is not None (e.g ["str"] -> ["str", "int"]): + # It works because we compute the diff with 'ignore_order=True' + new_values_in_type_list = [ # noqa: F841 + change + for change in spec_diff.get("iterable_item_added", []) + if change.path(output_format="list")[-2] == "type" + if change.t2 != "null" + ] + # enable the assertion below if we want to disallow type expansion: + # assert len(new_values_in_type_list) == 0, common_error_message + + # Detect the change of type of a type field + # e.g: + # - "str" -> ["str"] VALID + # - "str" -> ["str", "null"] VALID + # - "str" -> ["str", "int"] VALID + # - "str" -> 1 INVALID + # - ["str"] -> "str" VALID + # - ["str"] -> "int" INVALID + # - ["str"] -> 1 INVALID + type_changes = [change for change in spec_diff.get("type_changes", []) if change.path(output_format="list")[-1] == "type"] + for change in type_changes: + # We only accept change on the type field if the new type for this field is list or string + # This might be something already guaranteed by JSON schema validation. + if isinstance(change.t1, str): + assert isinstance( + change.t2, list + ), f"The current spec change a type field from string to an invalid value: {spec_diff.pretty()}" + assert ( + 0 < len(change.t2) <= 2 + ), f"The current spec change a type field from string to an invalid value. The type list length should not be empty and have a maximum of two items {spec_diff.pretty()}." + # If the new type field is a list we want to make sure it only has the original type (t1) and null: e.g. "str" -> ["str", "null"] + # We want to raise an error otherwise. + t2_not_null_types = [_type for _type in change.t2 if _type != "null"] + assert ( + len(t2_not_null_types) == 1 and t2_not_null_types[0] == change.t1 + ), "The current spec change a type field to a list with multiple invalid values." + if isinstance(change.t1, list): + assert isinstance( + change.t2, str + ), f"The current spec change a type field from list to an invalid value: {spec_diff.pretty()}" + assert len(change.t1) == 1 and change.t2 == change.t1[0], f"The current spec narrowed a field type: {spec_diff.pretty()}" + + # Detect when field was made not nullable but is still a list: e.g ["string", "null"] -> ["string"] + removed_nullable = [ + change for change in spec_diff.get("iterable_item_removed", []) if change.path(output_format="list")[-2] == "type" + ] + assert len(removed_nullable) == 0, f"The current spec narrowed a field type: {spec_diff.pretty()}" + + @pytest.mark.default_timeout(60) + @pytest.mark.backward_compatibility + def test_enum_field_has_narrowed(self, spec_diff: DeepDiff): + """Check if the list of values in a enum was shortened in a spec.""" + removals = [ + removal for removal in spec_diff.get("iterable_item_removed", []) if removal.up.path(output_format="list")[-1] == "enum" + ] + assert len(removals) == 0, f"The current spec narrowed a field enum: {spec_diff.pretty()}" + + @pytest.mark.default_timeout(60) + @pytest.mark.backward_compatibility + def test_new_enum_field_declaration(self, spec_diff: DeepDiff): + """Check if an 'enum' field was added to the spec.""" + added_enum_fields = [ + addition for addition in spec_diff.get("dictionary_item_added", []) if addition.path(output_format="list")[-1] == "enum" + ] + assert len(added_enum_fields) == 0, f"An 'enum' field was declared on an existing property of the spec: {spec_diff.pretty()}" + + def test_additional_properties_is_true(self, actual_connector_spec: ConnectorSpecification): """Check that value of the "additionalProperties" field is always true. A spec declaring "additionalProperties": false introduces the risk of accidental breaking changes. Specifically, when removing a property from the spec, existing connector configs will no longer be valid. @@ -196,24 +295,6 @@ def test_additional_properties_is_true(self, actual_connector_spec): [additional_properties_value is True for additional_properties_value in additional_properties_values] ), "When set, additionalProperties field value must be true for backward compatibility." - @pytest.mark.default_timeout(60) # Pulling the previous connector image can take more than 10 sec. - @pytest.mark.spec_backward_compatibility - def test_backward_compatibility( - self, inputs: SpecTestConfig, actual_connector_spec: ConnectorSpecification, previous_connector_spec: ConnectorSpecification - ): - """Run multiple checks to make sure the actual_connector_spec is backward compatible with the previous_connector_spec""" - if ( - inputs.backward_compatibility_tests_config.disable_backward_compatibility_tests_for_version - == inputs.backward_compatibility_tests_config.previous_connector_version - ): - pytest.skip( - f"Backward compatibility tests are disabled for version {inputs.backward_compatibility_tests_config.disable_backward_compatibility_tests_for_version}." - ) - if previous_connector_spec is None: - pytest.skip("The previous connector spec could not be retrieved.") - assert isinstance(actual_connector_spec, ConnectorSpecification) and isinstance(previous_connector_spec, ConnectorSpecification) - # TODO alafanechere: add the actual tests for backward compatibility below or in a dedicated module. - @pytest.mark.default_timeout(30) class TestConnection(BaseTest): diff --git a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_backward_compatibility.py b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_backward_compatibility.py new file mode 100644 index 000000000000..e97ee150c247 --- /dev/null +++ b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_backward_compatibility.py @@ -0,0 +1,977 @@ +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# + +import pytest +from airbyte_cdk.models import ConnectorSpecification +from source_acceptance_test.tests.test_core import TestSpec as _TestSpec + +from .conftest import does_not_raise + + +@pytest.mark.parametrize( + "connector_spec, expectation", + [ + (ConnectorSpecification(connectionSpecification={}), does_not_raise()), + (ConnectorSpecification(connectionSpecification={"type": "object", "additionalProperties": True}), does_not_raise()), + (ConnectorSpecification(connectionSpecification={"type": "object", "additionalProperties": False}), pytest.raises(AssertionError)), + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "additionalProperties": True, + "properties": {"my_object": {"type": "object", "additionalProperties": "foo"}}, + } + ), + pytest.raises(AssertionError), + ), + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "additionalProperties": True, + "properties": { + "my_oneOf_object": {"type": "object", "oneOf": [{"additionalProperties": True}, {"additionalProperties": False}]} + }, + } + ), + pytest.raises(AssertionError), + ), + ], +) +def test_additional_properties_is_true(connector_spec, expectation): + t = _TestSpec() + with expectation: + t.test_additional_properties_is_true(connector_spec) + + +@pytest.mark.parametrize( + "previous_connector_spec, actual_connector_spec, expectation", + [ + pytest.param( + ConnectorSpecification(connectionSpecification={}), + ConnectorSpecification( + connectionSpecification={ + "required": ["a", "b"], + } + ), + pytest.raises(AssertionError), + id="Top level: declaring the required field should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": {"my_optional_object": {"type": "object", "properties": {"optional_property": {"type": "string"}}}}, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_optional_object": { + "type": "object", + "required": ["optional_property"], + "properties": {"optional_property": {"type": "string"}}, + } + }, + } + ), + pytest.raises(AssertionError), + id="Nested level: adding the required field should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "required": ["my_required_string"], + "properties": { + "my_required_string": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "required": ["my_required_string"], + "properties": { + "my_required_string": {"type": "string"}, + "my_optional_object": { + "type": "object", + "required": ["another_required_string"], + "properties": {"another_required_string": {"type": "string"}}, + }, + }, + } + ), + does_not_raise(), + id="Adding an optional object with required properties should not fail.", + ), + ], +) +def test_new_required_field_declaration(previous_connector_spec, actual_connector_spec, expectation): + t = _TestSpec() + spec_diff = t.compute_spec_diff(actual_connector_spec, previous_connector_spec) + with expectation: + t.test_new_required_field_declaration(spec_diff) + + +@pytest.mark.parametrize( + "previous_connector_spec, actual_connector_spec, expectation", + [ + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "required": ["a"], + } + ), + ConnectorSpecification( + connectionSpecification={ + "required": ["a", "b"], + } + ), + pytest.raises(AssertionError), + id="Top level: adding a new required property should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_optional_object": { + "type": "object", + "required": ["first_required_property"], + "properties": {"first_required_property": {"type": "string"}}, + } + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_optional_object": { + "type": "object", + "required": ["first_required_property", "second_required_property"], + "properties": { + "first_required_property": {"type": "string"}, + "second_required_property": {"type": "string"}, + }, + } + }, + } + ), + pytest.raises(AssertionError), + id="Nested level: adding a new required property should fail.", + ), + ], +) +def test_new_required_property(previous_connector_spec, actual_connector_spec, expectation): + t = _TestSpec() + spec_diff = t.compute_spec_diff(actual_connector_spec, previous_connector_spec) + with expectation: + t.test_new_required_property(spec_diff) + + +@pytest.mark.parametrize( + "previous_connector_spec, actual_connector_spec, expectation", + [ + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "int"}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + does_not_raise(), + id="No change should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + does_not_raise(), + id="Changing a field type from a string to a list should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["int"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type from a string to a list with a different type value should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "int"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["int", "int"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type from a string to a list with duplicate same type should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "int"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["int", "null", "str"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type from a string to a list with more than two values should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "int"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": []}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type from a string to an empty list should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["int"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": []}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type from a list to an empty list should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "int"}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type should fail from a list to string with different value should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + does_not_raise(), + id="Changing a field type from a list to a string with same value should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["int"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type in list should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str", "int"]}, + }, + } + ), + does_not_raise(), + id="Adding a field type in list should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "str"]}, + }, + } + ), + does_not_raise(), + id="Making a field nullable should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str", "null"]}, + }, + } + ), + does_not_raise(), + id="Making a field nullable should not fail (change list order).", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "str"]}, + }, + } + ), + does_not_raise(), + id="Making a field nullable should not fail (from a list).", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str", "null"]}, + }, + } + ), + does_not_raise(), + id="Making a field nullable should not fail (from a list, changing order).", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["int", "null"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Making a field nullable and changing type should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "int"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Making a field nullable and changing type should fail (change list order).", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "str"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": 1}, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type from a string to something else than a list should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "int"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Nullable field: Changing a field type should fail", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str", "null"]}, + }, + } + ), + does_not_raise(), + id="Nullable field: Changing order should not fail", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "str"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["str"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Nullable field: Making a field not nullable should fail", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": ["null", "string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string"}, + }, + } + ), + pytest.raises(AssertionError), + id="Nullable: Making a field not nullable should fail (not in a list).", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["null", "int"]}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["int"]}}}, + }, + } + ), + pytest.raises(AssertionError), + id="Nested level: Narrowing a field type should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["int"]}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["null", "int"]}}}, + }, + } + ), + does_not_raise(), + id="Nested level: Expanding a field type should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": "str"}, + {"title": "b", "type": "int"}, + ] + }, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": "int"}, + {"title": "b", "type": "int"}, + ] + }, + }, + } + ), + pytest.raises(AssertionError), + id="Changing a field type in oneOf should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": "str"}, + {"title": "b", "type": "int"}, + ] + }, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "b", "type": "str"}, + {"title": "a", "type": "int"}, + ] + }, + }, + } + ), + does_not_raise(), + id="Changing a order in oneOf should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": ["str", "int"]}, + {"title": "b", "type": "int"}, + ] + }, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": ["str"]}, + {"title": "b", "type": "int"}, + ] + }, + }, + } + ), + pytest.raises(AssertionError), + id="Narrowing a field type in oneOf should fail.", + ), + ], +) +def test_field_type_changed(previous_connector_spec, actual_connector_spec, expectation): + t = _TestSpec() + spec_diff = t.compute_spec_diff(actual_connector_spec, previous_connector_spec) + with expectation: + t.test_field_type_changed(spec_diff) + + +@pytest.mark.parametrize( + "previous_connector_spec, actual_connector_spec, expectation", + [ + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Top level: Narrowing a field enum should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + does_not_raise(), + id="Top level: Expanding a field enum should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string", "enum": ["a", "b"]}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string", "enum": ["a"]}}}, + }, + } + ), + pytest.raises(AssertionError), + id="Nested level: Narrowing a field enum should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string", "enum": ["a"]}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string", "enum": ["a", "b"]}}}, + }, + } + ), + does_not_raise(), + id="Nested level: Expanding a field enum should not fail.", + ), + ], +) +def test_enum_field_has_narrowed(previous_connector_spec, actual_connector_spec, expectation): + t = _TestSpec() + spec_diff = t.compute_spec_diff(actual_connector_spec, previous_connector_spec) + with expectation: + t.test_enum_field_has_narrowed(spec_diff) + + +@pytest.mark.parametrize( + "previous_connector_spec, actual_connector_spec, expectation", + [ + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + pytest.raises(AssertionError), + id="Top level: Declaring a field enum should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string"}, + }, + } + ), + does_not_raise(), + id="Top level: Removing the field enum should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": {"my_string": {"type": "string", "enum": ["a", "b"]}, "my_enum": {"type": "string", "enum": ["c", "d"]}}, + } + ), + does_not_raise(), + id="Top level: Adding a new optional field with enum should not fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string"}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string", "enum": ["a", "b"]}}}, + }, + } + ), + pytest.raises(AssertionError), + id="Nested level: Declaring a field enum should fail.", + ), + pytest.param( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string", "enum": ["a", "b"]}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": "string"}}}, + }, + } + ), + does_not_raise(), + id="Nested level: Removing the enum field should not fail.", + ), + ], +) +def test_new_enum_field_declaration(previous_connector_spec, actual_connector_spec, expectation): + t = _TestSpec() + spec_diff = t.compute_spec_diff(actual_connector_spec, previous_connector_spec) + with expectation: + t.test_new_enum_field_declaration(spec_diff) diff --git a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py index 3a09ce07f887..ea8fe2d57dc9 100644 --- a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py +++ b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py @@ -8,8 +8,6 @@ from airbyte_cdk.models import ConnectorSpecification from source_acceptance_test.tests.test_core import TestSpec as _TestSpec -from .conftest import does_not_raise - @pytest.mark.parametrize( "connector_spec, should_fail", @@ -569,39 +567,3 @@ def test_validate_oauth_flow(connector_spec, expected_error): t.test_oauth_flow_parameters(connector_spec) else: t.test_oauth_flow_parameters(connector_spec) - - -@pytest.mark.parametrize( - "connector_spec, expectation", - [ - (ConnectorSpecification(connectionSpecification={}), does_not_raise()), - (ConnectorSpecification(connectionSpecification={"type": "object", "additionalProperties": True}), does_not_raise()), - (ConnectorSpecification(connectionSpecification={"type": "object", "additionalProperties": False}), pytest.raises(AssertionError)), - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "additionalProperties": True, - "properties": {"my_object": {"type": "object", "additionalProperties": "foo"}}, - } - ), - pytest.raises(AssertionError), - ), - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "additionalProperties": True, - "properties": { - "my_oneOf_object": {"type": "object", "oneOf": [{"additionalProperties": True}, {"additionalProperties": False}]} - }, - } - ), - pytest.raises(AssertionError), - ), - ], -) -def test_additional_properties_is_true(connector_spec, expectation): - t = _TestSpec() - with expectation: - t.test_additional_properties_is_true(connector_spec) diff --git a/docs/connector-development/testing-connectors/source-acceptance-tests-reference.md b/docs/connector-development/testing-connectors/source-acceptance-tests-reference.md index ffaadea30ee0..b3b99d1310b3 100644 --- a/docs/connector-development/testing-connectors/source-acceptance-tests-reference.md +++ b/docs/connector-development/testing-connectors/source-acceptance-tests-reference.md @@ -98,8 +98,8 @@ Verify that a spec operation issued to the connector returns a valid spec. | Input | Type | Default | Note | | :--- | :--- | :--- |:-------------------------------------------------------------------------------------------------| | `spec_path` | string | `secrets/spec.json` | Path to a YAML or JSON file representing the spec expected to be output by this connector | -| `backward_compatibility_tests_config.previous_connector_version` | string | `latest` | Previous connector version to use for backward compatibility tests. | -| `backward_compatibility_tests_config.run_backward_compatibility_tests` | boolean | True | Flag to run or skip backward compatibility tests. | +| `backward_compatibility_tests_config.previous_connector_version` | string | `latest` | Previous connector version to use for backward compatibility tests (expects a version following semantic versioning). | +| `backward_compatibility_tests_config.disable_for_version` | string | None | Disable the backward compatibility test for a specific version (expects a version following semantic versioning). | | `timeout_seconds` | int | 10 | Test execution timeout in seconds | ## Test Connection