diff --git a/airbyte-ci/connectors/connectors_qa/src/connectors_qa/checks/version.py b/airbyte-ci/connectors/connectors_qa/src/connectors_qa/checks/version.py index 633ba218236..bdda4fe0852 100644 --- a/airbyte-ci/connectors/connectors_qa/src/connectors_qa/checks/version.py +++ b/airbyte-ci/connectors/connectors_qa/src/connectors_qa/checks/version.py @@ -1,6 +1,6 @@ # Copyright (c) 2025 Airbyte, Inc., all rights reserved. -from typing import Any, Dict +from typing import Any, Dict, Optional import requests # type: ignore import semver @@ -83,14 +83,54 @@ class CheckVersionIncrement(Check): and master_version.patch == current_version.patch ) + @staticmethod + def _get_registry_override_tag(metadata: Optional[Dict[str, Any]], channel: str) -> Optional[str]: + """Extract the dockerImageTag from registryOverrides for a given channel. + + Args: + metadata: The metadata dictionary (master or current). + channel: The channel to extract from ("cloud" or "oss"). + + Returns: + The dockerImageTag value if present, None otherwise. + """ + if metadata is None: + return None + try: + return metadata.get("registryOverrides", {}).get(channel, {}).get("dockerImageTag") + except (TypeError, AttributeError): + return None + + def _has_registry_override_docker_tag_change( + self, master_metadata: Optional[Dict[str, Any]], current_metadata: Optional[Dict[str, Any]] + ) -> bool: + """Check if registryOverrides.cloud.dockerImageTag or registryOverrides.oss.dockerImageTag has changed. + + Args: + master_metadata: The metadata from the master branch. + current_metadata: The current metadata. + + Returns: + bool: True if either cloud or oss dockerImageTag has been added, removed, or changed. + """ + master_cloud = self._get_registry_override_tag(master_metadata, "cloud") + current_cloud = self._get_registry_override_tag(current_metadata, "cloud") + + master_oss = self._get_registry_override_tag(master_metadata, "oss") + current_oss = self._get_registry_override_tag(current_metadata, "oss") + + return (master_cloud != current_cloud) or (master_oss != current_oss) + def _run(self, connector: Connector) -> CheckResult: """Run the version increment check.""" if connector.metadata and connector.metadata.get("ab_internal", {}).get("requireVersionIncrementsInPullRequests") is False: return self.skip(connector, "Connector opts out of version increment checks.") try: - master_version = self._get_master_connector_version(connector) + master_metadata = self._get_master_metadata(connector) + master_version = self._parse_version_from_metadata(master_metadata) if master_metadata else semver.Version.parse("0.0.0") current_version = self._get_current_connector_version(connector) + same_versions_but_has_registry_override = False # Require a version increment if current_version < master_version: @@ -102,12 +142,17 @@ class CheckVersionIncrement(Check): f"Update your PR branch from the default branch to get the latest connector version.", ) if current_version == master_version: - return self.fail( - connector, - f"The dockerImageTag in {consts.METADATA_FILE_NAME} was not incremented. " - f"Master version is {master_version}, current version is {current_version}. " - f"Ignore this message if you do not intend to re-release the connector.", - ) + # Allow version to stay the same if registryOverrides.cloud.dockerImageTag or + # registryOverrides.oss.dockerImageTag has been added, removed, or changed + if not self._has_registry_override_docker_tag_change(master_metadata, connector.metadata): + return self.fail( + connector, + f"The dockerImageTag in {consts.METADATA_FILE_NAME} was not incremented. " + f"Master version is {master_version}, current version is {current_version}. " + f"Ignore this message if you do not intend to re-release the connector.", + ) + else: + same_versions_but_has_registry_override = True if self._are_both_versions_release_candidates(master_version, current_version): if not self._have_same_major_minor_patch(master_version, current_version): @@ -118,7 +163,14 @@ class CheckVersionIncrement(Check): f"current version is {current_version}", ) - return self.pass_(connector, f"Version was properly incremented from {master_version} to {current_version}.") + if same_versions_but_has_registry_override: + return self.skip( + connector, + f"The current change is modifying the registryOverrides pinned version on Cloud or OSS. Skipping this check " + f"because the defined version {current_version} is allowed to be unchanged", + ) + else: + return self.pass_(connector, f"Version was properly incremented from {master_version} to {current_version}.") except (requests.HTTPError, ValueError, TypeError) as e: return self.fail(connector, str(e)) diff --git a/airbyte-ci/connectors/connectors_qa/tests/unit_tests/test_checks/test_version.py b/airbyte-ci/connectors/connectors_qa/tests/unit_tests/test_checks/test_version.py index e7661d61f77..325a78fd11a 100644 --- a/airbyte-ci/connectors/connectors_qa/tests/unit_tests/test_checks/test_version.py +++ b/airbyte-ci/connectors/connectors_qa/tests/unit_tests/test_checks/test_version.py @@ -12,12 +12,19 @@ class TestVersionIncrementCheck: @pytest.fixture def mock_connector(self, mocker, tmp_path): connector = mocker.Mock(code_directory=str(tmp_path), technical_name="mock-connector") + # Set up default metadata without registryOverrides + connector.metadata = {"dockerImageTag": "1.0.0"} return connector - def _get_version_increment_check(self, mocker, master_version="1.0.0", current_version="1.0.1"): + def _get_version_increment_check( + self, mocker, master_version="1.0.0", current_version="1.0.1", master_metadata=None, current_metadata=None + ): + # Default master metadata if not provided + if master_metadata is None: + master_metadata = {"dockerImageTag": master_version} mocker.patch( - "connectors_qa.checks.version.CheckVersionIncrement._get_master_connector_version", - return_value=semver.Version.parse(master_version), + "connectors_qa.checks.version.CheckVersionIncrement._get_master_metadata", + return_value=master_metadata, ) mocker.patch( "connectors_qa.checks.version.CheckVersionIncrement._get_current_connector_version", @@ -27,11 +34,13 @@ class TestVersionIncrementCheck: return CheckVersionIncrement() def test_validate_success(self, mocker, mock_connector): + mock_connector.metadata = {"dockerImageTag": "1.0.1"} version_increment_check = self._get_version_increment_check(mocker, master_version="1.0.0", current_version="1.0.1") result = version_increment_check._run(mock_connector) assert result.status == CheckStatus.PASSED def test_validate_failure_no_increment(self, mock_connector, mocker): + mock_connector.metadata = {"dockerImageTag": "1.0.0"} version_increment_check = self._get_version_increment_check(mocker, master_version="1.0.0", current_version="1.0.0") result = version_increment_check._run(mock_connector) assert result.status == CheckStatus.FAILED @@ -41,6 +50,7 @@ class TestVersionIncrementCheck: ) def test_validate_failure_decrement(self, mock_connector, mocker): + mock_connector.metadata = {"dockerImageTag": "1.0.0"} version_increment_check = self._get_version_increment_check(mocker, master_version="1.1.0", current_version="1.0.0") result = version_increment_check._run(mock_connector) assert result.status == CheckStatus.FAILED @@ -50,11 +60,13 @@ class TestVersionIncrementCheck: ) def test_validate_success_rc_increment(self, mock_connector, mocker): + mock_connector.metadata = {"dockerImageTag": "1.0.1-rc.2"} version_increment_check = self._get_version_increment_check(mocker, master_version="1.0.1-rc.1", current_version="1.0.1-rc.2") result = version_increment_check._run(mock_connector) assert result.status == CheckStatus.PASSED def test_validate_failure_rc_with_different_versions(self, mock_connector, mocker): + mock_connector.metadata = {"dockerImageTag": "1.0.1-rc.1"} version_increment_check = self._get_version_increment_check(mocker, master_version="1.0.0-rc.1", current_version="1.0.1-rc.1") result = version_increment_check._run(mock_connector) assert result.status == CheckStatus.FAILED @@ -62,3 +74,33 @@ class TestVersionIncrementCheck: result.message == f"Master and current version are release candidates but they have different major, minor or patch versions. Release candidates should only differ in the prerelease part. Master version is 1.0.0-rc.1, current version is 1.0.1-rc.1" ) + + def test_validate_skipped_registry_override_cloud_added(self, mock_connector, mocker): + """Test that validation is skipped when registryOverrides.cloud.dockerImageTag is added.""" + master_metadata = {"dockerImageTag": "1.0.0"} + mock_connector.metadata = {"dockerImageTag": "1.0.0", "registryOverrides": {"cloud": {"dockerImageTag": "1.0.0-cloud"}}} + version_increment_check = self._get_version_increment_check( + mocker, master_version="1.0.0", current_version="1.0.0", master_metadata=master_metadata + ) + result = version_increment_check._run(mock_connector) + assert result.status == CheckStatus.SKIPPED + + def test_validate_skipped_registry_override_oss_changed(self, mock_connector, mocker): + """Test that validation is skipped when registryOverrides.oss.dockerImageTag is changed.""" + master_metadata = {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.0-oss"}}} + mock_connector.metadata = {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.1-oss"}}} + version_increment_check = self._get_version_increment_check( + mocker, master_version="1.0.0", current_version="1.0.0", master_metadata=master_metadata + ) + result = version_increment_check._run(mock_connector) + assert result.status == CheckStatus.SKIPPED + + def test_validate_failure_no_increment_registry_override_unchanged(self, mock_connector, mocker): + """Test that validation fails when version is not incremented and registryOverrides are unchanged.""" + master_metadata = {"dockerImageTag": "1.0.0", "registryOverrides": {"cloud": {"dockerImageTag": "1.0.0-cloud"}}} + mock_connector.metadata = {"dockerImageTag": "1.0.0", "registryOverrides": {"cloud": {"dockerImageTag": "1.0.0-cloud"}}} + version_increment_check = self._get_version_increment_check( + mocker, master_version="1.0.0", current_version="1.0.0", master_metadata=master_metadata + ) + result = version_increment_check._run(mock_connector) + assert result.status == CheckStatus.FAILED diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/context.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/context.py index b941d05f034..7bdb4027009 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/context.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/context.py @@ -34,6 +34,25 @@ class ConnectorTestContext(ConnectorContext): self.run_step_options = self._skip_metadata_disabled_test_suites(self.run_step_options) self.step_id_to_secrets_mapping = self._get_step_id_to_secret_mapping() + @property + def test_only_change(self) -> bool: + """Check if the only modified files are in unit_tests or integration_tests directories. + + Returns: + bool: True if all modified files are in unit_tests or integration_tests, False otherwise. + """ + test_directories = ("unit_tests", "integration_tests") + for modified_file in self.modified_files: + try: + rel_path = modified_file.relative_to(self.connector.code_directory) + top_level_dir = rel_path.parts[0] if rel_path.parts else "" + if top_level_dir not in test_directories: + return False + except ValueError: + # File not under connector directory, treat as non-test change + return False + return True + @staticmethod def _handle_missing_secret_store( secret_info: Dict[str, str | Dict[str, str]], raise_on_missing: bool, logger: Optional[Logger] = None diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/pipeline.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/pipeline.py index 495bfc17d57..f9dc1a827d6 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/pipeline.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/pipeline.py @@ -54,7 +54,8 @@ async def run_connector_test_pipeline(context: ConnectorTestContext, semaphore: all_steps_to_run += get_test_steps(context) - if not context.code_tests_only: + # Skip static analysis steps when only test files are modified + if not context.code_tests_only and not context.test_only_change: static_analysis_steps_to_run = [ [ StepToRun(id=CONNECTOR_TEST_STEP_ID.VERSION_INC_CHECK, step=VersionIncrementCheck(context)), diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py index 62869772169..0166e1d31dc 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py @@ -153,8 +153,51 @@ class VersionIncrementCheck(VersionCheck): and self.master_connector_version.patch == self.current_connector_version.patch ) + @staticmethod + def _get_registry_override_tag(metadata: Optional[dict], channel: str) -> Optional[str]: + """Extract the dockerImageTag from registryOverrides for a given channel. + + Args: + metadata: The metadata dictionary (master or current). + channel: The channel to extract from ("cloud" or "oss"). + + Returns: + The dockerImageTag value if present, None otherwise. + """ + if metadata is None: + return None + try: + return metadata.get("data", {}).get("registryOverrides", {}).get(channel, {}).get("dockerImageTag") + except (TypeError, AttributeError): + return None + + def _has_registry_override_docker_tag_change(self) -> bool: + """Check if registryOverrides.cloud.dockerImageTag or registryOverrides.oss.dockerImageTag has changed. + + Returns: + bool: True if either cloud or oss dockerImageTag has been added, removed, or changed. + """ + master_cloud = self._get_registry_override_tag(self.master_metadata, "cloud") + current_cloud = self._get_registry_override_tag(self.context.metadata, "cloud") + + master_oss = self._get_registry_override_tag(self.master_metadata, "oss") + current_oss = self._get_registry_override_tag(self.context.metadata, "oss") + + return (master_cloud != current_cloud) or (master_oss != current_oss) + def validate(self) -> StepResult: if self.is_version_not_incremented(): + # Allow version to stay the same if registryOverrides.cloud.dockerImageTag or + # registryOverrides.oss.dockerImageTag has been added, removed, or changed + if self._has_registry_override_docker_tag_change(): + return StepResult( + step=self, + status=StepStatus.SKIPPED, + stdout=( + f"The current change is modifying the registryOverrides pinned version on Cloud or OSS. Skipping this check " + f"because the defined version {self.current_connector_version} is allowed to be unchanged" + ), + ) return self._get_failure_result( ( f"The dockerImageTag in {METADATA_FILE_NAME} was not incremented. " diff --git a/airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py b/airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py index 02c5276ce47..3ab131da3b9 100644 --- a/airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py +++ b/airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py @@ -5,6 +5,7 @@ from connector_ops.utils import METADATA_FILE_NAME from semver import VersionInfo from pipelines.airbyte_ci.connectors.test.steps.common import VersionIncrementCheck +from pipelines.models.steps import StepStatus class TestVersionIncrementCheck: @@ -46,6 +47,13 @@ class TestVersionIncrementCheck: def test_validate_failure_no_increment(self, context, mocker): version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + # Set up metadata to ensure registryOverrides check doesn't bypass the failure + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0"}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0"}} result = version_increment_check.validate() assert not result.success assert ( @@ -70,3 +78,87 @@ class TestVersionIncrementCheck: ) result = version_increment_check.validate() assert result.success + + def test_validate_skipped_registry_override_oss_added(self, context, mocker): + """Test that version increment check is skipped when registryOverrides.oss.dockerImageTag is added.""" + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0"}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.0-oss"}}}} + result = version_increment_check.validate() + assert result.status == StepStatus.SKIPPED + + def test_validate_skipped_registry_override_oss_removed(self, context, mocker): + """Test that version increment check is skipped when registryOverrides.oss.dockerImageTag is removed.""" + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.0-oss"}}}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0"}} + result = version_increment_check.validate() + assert result.status == StepStatus.SKIPPED + + def test_validate_skipped_registry_override_oss_changed(self, context, mocker): + """Test that version increment check is skipped when registryOverrides.oss.dockerImageTag is changed.""" + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.0-oss"}}}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.1-oss"}}}} + result = version_increment_check.validate() + assert result.status == StepStatus.SKIPPED + + def test_validate_skipped_registry_override_cloud_added(self, context, mocker): + """Test that version increment check is skipped when registryOverrides.cloud.dockerImageTag is added.""" + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0"}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"cloud": {"dockerImageTag": "1.0.0-cloud"}}}} + result = version_increment_check.validate() + assert result.status == StepStatus.SKIPPED + + def test_validate_skipped_registry_override_cloud_removed(self, context, mocker): + """Test that version increment check is skipped when registryOverrides.cloud.dockerImageTag is removed.""" + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"cloud": {"dockerImageTag": "1.0.0-cloud"}}}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0"}} + result = version_increment_check.validate() + assert result.status == StepStatus.SKIPPED + + def test_validate_skipped_registry_override_cloud_changed(self, context, mocker): + """Test that version increment check is skipped when registryOverrides.cloud.dockerImageTag is changed.""" + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"cloud": {"dockerImageTag": "1.0.0-cloud"}}}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"cloud": {"dockerImageTag": "1.0.1-cloud"}}}} + result = version_increment_check.validate() + assert result.status == StepStatus.SKIPPED + + def test_validate_failure_no_increment_registry_override_unchanged(self, context, mocker): + """Test that version increment check fails when dockerImageTag is not incremented and registryOverrides are unchanged.""" + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + mocker.patch.object( + version_increment_check, + "master_metadata", + {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.0-oss"}}}}, + ) + context.metadata = {"data": {"dockerImageTag": "1.0.0", "registryOverrides": {"oss": {"dockerImageTag": "1.0.0-oss"}}}} + result = version_increment_check.validate() + assert not result.success diff --git a/airbyte-ci/connectors/pipelines/tests/test_tests/test_context.py b/airbyte-ci/connectors/pipelines/tests/test_tests/test_context.py new file mode 100644 index 00000000000..c5cdabc010e --- /dev/null +++ b/airbyte-ci/connectors/pipelines/tests/test_tests/test_context.py @@ -0,0 +1,70 @@ +# +# Copyright (c) 2023 Airbyte, Inc., all rights reserved. +# + +from pathlib import Path + +import pytest + +CONNECTOR_CODE_DIRECTORY = Path("/path/to/connector") + + +@pytest.mark.parametrize( + "modified_files,expected_result", + [ + pytest.param( + frozenset([CONNECTOR_CODE_DIRECTORY / "unit_tests" / "test_source.py"]), + True, + id="only_unit_tests_changed", + ), + pytest.param( + frozenset([CONNECTOR_CODE_DIRECTORY / "integration_tests" / "test_integration.py"]), + True, + id="only_integration_tests_changed", + ), + pytest.param( + frozenset( + [ + CONNECTOR_CODE_DIRECTORY / "unit_tests" / "test_source.py", + CONNECTOR_CODE_DIRECTORY / "integration_tests" / "test_integration.py", + ] + ), + True, + id="both_unit_and_integration_tests_changed", + ), + pytest.param( + frozenset( + [ + CONNECTOR_CODE_DIRECTORY / "unit_tests" / "test_source.py", + CONNECTOR_CODE_DIRECTORY / "src" / "main.py", + ] + ), + False, + id="unit_tests_and_source_code_changed", + ), + pytest.param( + frozenset([CONNECTOR_CODE_DIRECTORY / "src" / "main.py"]), + False, + id="only_source_code_changed", + ), + pytest.param( + frozenset(), + True, + id="no_modified_files", + ), + ], +) +def test_test_only_change(mocker, modified_files, expected_result): + """Test that test_only_change correctly identifies when only test files are modified.""" + from pipelines.airbyte_ci.connectors.test.context import ConnectorTestContext + + mock_connector = mocker.Mock() + mock_connector.code_directory = CONNECTOR_CODE_DIRECTORY + + mock_context = mocker.Mock(spec=ConnectorTestContext) + mock_context.modified_files = modified_files + mock_context.connector = mock_connector + + result = ConnectorTestContext.test_only_change.fget(mock_context) + + assert result == expected_result