From 3c265e4653c654ddcc2b4c509ecaee78bcc92081 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Fri, 5 Aug 2022 08:31:22 +0200 Subject: [PATCH 01/14] refactoring + tests with hypothesis-jsonschema --- .../bases/source-acceptance-test/CHANGELOG.md | 3 + .../bases/source-acceptance-test/Dockerfile | 2 +- .../bases/source-acceptance-test/setup.py | 4 +- .../source_acceptance_test/tests/test_core.py | 140 +- .../utils/backward_compatibility.py | 154 ++ .../unit_tests/test_backward_compatibility.py | 1796 ++++++++--------- .../unit_tests/test_spec.py | 38 + 7 files changed, 1073 insertions(+), 1064 deletions(-) create mode 100644 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index 4735703751e08..5295716433622 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.60 +Backward compatibility tests: validate fake previous config against current connector specification. + ## 0.1.59 Backward compatibility tests: add syntactic validation of specs [#15194](https://github.com/airbytehq/airbyte/pull/15194/). diff --git a/airbyte-integrations/bases/source-acceptance-test/Dockerfile b/airbyte-integrations/bases/source-acceptance-test/Dockerfile index 92d1f22eb9bb0..69c749df2d373 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.59 +LABEL io.airbyte.version=0.1.60 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/setup.py b/airbyte-integrations/bases/source-acceptance-test/setup.py index 6b53663573c2b..7f44dc48c8ec8 100644 --- a/airbyte-integrations/bases/source-acceptance-test/setup.py +++ b/airbyte-integrations/bases/source-acceptance-test/setup.py @@ -21,8 +21,10 @@ "jsonschema~=3.2.0", "jsonref==0.2", "deepdiff~=5.8.0", - "requests-mock", + "requests-mock~=1.9.3", "pytest-mock~=3.6.1", + "hypothesis~=6.54.1", + "hypothesis-jsonschema~=0.20.1", # TODO alafanechere upgrade to latest when jsonschema lib is upgraded to >= 4.0.0 in airbyte-cdk and SAT ] setuptools.setup( 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 068bd614e6c82..54bdd2c1b568c 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 @@ -30,6 +30,7 @@ from source_acceptance_test.base import BaseTest from source_acceptance_test.config import BasicReadTestConfig, ConnectionTestConfig, SpecTestConfig from source_acceptance_test.utils import ConnectorRunner, SecretDict, filter_output, make_hashable, verify_records_schema +from source_acceptance_test.utils.backward_compatibility import SpecDiffChecker, validate_previous_configs from source_acceptance_test.utils.common import find_all_values_for_key_in_schema, find_keyword_schema from source_acceptance_test.utils.json_schema_helper import JsonSchemaHelper, get_expected_schema_structure, get_object_structure @@ -47,7 +48,12 @@ class TestSpec(BaseTest): @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) + return DeepDiff( + previous_connector_spec.dict()["connectionSpecification"], + actual_connector_spec.dict()["connectionSpecification"], + 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: @@ -61,16 +67,6 @@ def skip_backward_compatibility_tests_fixture(self, inputs: SpecTestConfig, prev 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 @@ -180,106 +176,34 @@ 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}" - @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.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 @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_backward_compatibility( + self, + skip_backward_compatibility_tests: bool, + inputs, + actual_connector_spec: ConnectorSpecification, + previous_connector_spec: ConnectorSpecification, + ): + """Check if the current spec is backward_compatible: + 1. Perform multiple hardcoded syntactic checks with SpecDiffChecker + 2. validate fake generated previous configs against the actual connector specification with validate_previous_configs + """ + assert isinstance(actual_connector_spec, ConnectorSpecification) and isinstance(previous_connector_spec, ConnectorSpecification) + spec_diff = self.compute_spec_diff(actual_connector_spec, previous_connector_spec) + checker = SpecDiffChecker(spec_diff) + checker.check() + validate_previous_configs(previous_connector_spec, actual_connector_spec) def test_additional_properties_is_true(self, actual_connector_spec: ConnectorSpecification): """Check that value of the "additionalProperties" field is always true. diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py new file mode 100644 index 0000000000000..2c947147609c7 --- /dev/null +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py @@ -0,0 +1,154 @@ +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# +import jsonschema +from airbyte_cdk.models import ConnectorSpecification +from deepdiff import DeepDiff +from hypothesis import given, settings +from hypothesis_jsonschema import from_schema +from source_acceptance_test.utils import SecretDict + + +class NonBackwardCompatibleSpecError(Exception): + pass + + +class SpecDiffChecker: + """A class to perform multiple backward compatible checks on a spec diff""" + + def __init__(self, diff: DeepDiff) -> None: + self._diff = diff + + def check(self): + self.check_if_declared_new_required_field() + self.check_if_added_a_new_required_property() + self.check_if_value_of_type_field_changed() + # self.check_if_new_type_was_added() We want to allow type expansion atm + self.check_if_type_of_type_field_changed() + self.check_if_field_was_made_not_nullable() + self.check_if_enum_was_narrowed() + self.check_if_declared_new_enum_field() + + def _raise_error(self, message: str): + raise NonBackwardCompatibleSpecError(f"{message}: {self._diff.pretty()}") + + def check_if_declared_new_required_field(self): + """Check if the new spec declared a 'required' field.""" + added_required_fields = [ + addition for addition in self._diff.get("dictionary_item_added", []) if addition.path(output_format="list")[-1] == "required" + ] + if added_required_fields: + self._raise_error("The current spec declared a new 'required' field") + + def check_if_added_a_new_required_property(self): + """Check if the new spec added a property to the 'required' list.""" + added_required_properties = [ + addition for addition in self._diff.get("iterable_item_added", []) if addition.up.path(output_format="list")[-1] == "required" + ] + if added_required_properties: + self._raise_error("A new property was added to 'required'") + + def check_if_value_of_type_field_changed(self): + """Check if a type was changed""" + # Detect type value change in case type field is declared as a string (e.g "str" -> "int"): + type_values_changed = [change for change in self._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 self._diff.get("values_changed", []) if change.path(output_format="list")[-2] == "type" + ] + if type_values_changed or type_values_changed_in_list: + self._raise_error("The current spec changed the value of a 'type' field") + + def check_if_new_type_was_added(self): + """Detect type value added to type list if new type value is not None (e.g ["str"] -> ["str", "int"])""" + new_values_in_type_list = [ + change + for change in self._diff.get("iterable_item_added", []) + if change.path(output_format="list")[-2] == "type" + if change.t2 != "null" + ] + if new_values_in_type_list: + self._raise_error("The current spec changed the value of a 'type' field") + + def check_if_type_of_type_field_changed(self): + """ + 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 self._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): + if not isinstance(change.t2, list): + self._raise_error("The current spec change a type field from string to an invalid value.") + if not 0 < len(change.t2) <= 2: + self._raise_error( + "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." + ) + # 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"] + if not (len(t2_not_null_types) == 1 and t2_not_null_types[0] == change.t1): + self._raise_error("The current spec change a type field to a list with multiple invalid values.") + if isinstance(change.t1, list): + if not isinstance(change.t2, str): + self._raise_error("The current spec change a type field from list to an invalid value.") + if not (len(change.t1) == 1 and change.t2 == change.t1[0]): + self._raise_error("The current spec narrowed a field type.") + + def check_if_field_was_made_not_nullable(self): + """Detect when field was made not nullable but is still a list: e.g ["string", "null"] -> ["string"]""" + removed_nullable = [ + change for change in self._diff.get("iterable_item_removed", []) if change.path(output_format="list")[-2] == "type" + ] + if removed_nullable: + self._raise_error("The current spec narrowed a field type or made a field not nullable.") + + def check_if_enum_was_narrowed(self): + """Check if the list of values in a enum was shortened in a spec.""" + enum_removals = [ + enum_removal + for enum_removal in self._diff.get("iterable_item_removed", []) + if enum_removal.up.path(output_format="list")[-1] == "enum" + ] + if enum_removals: + self._raise_error("The current spec narrowed an enum field.") + + def check_if_declared_new_enum_field(self): + """Check if an 'enum' field was added to the spec.""" + enum_additions = [ + enum_addition + for enum_addition in self._diff.get("dictionary_item_added", []) + if enum_addition.path(output_format="list")[-1] == "enum" + ] + if enum_additions: + self._raise_error("An 'enum' field was declared on an existing property of the spec.") + + +def validate_previous_configs( + previous_connector_spec: ConnectorSpecification, actual_connector_spec: ConnectorSpecification, number_of_configs_to_generate=100 +): + """Use hypothesis and hypothesis-jsonschema to run property based testing: + 1. Generate fake previous config with the previous connector specification json schema. + 2. Validate a fake previous config against the actual connector specification json schema.""" + + @given(from_schema(previous_connector_spec.dict()["connectionSpecification"])) + @settings(max_examples=number_of_configs_to_generate) + def check_fake_previous_config_against_actual_spec(fake_previous_config): + fake_previous_config = SecretDict(fake_previous_config) + filtered_fake_previous_config = {key: value for key, value in fake_previous_config.data.items() if not key.startswith("_")} + try: + jsonschema.validate(instance=filtered_fake_previous_config, schema=actual_connector_spec.connectionSpecification) + except jsonschema.exceptions.ValidationError as err: + raise NonBackwardCompatibleSpecError(err) + + check_fake_previous_config_against_actual_spec() 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 index e97ee150c2472..85e7e03abb6c8 100644 --- 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 @@ -2,976 +2,864 @@ # Copyright (c) 2022 Airbyte, Inc., all rights reserved. # +from dataclasses import dataclass + import pytest from airbyte_cdk.models import ConnectorSpecification from source_acceptance_test.tests.test_core import TestSpec as _TestSpec +from source_acceptance_test.utils.backward_compatibility import NonBackwardCompatibleSpecError 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) +@dataclass +class Transition: + """An helper class to improve readability of the test cases""" + previous_connector_specification: ConnectorSpecification + current_connector_specification: ConnectorSpecification + should_fail: bool + name: str -@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) + def as_pytest_param(self): + return pytest.param(self.previous_connector_specification, self.current_connector_specification, self.should_fail, id=self.name) -@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) +FAILING_TRANSITIONS = [ + Transition( + ConnectorSpecification(connectionSpecification={}), + ConnectorSpecification( + connectionSpecification={ + "required": ["a", "b"], + } + ), + should_fail=True, + name="Top level: declaring the required field should fail.", + ), + Transition( + 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"}}, + } + }, + } + ), + should_fail=True, + name="Nested level: adding the required field should fail.", + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "required": ["a"], + } + ), + ConnectorSpecification( + connectionSpecification={ + "required": ["a", "b"], + } + ), + name="Top level: adding a new required property should fail.", + should_fail=True, + ), + Transition( + 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"}, + }, + } + }, + } + ), + name="Nested level: adding a new required property should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": ["null", "string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string"}, + }, + } + ), + name="Nullable: Making a field not nullable should fail (not in a list).", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["null", "integer"]}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["integer"]}}}, + }, + } + ), + name="Nested level: Narrowing a field type should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + name="Nullable field: Making a field not nullable should fail", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "integer"}, + }, + } + ), + name="Changing a field type should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["integer"]}, + }, + } + ), + name="Changing a field type from a string to a list with a different type value should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "integer"}, + }, + } + ), + name="Changing a field type should fail from a list to string with different value should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["integer"]}, + }, + } + ), + name="Changing a field type in list should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["integer", "null"]}, + }, + } + ), + name="Making a field nullable and changing type should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "integer"]}, + }, + } + ), + name="Making a field nullable and changing type should fail (change list order).", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "integer"]}, + }, + } + ), + name="Nullable field: Changing a field type should fail", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": "string"}, + {"title": "b", "type": "integer"}, + ] + }, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": "integer"}, + {"title": "b", "type": "integer"}, + ] + }, + }, + } + ), + name="Changing a field type in oneOf should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": ["string", "null"]}, + {"title": "b", "type": "integer"}, + ] + }, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": ["string"]}, + {"title": "b", "type": "integer"}, + ] + }, + }, + } + ), + name="Narrowing a field type in oneOf should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a"]}, + }, + } + ), + name="Top level: Narrowing a field enum should fail.", + should_fail=True, + ), + Transition( + 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"]}}}, + }, + } + ), + name="Nested level: Narrowing a field enum should fail.", + should_fail=True, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + name="Top level: Declaring a field enum should fail.", + should_fail=True, + ), + Transition( + 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"]}}}, + }, + } + ), + name="Nested level: Declaring a field enum should fail.", + should_fail=True, + ), +] +VALID_TRANSITIONS = [ + Transition( + 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"}, + }, + } + ), + name="Not changing a spec should not fail", + should_fail=False, + ), + Transition( + 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": {"type": "string"}, + }, + } + ), + name="Adding an optional field should not fail.", + should_fail=False, + ), + Transition( + 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"}}, + }, + }, + } + ), + name="Adding an optional object with required properties should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + name="No change should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + name="Changing a field type from a list to a string with same value should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + name="Changing a field type from a string to a list should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string", "integer"]}, + }, + } + ), + name="Adding a field type in list should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "string"]}, + }, + } + ), + name="Making a field nullable should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string", "null"]}, + }, + } + ), + name="Making a field nullable should not fail (change list order).", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "string"]}, + }, + } + ), + name="Making a field nullable should not fail (from a list).", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string", "null"]}, + }, + } + ), + name="Making a field nullable should not fail (from a list, changing order).", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["null", "string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_int": {"type": ["string", "null"]}, + }, + } + ), + name="Nullable field: Changing order should not fail", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["integer"]}}}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_nested_object": {"type": "object", "properties": {"my_property": {"type": ["null", "integer"]}}}, + }, + } + ), + name="Nested level: Expanding a field type should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "a", "type": "string"}, + {"title": "b", "type": "integer"}, + ] + }, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "oneOf": [ + {"title": "b", "type": "string"}, + {"title": "a", "type": "integer"}, + ] + }, + }, + } + ), + name="Changing a order in oneOf should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + name="Top level: Expanding a field enum should not fail.", + should_fail=False, + ), + Transition( + 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"]}}}, + }, + } + ), + name="Nested level: Expanding a field enum should not fail.", + should_fail=False, + ), + Transition( + 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"]}}, + } + ), + name="Top level: Adding a new optional field with enum should not fail.", + should_fail=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string", "enum": ["a", "b"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string"}, + }, + } + ), + name="Top level: Removing the field enum should not fail.", + should_fail=False, + ), + Transition( + 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"}}}, + }, + } + ), + name="Nested level: Removing the enum field should not fail.", + should_fail=False, + ), +] -@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) +# Checking that all transition in FAILING_TRANSITIONS have should_fail == True to prevent typos +assert all([transition.should_fail for transition in FAILING_TRANSITIONS]) +# Checking that all transition in VALID_TRANSITIONS have should_fail = False to prevent typos +assert not all([transition.should_fail for transition in VALID_TRANSITIONS]) -@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) +ALL_TRANSITIONS_PARAMS = [transition.as_pytest_param() for transition in FAILING_TRANSITIONS + VALID_TRANSITIONS] -@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): +@pytest.mark.parametrize("previous_connector_spec, actual_connector_spec, should_fail", ALL_TRANSITIONS_PARAMS) +def test_backward_compatibility(previous_connector_spec, actual_connector_spec, should_fail): t = _TestSpec() - spec_diff = t.compute_spec_diff(actual_connector_spec, previous_connector_spec) + expectation = pytest.raises(NonBackwardCompatibleSpecError) if should_fail else does_not_raise() with expectation: - t.test_new_enum_field_declaration(spec_diff) + t.test_backward_compatibility(False, actual_connector_spec, previous_connector_spec) 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 ea8fe2d57dc9d..3a09ce07f887f 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,6 +8,8 @@ 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", @@ -567,3 +569,39 @@ 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) From 5c17068559b69ca974089c498095ed50ceac51ac Mon Sep 17 00:00:00 2001 From: alafanechere Date: Fri, 5 Aug 2022 17:18:24 +0200 Subject: [PATCH 02/14] clean --- .../bases/source-acceptance-test/CHANGELOG.md | 2 +- .../source_acceptance_test/tests/test_core.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index 5295716433622..b49ca430ec946 100644 --- a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md +++ b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog ## 0.1.60 -Backward compatibility tests: validate fake previous config against current connector specification. +Backward compatibility tests: validate fake previous config against current connector specification. [#15367](https://github.com/airbytehq/airbyte/pull/15367) ## 0.1.59 Backward compatibility tests: add syntactic validation of specs [#15194](https://github.com/airbytehq/airbyte/pull/15194/). 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 54bdd2c1b568c..587dc68b7e3e0 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 @@ -196,8 +196,8 @@ def test_backward_compatibility( previous_connector_spec: ConnectorSpecification, ): """Check if the current spec is backward_compatible: - 1. Perform multiple hardcoded syntactic checks with SpecDiffChecker - 2. validate fake generated previous configs against the actual connector specification with validate_previous_configs + 1. Perform multiple hardcoded syntactic checks with SpecDiffChecker. + 2. Validate fake generated previous configs against the actual connector specification with validate_previous_configs. """ assert isinstance(actual_connector_spec, ConnectorSpecification) and isinstance(previous_connector_spec, ConnectorSpecification) spec_diff = self.compute_spec_diff(actual_connector_spec, previous_connector_spec) From b8be9121cb94f04a64fabe35f96b041987dd1315 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 8 Aug 2022 09:31:44 +0200 Subject: [PATCH 03/14] rename Transition to SpecTransition --- .../unit_tests/test_backward_compatibility.py | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) 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 index 85e7e03abb6c8..1cd133f620e32 100644 --- 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 @@ -13,7 +13,7 @@ @dataclass -class Transition: +class SpecTransition: """An helper class to improve readability of the test cases""" previous_connector_specification: ConnectorSpecification @@ -26,7 +26,7 @@ def as_pytest_param(self): FAILING_TRANSITIONS = [ - Transition( + SpecTransition( ConnectorSpecification(connectionSpecification={}), ConnectorSpecification( connectionSpecification={ @@ -36,7 +36,7 @@ def as_pytest_param(self): should_fail=True, name="Top level: declaring the required field should fail.", ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -58,7 +58,7 @@ def as_pytest_param(self): should_fail=True, name="Nested level: adding the required field should fail.", ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "required": ["a"], @@ -72,7 +72,7 @@ def as_pytest_param(self): name="Top level: adding a new required property should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -103,7 +103,7 @@ def as_pytest_param(self): name="Nested level: adding a new required property should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -123,7 +123,7 @@ def as_pytest_param(self): name="Nullable: Making a field not nullable should fail (not in a list).", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -143,7 +143,7 @@ def as_pytest_param(self): name="Nested level: Narrowing a field type should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -163,7 +163,7 @@ def as_pytest_param(self): name="Nullable field: Making a field not nullable should fail", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -183,7 +183,7 @@ def as_pytest_param(self): name="Changing a field type should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -203,7 +203,7 @@ def as_pytest_param(self): name="Changing a field type from a string to a list with a different type value should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -223,7 +223,7 @@ def as_pytest_param(self): name="Changing a field type should fail from a list to string with different value should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -243,7 +243,7 @@ def as_pytest_param(self): name="Changing a field type in list should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -263,7 +263,7 @@ def as_pytest_param(self): name="Making a field nullable and changing type should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -283,7 +283,7 @@ def as_pytest_param(self): name="Making a field nullable and changing type should fail (change list order).", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -303,7 +303,7 @@ def as_pytest_param(self): name="Nullable field: Changing a field type should fail", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -333,7 +333,7 @@ def as_pytest_param(self): name="Changing a field type in oneOf should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -363,7 +363,7 @@ def as_pytest_param(self): name="Narrowing a field type in oneOf should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -383,7 +383,7 @@ def as_pytest_param(self): name="Top level: Narrowing a field enum should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -403,7 +403,7 @@ def as_pytest_param(self): name="Nested level: Narrowing a field enum should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -423,7 +423,7 @@ def as_pytest_param(self): name="Top level: Declaring a field enum should fail.", should_fail=True, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -446,7 +446,7 @@ def as_pytest_param(self): ] VALID_TRANSITIONS = [ - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -468,7 +468,7 @@ def as_pytest_param(self): name="Not changing a spec should not fail", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -491,7 +491,7 @@ def as_pytest_param(self): name="Adding an optional field should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -518,7 +518,7 @@ def as_pytest_param(self): name="Adding an optional object with required properties should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -538,7 +538,7 @@ def as_pytest_param(self): name="No change should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -558,7 +558,7 @@ def as_pytest_param(self): name="Changing a field type from a list to a string with same value should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -578,7 +578,7 @@ def as_pytest_param(self): name="Changing a field type from a string to a list should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -598,7 +598,7 @@ def as_pytest_param(self): name="Adding a field type in list should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -618,7 +618,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -638,7 +638,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail (change list order).", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -658,7 +658,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail (from a list).", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -678,7 +678,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail (from a list, changing order).", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -698,7 +698,7 @@ def as_pytest_param(self): name="Nullable field: Changing order should not fail", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -718,7 +718,7 @@ def as_pytest_param(self): name="Nested level: Expanding a field type should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -748,7 +748,7 @@ def as_pytest_param(self): name="Changing a order in oneOf should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -768,7 +768,7 @@ def as_pytest_param(self): name="Top level: Expanding a field enum should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -788,7 +788,7 @@ def as_pytest_param(self): name="Nested level: Expanding a field enum should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -806,7 +806,7 @@ def as_pytest_param(self): name="Top level: Adding a new optional field with enum should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -826,7 +826,7 @@ def as_pytest_param(self): name="Top level: Removing the field enum should not fail.", should_fail=False, ), - Transition( + SpecTransition( ConnectorSpecification( connectionSpecification={ "type": "object", From 62e549dc4b82cb99444f3a3ee856d80b048bc696 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 8 Aug 2022 09:45:45 +0200 Subject: [PATCH 04/14] rename Transition to SpecTransition --- .../unit_tests/test_backward_compatibility.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 index 1cd133f620e32..4beac9b27dfc5 100644 --- 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 @@ -25,7 +25,7 @@ def as_pytest_param(self): return pytest.param(self.previous_connector_specification, self.current_connector_specification, self.should_fail, id=self.name) -FAILING_TRANSITIONS = [ +FAILING_SPEC_TRANSITIONS = [ SpecTransition( ConnectorSpecification(connectionSpecification={}), ConnectorSpecification( @@ -445,7 +445,7 @@ def as_pytest_param(self): ), ] -VALID_TRANSITIONS = [ +VALID_SPEC_TRANSITIONS = [ SpecTransition( ConnectorSpecification( connectionSpecification={ @@ -848,16 +848,16 @@ def as_pytest_param(self): ), ] -# Checking that all transition in FAILING_TRANSITIONS have should_fail == True to prevent typos -assert all([transition.should_fail for transition in FAILING_TRANSITIONS]) -# Checking that all transition in VALID_TRANSITIONS have should_fail = False to prevent typos -assert not all([transition.should_fail for transition in VALID_TRANSITIONS]) +# Checking that all transition in FAILING_SPEC_TRANSITIONS have should_fail == True to prevent typos +assert all([transition.should_fail for transition in FAILING_SPEC_TRANSITIONS]) +# Checking that all transition in VALID_SPEC_TRANSITIONS have should_fail = False to prevent typos +assert not all([transition.should_fail for transition in VALID_SPEC_TRANSITIONS]) -ALL_TRANSITIONS_PARAMS = [transition.as_pytest_param() for transition in FAILING_TRANSITIONS + VALID_TRANSITIONS] +ALL_SPEC_TRANSITIONS_PARAMS = [transition.as_pytest_param() for transition in FAILING_SPEC_TRANSITIONS + VALID_SPEC_TRANSITIONS] -@pytest.mark.parametrize("previous_connector_spec, actual_connector_spec, should_fail", ALL_TRANSITIONS_PARAMS) +@pytest.mark.parametrize("previous_connector_spec, actual_connector_spec, should_fail", ALL_SPEC_TRANSITIONS_PARAMS) def test_backward_compatibility(previous_connector_spec, actual_connector_spec, should_fail): t = _TestSpec() expectation = pytest.raises(NonBackwardCompatibleSpecError) if should_fail else does_not_raise() From 81cc9e7796a6dbc372e1b66d9093db0feaef6c75 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 8 Aug 2022 09:46:07 +0200 Subject: [PATCH 05/14] clean --- .../unit_tests/test_backward_compatibility.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 4beac9b27dfc5..d0929debdfcde 100644 --- 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 @@ -848,9 +848,9 @@ def as_pytest_param(self): ), ] -# Checking that all transition in FAILING_SPEC_TRANSITIONS have should_fail == True to prevent typos +# Checking that all transitions in FAILING_SPEC_TRANSITIONS have should_fail == True to prevent typos assert all([transition.should_fail for transition in FAILING_SPEC_TRANSITIONS]) -# Checking that all transition in VALID_SPEC_TRANSITIONS have should_fail = False to prevent typos +# Checking that all transitions in VALID_SPEC_TRANSITIONS have should_fail = False to prevent typos assert not all([transition.should_fail for transition in VALID_SPEC_TRANSITIONS]) From f42aa452d811788f137ba6e07a52637526df32ed Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 8 Aug 2022 09:56:32 +0200 Subject: [PATCH 06/14] clean spec_diff --- .../source_acceptance_test/tests/test_core.py | 11 ----------- 1 file changed, 11 deletions(-) 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 587dc68b7e3e0..09e91e67cc8f3 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 @@ -176,22 +176,11 @@ 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}" - @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 - @pytest.mark.default_timeout(60) @pytest.mark.backward_compatibility def test_backward_compatibility( self, skip_backward_compatibility_tests: bool, - inputs, actual_connector_spec: ConnectorSpecification, previous_connector_spec: ConnectorSpecification, ): From 0cac90b39a006c1fb6a44c1ee99de8d145d89138 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 8 Aug 2022 15:04:53 +0200 Subject: [PATCH 07/14] Implement backward compatibility tests on catalogs. --- .../source_acceptance_test/config.py | 3 + .../source_acceptance_test/conftest.py | 19 ++ .../source_acceptance_test/tests/test_core.py | 42 ++- .../utils/backward_compatibility.py | 115 +++++--- .../unit_tests/test_backward_compatibility.py | 250 ++++++++++++++---- 5 files changed, 342 insertions(+), 87 deletions(-) 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 dfcbc8ba33dbc..1e1e6f8ee4da0 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 @@ -57,6 +57,9 @@ class Status(Enum): class DiscoveryTestConfig(BaseConfig): config_path: str = config_path timeout_seconds: int = timeout_seconds + backward_compatibility_tests_config: BackwardCompatibilityTestsConfig = Field( + description="Configuration for the backward compatibility tests.", default=BackwardCompatibilityTestsConfig() + ) class ExpectedRecordsConfig(BaseModel): 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 301267c0329a7..8c71b02db7fba 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 @@ -178,6 +178,12 @@ def cached_schemas_fixture() -> MutableMapping[str, AirbyteStream]: return {} +@pytest.fixture(name="previous_cached_schemas", scope="session") +def previous_cached_schemas_fixture() -> MutableMapping[str, AirbyteStream]: + """Simple cache for discovered catalog of previous connector: stream_name -> json_schema""" + return {} + + @pytest.fixture(name="discovered_catalog") def discovered_catalog_fixture(connector_config, docker_runner: ConnectorRunner, cached_schemas) -> MutableMapping[str, AirbyteStream]: """JSON schemas for each stream""" @@ -190,6 +196,19 @@ def discovered_catalog_fixture(connector_config, docker_runner: ConnectorRunner, return cached_schemas +@pytest.fixture(name="previous_discovered_catalog") +def previous_discovered_catalog_fixture( + connector_config, previous_connector_docker_runner: ConnectorRunner, previous_cached_schemas +) -> MutableMapping[str, AirbyteStream]: + """JSON schemas for each stream""" + if not previous_cached_schemas: + output = previous_connector_docker_runner.call_discover(config=connector_config) + catalogs = [message.catalog for message in output if message.type == Type.CATALOG] + for stream in catalogs[-1].streams: + previous_cached_schemas[stream.name] = stream + return previous_cached_schemas + + @pytest.fixture def detailed_logger() -> Logger: """ 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 09e91e67cc8f3..347452aa0945d 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 @@ -30,7 +30,7 @@ from source_acceptance_test.base import BaseTest from source_acceptance_test.config import BasicReadTestConfig, ConnectionTestConfig, SpecTestConfig from source_acceptance_test.utils import ConnectorRunner, SecretDict, filter_output, make_hashable, verify_records_schema -from source_acceptance_test.utils.backward_compatibility import SpecDiffChecker, validate_previous_configs +from source_acceptance_test.utils.backward_compatibility import CatalogDiffChecker, SpecDiffChecker, validate_previous_configs from source_acceptance_test.utils.common import find_all_values_for_key_in_schema, find_keyword_schema from source_acceptance_test.utils.json_schema_helper import JsonSchemaHelper, get_expected_schema_structure, get_object_structure @@ -234,6 +234,29 @@ def test_check(self, connector_config, inputs: ConnectionTestConfig, docker_runn @pytest.mark.default_timeout(30) class TestDiscovery(BaseTest): + @staticmethod + def compute_discovered_catalog_diff( + discovered_catalog: MutableMapping[str, AirbyteStream], previous_discovered_catalog: MutableMapping[str, AirbyteStream] + ): + return DeepDiff( + {stream_name: airbyte_stream.dict().pop("json_schema") for stream_name, airbyte_stream in previous_discovered_catalog.items()}, + {stream_name: airbyte_stream.dict().pop("json_schema") for stream_name, airbyte_stream in discovered_catalog.items()}, + 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 + def test_discover(self, connector_config, docker_runner: ConnectorRunner): """Verify that discover produce correct schema.""" output = docker_runner.call_discover(config=connector_config) @@ -306,6 +329,23 @@ def test_additional_properties_is_true(self, discovered_catalog: Mapping[str, An [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) + @pytest.mark.backward_compatibility + def test_backward_compatibility( + self, + skip_backward_compatibility_tests: bool, + discovered_catalog: MutableMapping[str, AirbyteStream], + previous_discovered_catalog: MutableMapping[str, AirbyteStream], + ): + """Check if the current spec is backward_compatible: + 1. Perform multiple hardcoded syntactic checks with SpecDiffChecker. + 2. Validate fake generated previous configs against the actual connector specification with validate_previous_configs. + """ + assert isinstance(discovered_catalog, MutableMapping) and isinstance(previous_discovered_catalog, MutableMapping) + catalog_diff = self.compute_discovered_catalog_diff(discovered_catalog, previous_discovered_catalog) + checker = CatalogDiffChecker(catalog_diff) + checker.check() + def primary_keys_for_records(streams, records): streams_with_primary_key = [stream for stream in streams if stream.stream.source_defined_primary_key] diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py index 2c947147609c7..3198c419a96bf 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py @@ -1,6 +1,10 @@ # # Copyright (c) 2022 Airbyte, Inc., all rights reserved. # + +from abc import ABC, abstractmethod +from multiprocessing import context + import jsonschema from airbyte_cdk.models import ConnectorSpecification from deepdiff import DeepDiff @@ -9,44 +13,25 @@ from source_acceptance_test.utils import SecretDict -class NonBackwardCompatibleSpecError(Exception): +class NonBackwardCompatibleError(Exception): pass -class SpecDiffChecker: - """A class to perform multiple backward compatible checks on a spec diff""" - +class BaseDiffChecker(ABC): def __init__(self, diff: DeepDiff) -> None: self._diff = diff - def check(self): - self.check_if_declared_new_required_field() - self.check_if_added_a_new_required_property() - self.check_if_value_of_type_field_changed() - # self.check_if_new_type_was_added() We want to allow type expansion atm - self.check_if_type_of_type_field_changed() - self.check_if_field_was_made_not_nullable() - self.check_if_enum_was_narrowed() - self.check_if_declared_new_enum_field() - def _raise_error(self, message: str): - raise NonBackwardCompatibleSpecError(f"{message}: {self._diff.pretty()}") + raise NonBackwardCompatibleError(f"{context} - {message}: {self._diff.pretty()}") - def check_if_declared_new_required_field(self): - """Check if the new spec declared a 'required' field.""" - added_required_fields = [ - addition for addition in self._diff.get("dictionary_item_added", []) if addition.path(output_format="list")[-1] == "required" - ] - if added_required_fields: - self._raise_error("The current spec declared a new 'required' field") + @property + @abstractmethod + def context(self): + pass - def check_if_added_a_new_required_property(self): - """Check if the new spec added a property to the 'required' list.""" - added_required_properties = [ - addition for addition in self._diff.get("iterable_item_added", []) if addition.up.path(output_format="list")[-1] == "required" - ] - if added_required_properties: - self._raise_error("A new property was added to 'required'") + @abstractmethod + def check(self): + pass def check_if_value_of_type_field_changed(self): """Check if a type was changed""" @@ -58,7 +43,7 @@ def check_if_value_of_type_field_changed(self): change for change in self._diff.get("values_changed", []) if change.path(output_format="list")[-2] == "type" ] if type_values_changed or type_values_changed_in_list: - self._raise_error("The current spec changed the value of a 'type' field") + self._raise_error("The value of a 'type' field was changed.") def check_if_new_type_was_added(self): """Detect type value added to type list if new type value is not None (e.g ["str"] -> ["str", "int"])""" @@ -69,7 +54,7 @@ def check_if_new_type_was_added(self): if change.t2 != "null" ] if new_values_in_type_list: - self._raise_error("The current spec changed the value of a 'type' field") + self._raise_error("A new value was added to a 'type' field.") def check_if_type_of_type_field_changed(self): """ @@ -89,21 +74,53 @@ def check_if_type_of_type_field_changed(self): # This might be something already guaranteed by JSON schema validation. if isinstance(change.t1, str): if not isinstance(change.t2, list): - self._raise_error("The current spec change a type field from string to an invalid value.") + self._raise_error("The current {context} change a type field from string to an invalid value.") if not 0 < len(change.t2) <= 2: self._raise_error( - "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." + "A type field changed from string to an invalid value. The type list should not be empty and have a maximum of two items." ) # 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"] if not (len(t2_not_null_types) == 1 and t2_not_null_types[0] == change.t1): - self._raise_error("The current spec change a type field to a list with multiple invalid values.") + self._raise_error("The type field changed to a list with multiple invalid values.") if isinstance(change.t1, list): if not isinstance(change.t2, str): - self._raise_error("The current spec change a type field from list to an invalid value.") + self._raise_error("The type field changed from a list to an invalid value.") if not (len(change.t1) == 1 and change.t2 == change.t1[0]): - self._raise_error("The current spec narrowed a field type.") + self._raise_error("An element was removed from the list of valid types.") + + +class SpecDiffChecker(BaseDiffChecker): + """A class to perform backward compatibility checks on a connector specification diff""" + + context = "Specification" + + def check(self): + self.check_if_declared_new_required_field() + self.check_if_added_a_new_required_property() + self.check_if_value_of_type_field_changed() + # self.check_if_new_type_was_added() We want to allow type expansion atm + self.check_if_type_of_type_field_changed() + self.check_if_field_was_made_not_nullable() + self.check_if_enum_was_narrowed() + self.check_if_declared_new_enum_field() + + def check_if_declared_new_required_field(self): + """Check if the new spec declared a 'required' field.""" + added_required_fields = [ + addition for addition in self._diff.get("dictionary_item_added", []) if addition.path(output_format="list")[-1] == "required" + ] + if added_required_fields: + self._raise_error(f"The current {context} declared a new 'required' field") + + def check_if_added_a_new_required_property(self): + """Check if the new spec added a property to the 'required' list.""" + added_required_properties = [ + addition for addition in self._diff.get("iterable_item_added", []) if addition.up.path(output_format="list")[-1] == "required" + ] + if added_required_properties: + self._raise_error("A new property was added to 'required'") def check_if_field_was_made_not_nullable(self): """Detect when field was made not nullable but is still a list: e.g ["string", "null"] -> ["string"]""" @@ -111,7 +128,7 @@ def check_if_field_was_made_not_nullable(self): change for change in self._diff.get("iterable_item_removed", []) if change.path(output_format="list")[-2] == "type" ] if removed_nullable: - self._raise_error("The current spec narrowed a field type or made a field not nullable.") + self._raise_error("A field type was narrowed or made a field not nullable.") def check_if_enum_was_narrowed(self): """Check if the list of values in a enum was shortened in a spec.""" @@ -121,7 +138,7 @@ def check_if_enum_was_narrowed(self): if enum_removal.up.path(output_format="list")[-1] == "enum" ] if enum_removals: - self._raise_error("The current spec narrowed an enum field.") + self._raise_error("An enum field was narrowed.") def check_if_declared_new_enum_field(self): """Check if an 'enum' field was added to the spec.""" @@ -149,6 +166,26 @@ def check_fake_previous_config_against_actual_spec(fake_previous_config): try: jsonschema.validate(instance=filtered_fake_previous_config, schema=actual_connector_spec.connectionSpecification) except jsonschema.exceptions.ValidationError as err: - raise NonBackwardCompatibleSpecError(err) + raise NonBackwardCompatibleError(err) check_fake_previous_config_against_actual_spec() + + +class CatalogDiffChecker(BaseDiffChecker): + """A class to perform backward compatibility checks on a discoverd catalog diff""" + + context = "Catalog" + + def check(self): + self.check_if_stream_was_removed() + self.check_if_value_of_type_field_changed() + self.check_if_type_of_type_field_changed() + + def check_if_stream_was_removed(self): + """Check if a stream was removed from the catalog.""" + removed_streams = [] + for removal in self._diff.get("dictionary_item_removed", []): + if removal.path() != "root" and removal.up.path() == "root": + removed_streams.append(removal.path(output_format="list")[0]) + if removed_streams: + self._raise_error(f"The following streams were removed: {','.join(removed_streams)}") 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 index d0929debdfcde..bffd0bd92ccb1 100644 --- 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 @@ -3,30 +3,32 @@ # from dataclasses import dataclass +from typing import MutableMapping, Union import pytest -from airbyte_cdk.models import ConnectorSpecification +from airbyte_cdk.models import AirbyteStream, ConnectorSpecification +from source_acceptance_test.tests.test_core import TestDiscovery as _TestDiscovery from source_acceptance_test.tests.test_core import TestSpec as _TestSpec -from source_acceptance_test.utils.backward_compatibility import NonBackwardCompatibleSpecError +from source_acceptance_test.utils.backward_compatibility import NonBackwardCompatibleError from .conftest import does_not_raise @dataclass -class SpecTransition: +class Transition: """An helper class to improve readability of the test cases""" - previous_connector_specification: ConnectorSpecification - current_connector_specification: ConnectorSpecification + previous: Union[ConnectorSpecification, MutableMapping[str, AirbyteStream]] + current: Union[ConnectorSpecification, MutableMapping[str, AirbyteStream]] should_fail: bool name: str def as_pytest_param(self): - return pytest.param(self.previous_connector_specification, self.current_connector_specification, self.should_fail, id=self.name) + return pytest.param(self.previous, self.current, self.should_fail, id=self.name) FAILING_SPEC_TRANSITIONS = [ - SpecTransition( + Transition( ConnectorSpecification(connectionSpecification={}), ConnectorSpecification( connectionSpecification={ @@ -36,7 +38,7 @@ def as_pytest_param(self): should_fail=True, name="Top level: declaring the required field should fail.", ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -58,7 +60,7 @@ def as_pytest_param(self): should_fail=True, name="Nested level: adding the required field should fail.", ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "required": ["a"], @@ -72,7 +74,7 @@ def as_pytest_param(self): name="Top level: adding a new required property should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -103,7 +105,7 @@ def as_pytest_param(self): name="Nested level: adding a new required property should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -123,7 +125,7 @@ def as_pytest_param(self): name="Nullable: Making a field not nullable should fail (not in a list).", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -143,7 +145,7 @@ def as_pytest_param(self): name="Nested level: Narrowing a field type should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -163,7 +165,7 @@ def as_pytest_param(self): name="Nullable field: Making a field not nullable should fail", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -183,7 +185,7 @@ def as_pytest_param(self): name="Changing a field type should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -203,7 +205,7 @@ def as_pytest_param(self): name="Changing a field type from a string to a list with a different type value should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -223,7 +225,7 @@ def as_pytest_param(self): name="Changing a field type should fail from a list to string with different value should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -243,7 +245,7 @@ def as_pytest_param(self): name="Changing a field type in list should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -263,7 +265,7 @@ def as_pytest_param(self): name="Making a field nullable and changing type should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -283,7 +285,7 @@ def as_pytest_param(self): name="Making a field nullable and changing type should fail (change list order).", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -303,7 +305,7 @@ def as_pytest_param(self): name="Nullable field: Changing a field type should fail", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -333,7 +335,7 @@ def as_pytest_param(self): name="Changing a field type in oneOf should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -363,7 +365,7 @@ def as_pytest_param(self): name="Narrowing a field type in oneOf should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -383,7 +385,7 @@ def as_pytest_param(self): name="Top level: Narrowing a field enum should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -403,7 +405,7 @@ def as_pytest_param(self): name="Nested level: Narrowing a field enum should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -423,7 +425,7 @@ def as_pytest_param(self): name="Top level: Declaring a field enum should fail.", should_fail=True, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -446,7 +448,7 @@ def as_pytest_param(self): ] VALID_SPEC_TRANSITIONS = [ - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -468,7 +470,7 @@ def as_pytest_param(self): name="Not changing a spec should not fail", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -491,7 +493,7 @@ def as_pytest_param(self): name="Adding an optional field should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -518,7 +520,7 @@ def as_pytest_param(self): name="Adding an optional object with required properties should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -538,7 +540,7 @@ def as_pytest_param(self): name="No change should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -558,7 +560,7 @@ def as_pytest_param(self): name="Changing a field type from a list to a string with same value should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -578,7 +580,7 @@ def as_pytest_param(self): name="Changing a field type from a string to a list should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -598,7 +600,7 @@ def as_pytest_param(self): name="Adding a field type in list should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -618,7 +620,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -638,7 +640,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail (change list order).", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -658,7 +660,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail (from a list).", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -678,7 +680,7 @@ def as_pytest_param(self): name="Making a field nullable should not fail (from a list, changing order).", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -698,7 +700,7 @@ def as_pytest_param(self): name="Nullable field: Changing order should not fail", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -718,7 +720,7 @@ def as_pytest_param(self): name="Nested level: Expanding a field type should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -748,7 +750,7 @@ def as_pytest_param(self): name="Changing a order in oneOf should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -768,7 +770,7 @@ def as_pytest_param(self): name="Top level: Expanding a field enum should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -788,7 +790,7 @@ def as_pytest_param(self): name="Nested level: Expanding a field enum should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -806,7 +808,7 @@ def as_pytest_param(self): name="Top level: Adding a new optional field with enum should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -826,7 +828,7 @@ def as_pytest_param(self): name="Top level: Removing the field enum should not fail.", should_fail=False, ), - SpecTransition( + Transition( ConnectorSpecification( connectionSpecification={ "type": "object", @@ -858,8 +860,162 @@ def as_pytest_param(self): @pytest.mark.parametrize("previous_connector_spec, actual_connector_spec, should_fail", ALL_SPEC_TRANSITIONS_PARAMS) -def test_backward_compatibility(previous_connector_spec, actual_connector_spec, should_fail): +def test_spec_backward_compatibility(previous_connector_spec, actual_connector_spec, should_fail): t = _TestSpec() - expectation = pytest.raises(NonBackwardCompatibleSpecError) if should_fail else does_not_raise() + expectation = pytest.raises(NonBackwardCompatibleError) if should_fail else does_not_raise() with expectation: t.test_backward_compatibility(False, actual_connector_spec, previous_connector_spec) + + +FAILING_CATALOG_TRANSITIONS = [ + # Transition( + # name="Removing a stream from a catalog should fail.", + # should_fail=True, + # previous={ + # "test_stream": AirbyteStream.parse_obj( + # { + # "name": "test_stream", + # "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + # } + # ), + # "other_test_stream": AirbyteStream.parse_obj( + # { + # "name": "other_test_stream", + # "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + # } + # ) + # }, + # current={ + # "test_stream": AirbyteStream.parse_obj( + # { + # "name": "test_stream", + # "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + # } + # ) + # } + # ), + # Transition( + # name="Changing a field type should fail.", + # should_fail=True, + # previous={ + # "test_stream": AirbyteStream.parse_obj( + # { + # "name": "test_stream", + # "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + # } + # ) + # }, + # current={ + # "test_stream": AirbyteStream.parse_obj( + # { + # "name": "test_stream", + # "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "integer"}}}}}, + # } + # ) + # } + # ), + Transition( + name="Renaming a stream should fail.", + should_fail=True, + previous={ + "test_stream": AirbyteStream.parse_obj( + { + "name": "test_stream", + "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + } + ) + }, + current={ + "new_test_stream": AirbyteStream.parse_obj( + { + "name": "new_test_stream", + "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + } + ) + }, + ) +] + +VALID_CATALOG_TRANSITIONS = [ + Transition( + name="Making a field nullable should not fail.", + should_fail=False, + previous={ + "test_stream": AirbyteStream.parse_obj( + { + "name": "test_stream", + "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + } + ) + }, + current={ + "test_stream": AirbyteStream.parse_obj( + { + "name": "test_stream", + "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": ["string", "null"]}}}}}, + } + ) + }, + ), + Transition( + name="Changing 'type' field to list should not fail.", + should_fail=False, + previous={ + "test_stream": AirbyteStream.parse_obj( + { + "name": "test_stream", + "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + } + ) + }, + current={ + "test_stream": AirbyteStream.parse_obj( + { + "name": "test_stream", + "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": ["string"]}}}}}, + } + ) + }, + ), + Transition( + name="Removing a field should not fail.", + should_fail=False, + previous={ + "test_stream": AirbyteStream.parse_obj( + { + "name": "test_stream", + "json_schema": { + "properties": { + "user": {"type": "object", "properties": {"username": {"type": "string"}, "email": {"type": "string"}}} + } + }, + } + ) + }, + current={ + "test_stream": AirbyteStream.parse_obj( + { + "name": "test_stream", + "json_schema": {"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, + } + ) + }, + ), +] + + +# Checking that all transitions in FAILING_CATALOG_TRANSITIONS have should_fail == True to prevent typos +assert all([transition.should_fail for transition in FAILING_CATALOG_TRANSITIONS]) +# Checking that all transitions in VALID_CATALOG_TRANSITIONS have should_fail = False to prevent typos +assert not all([transition.should_fail for transition in VALID_CATALOG_TRANSITIONS]) + + +ALL_CATALOG_TRANSITIONS_PARAMS = [transition.as_pytest_param() for transition in FAILING_CATALOG_TRANSITIONS + VALID_CATALOG_TRANSITIONS] + + +@pytest.mark.parametrize("previous_discovered_catalog, discovered_catalog, should_fail", ALL_CATALOG_TRANSITIONS_PARAMS) +def test_catalog_backward_compatibility(previous_discovered_catalog, discovered_catalog, should_fail): + t = _TestDiscovery() + expectation = pytest.raises(NonBackwardCompatibleError) if should_fail else does_not_raise() + with expectation: + t.test_backward_compatibility(False, discovered_catalog, previous_discovered_catalog) From 38486fe4edc64e1ed26efca486e730fd2103cac9 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Tue, 9 Aug 2022 23:36:59 +0200 Subject: [PATCH 08/14] bump version --- airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md | 3 +++ airbyte-integrations/bases/source-acceptance-test/Dockerfile | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index b49ca430ec946..ea5f9d07ac704 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.61 +Backward compatibility tests: add syntactic validation of catalogs [#TBD](https://github.com/airbytehq/airbyte/pull/TBD/) + ## 0.1.60 Backward compatibility tests: validate fake previous config against current connector specification. [#15367](https://github.com/airbytehq/airbyte/pull/15367) diff --git a/airbyte-integrations/bases/source-acceptance-test/Dockerfile b/airbyte-integrations/bases/source-acceptance-test/Dockerfile index 69c749df2d373..fdffec159d68c 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.60 +LABEL io.airbyte.version=0.1.61 LABEL io.airbyte.name=airbyte/source-acceptance-test ENTRYPOINT ["python", "-m", "pytest", "-p", "source_acceptance_test.plugin", "-r", "fEsx"] From 9bfdd7782d96d57dd4df7abd288b683fc925eac9 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 10 Aug 2022 00:29:15 +0200 Subject: [PATCH 09/14] re set number of generated configs in unit test --- .../unit_tests/test_backward_compatibility.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 8108141d574d9..7bcb2245090ed 100644 --- 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 @@ -864,7 +864,7 @@ def test_spec_backward_compatibility(previous_connector_spec, actual_connector_s t = _TestSpec() expectation = pytest.raises(NonBackwardCompatibleError) if should_fail else does_not_raise() with expectation: - t.test_backward_compatibility(False, actual_connector_spec, previous_connector_spec) + t.test_backward_compatibility(False, actual_connector_spec, previous_connector_spec, 10) FAILING_CATALOG_TRANSITIONS = [ From 305ae21fd64d18e4682e54493c0eefcae2d0c88e Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 10 Aug 2022 00:43:49 +0200 Subject: [PATCH 10/14] update SAT reference --- .../testing-connectors/source-acceptance-tests-reference.md | 2 ++ 1 file changed, 2 insertions(+) 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 b3b99d1310b38..c80a4d968d0c5 100644 --- a/docs/connector-development/testing-connectors/source-acceptance-tests-reference.md +++ b/docs/connector-development/testing-connectors/source-acceptance-tests-reference.md @@ -121,6 +121,8 @@ Verifies when a discover operation is run on the connector using the given confi | `config_path` | string | `secrets/config.json` | Path to a JSON object representing a valid connector configuration | | `configured_catalog_path` | string | `integration_tests/configured_catalog.json` | Path to configured catalog | | `timeout_seconds` | int | 30 | Test execution timeout in seconds | +| `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). | ## Test Basic Read From 6f6c3858fe6c4327820a8c6ed47d49ce364fabfc Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 10 Aug 2022 16:06:00 +0200 Subject: [PATCH 11/14] test validate_previous_configs directly --- .../unit_tests/test_backward_compatibility.py | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) 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 index 7bcb2245090ed..95e7c174a9668 100644 --- 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 @@ -9,7 +9,7 @@ from airbyte_cdk.models import AirbyteStream, ConnectorSpecification from source_acceptance_test.tests.test_core import TestDiscovery as _TestDiscovery from source_acceptance_test.tests.test_core import TestSpec as _TestSpec -from source_acceptance_test.utils.backward_compatibility import NonBackwardCompatibleError +from source_acceptance_test.utils.backward_compatibility import NonBackwardCompatibleError, validate_previous_configs from .conftest import does_not_raise @@ -22,6 +22,7 @@ class Transition: current: Union[ConnectorSpecification, MutableMapping[str, AirbyteStream]] should_fail: bool name: str + is_valid_json_schema: bool = True def as_pytest_param(self): return pytest.param(self.previous, self.current, self.should_fail, id=self.name) @@ -445,6 +446,48 @@ def as_pytest_param(self): name="Nested level: Declaring a field enum should fail.", should_fail=True, ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": "string"}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": {}}, + }, + } + ), + name="Changing a 'type' field from a string to something else than a list should fail.", + should_fail=True, + is_valid_json_schema=False, + ), + Transition( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": ["string"]}, + }, + } + ), + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "my_string": {"type": {}}, + }, + } + ), + name="Changing a 'type' field from a list to something else than a string should fail.", + should_fail=True, + is_valid_json_schema=False, + ), ] VALID_SPEC_TRANSITIONS = [ @@ -867,6 +910,18 @@ def test_spec_backward_compatibility(previous_connector_spec, actual_connector_s t.test_backward_compatibility(False, actual_connector_spec, previous_connector_spec, 10) +VALID_JSON_SCHEMA_TRANSITIONS_PARAMS = [ + transition.as_pytest_param() for transition in FAILING_SPEC_TRANSITIONS + VALID_SPEC_TRANSITIONS if transition.is_valid_json_schema +] + + +@pytest.mark.parametrize("previous_connector_spec, actual_connector_spec, should_fail", VALID_JSON_SCHEMA_TRANSITIONS_PARAMS) +def test_validate_previous_configs(previous_connector_spec, actual_connector_spec, should_fail): + expectation = pytest.raises(NonBackwardCompatibleError) if should_fail else does_not_raise() + with expectation: + validate_previous_configs(previous_connector_spec, actual_connector_spec, 100) + + FAILING_CATALOG_TRANSITIONS = [ Transition( name="Removing a stream from a catalog should fail.", From e347c890bec84f575bd7dd217b67e7e84b582092 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 10 Aug 2022 16:07:05 +0200 Subject: [PATCH 12/14] improve test coverage --- .../source_acceptance_test/tests/test_core.py | 6 ++-- .../utils/backward_compatibility.py | 29 +++++++++---------- 2 files changed, 17 insertions(+), 18 deletions(-) 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 0bcd70d7460a2..38ca1c657341e 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 @@ -28,7 +28,7 @@ from docker.errors import ContainerError from jsonschema._utils import flatten from source_acceptance_test.base import BaseTest -from source_acceptance_test.config import BasicReadTestConfig, ConnectionTestConfig, SpecTestConfig +from source_acceptance_test.config import BasicReadTestConfig, ConnectionTestConfig, DiscoveryTestConfig, SpecTestConfig from source_acceptance_test.utils import ConnectorRunner, SecretDict, filter_output, make_hashable, verify_records_schema from source_acceptance_test.utils.backward_compatibility import CatalogDiffChecker, SpecDiffChecker, validate_previous_configs from source_acceptance_test.utils.common import find_all_values_for_key_in_schema, find_keyword_schema @@ -247,7 +247,9 @@ def compute_discovered_catalog_diff( ) @pytest.fixture(name="skip_backward_compatibility_tests") - def skip_backward_compatibility_tests_fixture(self, inputs: SpecTestConfig, previous_connector_docker_runner: ConnectorRunner) -> bool: + def skip_backward_compatibility_tests_fixture( + self, inputs: DiscoveryTestConfig, previous_connector_docker_runner: ConnectorRunner + ) -> bool: if previous_connector_docker_runner is None: pytest.skip("The previous connector image could not be retrieved.") diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py index ef3241565a9fe..eff9603728bf2 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py @@ -8,7 +8,7 @@ import jsonschema from airbyte_cdk.models import ConnectorSpecification from deepdiff import DeepDiff -from hypothesis import given, settings +from hypothesis import Verbosity, given, settings from hypothesis_jsonschema import from_schema from source_acceptance_test.utils import SecretDict @@ -26,11 +26,11 @@ def _raise_error(self, message: str): @property @abstractmethod - def context(self): + def context(self): # pragma: no cover pass @abstractmethod - def assert_is_backward_compatible(self): + def assert_is_backward_compatible(self): # pragma: no cover pass def check_if_value_of_type_field_changed(self): @@ -45,7 +45,7 @@ def check_if_value_of_type_field_changed(self): if type_values_changed or type_values_changed_in_list: self._raise_error("The value of a 'type' field was changed.") - def check_if_new_type_was_added(self): + def check_if_new_type_was_added(self): # pragma: no cover """Detect type value added to type list if new type value is not None (e.g ["str"] -> ["str", "int"])""" new_values_in_type_list = [ change @@ -74,11 +74,7 @@ def check_if_type_of_type_field_changed(self): # This might be something already guaranteed by JSON schema validation. if isinstance(change.t1, str): if not isinstance(change.t2, list): - self._raise_error("The current {context} change a type field from string to an invalid value.") - if not 0 < len(change.t2) <= 2: - self._raise_error( - "A type field changed from string to an invalid value. The type list should not be empty and have a maximum of two items." - ) + self._raise_error("A 'type' field was changed from string to an invalid value.") # 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"] @@ -159,14 +155,15 @@ def validate_previous_configs( 2. Validate a fake previous config against the actual connector specification json schema.""" @given(from_schema(previous_connector_spec.dict()["connectionSpecification"])) - @settings(max_examples=number_of_configs_to_generate) + @settings(max_examples=number_of_configs_to_generate, verbosity=Verbosity.quiet) def check_fake_previous_config_against_actual_spec(fake_previous_config): - fake_previous_config = SecretDict(fake_previous_config) - filtered_fake_previous_config = {key: value for key, value in fake_previous_config.data.items() if not key.startswith("_")} - try: - jsonschema.validate(instance=filtered_fake_previous_config, schema=actual_connector_spec.connectionSpecification) - except jsonschema.exceptions.ValidationError as err: - raise NonBackwardCompatibleError(err) + if isinstance(fake_previous_config, dict): # Looks like hypothesis-jsonschema not only generate dict objects... + fake_previous_config = SecretDict(fake_previous_config) + filtered_fake_previous_config = {key: value for key, value in fake_previous_config.data.items() if not key.startswith("_")} + try: + jsonschema.validate(instance=filtered_fake_previous_config, schema=actual_connector_spec.connectionSpecification) + except jsonschema.exceptions.ValidationError as err: + raise NonBackwardCompatibleError(err) check_fake_previous_config_against_actual_spec() From 3c84690f38ba1b46be5517acfd06a23032e62034 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 10 Aug 2022 16:11:16 +0200 Subject: [PATCH 13/14] improve consistency in error messages --- .../utils/backward_compatibility.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py index eff9603728bf2..acecb909e65d5 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py @@ -22,7 +22,7 @@ def __init__(self, diff: DeepDiff) -> None: self._diff = diff def _raise_error(self, message: str): - raise NonBackwardCompatibleError(f"{context} - {message}: {self._diff.pretty()}") + raise NonBackwardCompatibleError(f"{context} - {message}. Diff: {self._diff.pretty()}") @property @abstractmethod @@ -43,7 +43,7 @@ def check_if_value_of_type_field_changed(self): change for change in self._diff.get("values_changed", []) if change.path(output_format="list")[-2] == "type" ] if type_values_changed or type_values_changed_in_list: - self._raise_error("The value of a 'type' field was changed.") + self._raise_error("The'type' field value was changed.") def check_if_new_type_was_added(self): # pragma: no cover """Detect type value added to type list if new type value is not None (e.g ["str"] -> ["str", "int"])""" @@ -54,7 +54,7 @@ def check_if_new_type_was_added(self): # pragma: no cover if change.t2 != "null" ] if new_values_in_type_list: - self._raise_error("A new value was added to a 'type' field.") + self._raise_error("A new value was added to a 'type' field") def check_if_type_of_type_field_changed(self): """ @@ -79,12 +79,12 @@ def check_if_type_of_type_field_changed(self): # We want to raise an error otherwise. t2_not_null_types = [_type for _type in change.t2 if _type != "null"] if not (len(t2_not_null_types) == 1 and t2_not_null_types[0] == change.t1): - self._raise_error("The type field changed to a list with multiple invalid values.") + self._raise_error("The 'type' field was changed to a list with multiple invalid values") if isinstance(change.t1, list): if not isinstance(change.t2, str): - self._raise_error("The type field changed from a list to an invalid value.") + self._raise_error("The 'type' field was changed from a list to an invalid value") if not (len(change.t1) == 1 and change.t2 == change.t1[0]): - self._raise_error("An element was removed from the list of valid types.") + self._raise_error("An element was removed from the list of 'type'") class SpecDiffChecker(BaseDiffChecker): @@ -108,10 +108,10 @@ def check_if_declared_new_required_field(self): addition for addition in self._diff.get("dictionary_item_added", []) if addition.path(output_format="list")[-1] == "required" ] if added_required_fields: - self._raise_error(f"The current {context} declared a new 'required' field") + self._raise_error("A new 'required' field was declared.") def check_if_added_a_new_required_property(self): - """Check if the new spec added a property to the 'required' list.""" + """Check if the new spec added a property to the 'required' list""" added_required_properties = [ addition for addition in self._diff.get("iterable_item_added", []) if addition.up.path(output_format="list")[-1] == "required" ] @@ -124,7 +124,7 @@ def check_if_field_was_made_not_nullable(self): change for change in self._diff.get("iterable_item_removed", []) if change.path(output_format="list")[-2] == "type" ] if removed_nullable: - self._raise_error("A field type was narrowed or made a field not nullable.") + self._raise_error("A field type was narrowed or made a field not nullable") def check_if_enum_was_narrowed(self): """Check if the list of values in a enum was shortened in a spec.""" @@ -144,7 +144,7 @@ def check_if_declared_new_enum_field(self): if enum_addition.path(output_format="list")[-1] == "enum" ] if enum_additions: - self._raise_error("An 'enum' field was declared on an existing property of the spec.") + self._raise_error("An 'enum' field was declared on an existing property") def validate_previous_configs( From 742c9f7d7b7206079a2bbb40eed462119edc8042 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 10 Aug 2022 16:55:32 +0200 Subject: [PATCH 14/14] hypothesis: suppress too slow healthcheck for CI --- .../source_acceptance_test/utils/backward_compatibility.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py index acecb909e65d5..26f5152c88a2e 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py @@ -8,7 +8,7 @@ import jsonschema from airbyte_cdk.models import ConnectorSpecification from deepdiff import DeepDiff -from hypothesis import Verbosity, given, settings +from hypothesis import HealthCheck, Verbosity, given, settings from hypothesis_jsonschema import from_schema from source_acceptance_test.utils import SecretDict @@ -155,7 +155,7 @@ def validate_previous_configs( 2. Validate a fake previous config against the actual connector specification json schema.""" @given(from_schema(previous_connector_spec.dict()["connectionSpecification"])) - @settings(max_examples=number_of_configs_to_generate, verbosity=Verbosity.quiet) + @settings(max_examples=number_of_configs_to_generate, verbosity=Verbosity.quiet, suppress_health_check=(HealthCheck.too_slow,)) def check_fake_previous_config_against_actual_spec(fake_previous_config): if isinstance(fake_previous_config, dict): # Looks like hypothesis-jsonschema not only generate dict objects... fake_previous_config = SecretDict(fake_previous_config)