🐛 Backward compatibility test: Don't fail on updating additionalProperties (#15532)

Author: girarda

airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 0.2.1
+Don't fail on updating `additionalProperties`: fix IndexError [#15532](https://github.com/airbytehq/airbyte/pull/15532/)
+
 ## 0.2.0
 Finish backward compatibility syntactic tests implementation: check that cursor fields were not changed. [#15520](https://github.com/airbytehq/airbyte/pull/15520/)
 

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.2.0
+LABEL io.airbyte.version=0.2.1
 LABEL io.airbyte.name=airbyte/source-acceptance-test
 
 ENTRYPOINT ["python", "-m", "pytest", "-p", "source_acceptance_test.plugin", "-r", "fEsx"]

airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/backward_compatibility.py

@@ -51,15 +51,12 @@ def assert_is_backward_compatible(self):  # pragma: no cover
         pass
 
     def check_if_value_of_type_field_changed(self, diff: DeepDiff):
-        """Check if a type was changed"""
+        """Check if a type was changed on a property"""
         # Detect type value change in case type field is declared as a string (e.g "str" -> "int"):
-        type_values_changed = [change for change in 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 diff.get("values_changed", []) if change.path(output_format="list")[-2] == "type"
+        changes_on_property_type = [
+            change for change in diff.get("values_changed", []) if {"properties", "type"}.issubset(change.path(output_format="list"))
         ]
-        if type_values_changed or type_values_changed_in_list:
+        if changes_on_property_type:
             self._raise_error("The'type' field value was changed.", diff)
 
     def check_if_new_type_was_added(self, diff: DeepDiff):  # pragma: no cover
@@ -75,7 +72,7 @@ def check_if_new_type_was_added(self, diff: DeepDiff):  # pragma: no cover
 
     def check_if_type_of_type_field_changed(self, diff: DeepDiff):
         """
-        Detect the change of type of a type field
+        Detect the change of type of a type field on a property
         e.g:
         - "str" -> ["str"] VALID
         - "str" -> ["str", "null"] VALID
@@ -85,7 +82,9 @@ def check_if_type_of_type_field_changed(self, diff: DeepDiff):
         - ["str"] -> "int" INVALID
         - ["str"] -> 1 INVALID
         """
-        type_changes = [change for change in diff.get("type_changes", []) if change.path(output_format="list")[-1] == "type"]
+        type_changes = [
+            change for change in diff.get("type_changes", []) if {"properties", "type"}.issubset(change.path(output_format="list"))
+        ]
         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.
@@ -145,7 +144,9 @@ def check_if_added_a_new_required_property(self, diff: DeepDiff):
 
     def check_if_field_was_made_not_nullable(self, diff: DeepDiff):
         """Detect when field was made not nullable but is still a list: e.g ["string", "null"] -> ["string"]"""
-        removed_nullable = [change for change in diff.get("iterable_item_removed", []) if change.path(output_format="list")[-2] == "type"]
+        removed_nullable = [
+            change for change in diff.get("iterable_item_removed", []) if {"properties", "type"}.issubset(change.path(output_format="list"))
+        ]
         if removed_nullable:
             self._raise_error("A field type was narrowed or made a field not nullable", diff)
 

airbyte-integrations/bases/source-acceptance-test/unit_tests/test_backward_compatibility.py

@@ -513,6 +513,54 @@ def as_pytest_param(self):
         name="Not changing a spec should not fail",
         should_fail=False,
     ),
+    Transition(
+        ConnectorSpecification(
+            connectionSpecification={
+                "type": "object",
+                "required": ["my_required_string"],
+                "additionalProperties": False,
+                "properties": {
+                    "my_required_string": {"type": "string"},
+                },
+            }
+        ),
+        ConnectorSpecification(
+            connectionSpecification={
+                "type": "object",
+                "required": ["my_required_string"],
+                "additionalProperties": True,
+                "properties": {
+                    "my_required_string": {"type": "string"},
+                },
+            }
+        ),
+        name="Top level: Changing the value of additionalProperties 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",
+                        "additionalProperties": True,
+                        "properties": {"my_property": {"type": ["integer"]}},
+                    },
+                },
+            }
+        ),
+        name="Nested level: Changing the value of additionalProperties should not fail",
+        should_fail=False,
+    ),
     Transition(
         ConnectorSpecification(
             connectionSpecification={
@@ -898,7 +946,6 @@ def as_pytest_param(self):
 # 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])
 
-
 ALL_SPEC_TRANSITIONS_PARAMS = [transition.as_pytest_param() for transition in FAILING_SPEC_TRANSITIONS + VALID_SPEC_TRANSITIONS]
 
 
@@ -920,7 +967,7 @@ def test_spec_backward_compatibility(previous_connector_spec, actual_connector_s
 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)
+        validate_previous_configs(previous_connector_spec, actual_connector_spec, 200)
 
 
 FAILING_CATALOG_TRANSITIONS = [
@@ -1152,7 +1199,6 @@ def test_validate_previous_configs(previous_connector_spec, actual_connector_spe
 # 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]