Skip to content

Commit

Permalink
feat(qa-checks): Add AcceptanceTestsEnabledCheck (#38736)
Browse files Browse the repository at this point in the history
Co-authored-by: alafanechere <[email protected]>
  • Loading branch information
bnchrch and alafanechere authored May 29, 2024
1 parent e56dc9d commit 6db379d
Show file tree
Hide file tree
Showing 12 changed files with 245 additions and 5 deletions.
2 changes: 1 addition & 1 deletion airbyte-ci/connectors/connector_ops/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,5 @@ poetry run pytest
```

## Changelog

- 0.5.0: Added `cloud_usage` property to `Connector` class.
- 0.4.0: Removed acceptance test configuration and allowed hosts checks as they're not used.
33 changes: 32 additions & 1 deletion airbyte-ci/connectors/connector_ops/connector_ops/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import requests
import yaml
from ci_credentials import SecretsManager
from pydash.collections import find
from pydash.objects import get
from rich.console import Console
from simpleeval import simple_eval
Expand All @@ -24,6 +25,7 @@

DIFFED_BRANCH = os.environ.get("DIFFED_BRANCH", "origin/master")
OSS_CATALOG_URL = "https://connectors.airbyte.com/files/registries/v0/oss_registry.json"
CLOUD_CATALOG_URL = "https://connectors.airbyte.com/files/registries/v0/cloud_registry.json"
BASE_AIRBYTE_DOCS_URL = "https://docs.airbyte.com"
CONNECTOR_PATH_PREFIX = "airbyte-integrations/connectors"
SOURCE_CONNECTOR_PATH_PREFIX = CONNECTOR_PATH_PREFIX + "/source-"
Expand Down Expand Up @@ -563,6 +565,17 @@ def normalization_tag(self) -> Optional[str]:
def is_using_poetry(self) -> bool:
return Path(self.code_directory / "pyproject.toml").exists()

@property
def registry_primary_key_field(self) -> str:
"""
The primary key field of the connector in the registry.
example:
- source -> sourceDefinitionId
- destination -> destinationDefinitionId
"""
return f"{self.connector_type}DefinitionId"

@property
def is_released(self) -> bool:
"""Pull the the OSS registry and check if it the current definition ID and docker image tag are in the registry.
Expand All @@ -576,12 +589,30 @@ def is_released(self) -> bool:
registry = download_catalog(OSS_CATALOG_URL)
for connector in registry[f"{self.connector_type}s"]:
if (
connector[f"{self.connector_type}DefinitionId"] == metadata["definitionId"]
connector[self.registry_primary_key_field] == metadata["definitionId"]
and connector["dockerImageTag"] == metadata["dockerImageTag"]
):
return True
return False

@property
def cloud_usage(self) -> Optional[str]:
"""Pull the cloud registry, check if the connector is in the registry and return the usage metrics.
Returns:
Optional[str]: The usage metrics of the connector, could be one of ["low", "medium", "high"] or None if the connector is not in the registry.
"""
metadata = self.metadata
definition_id = metadata.get("definitionId")
cloud_registry = download_catalog(CLOUD_CATALOG_URL)

all_connectors_of_type = cloud_registry[f"{self.connector_type}s"]
connector_entry = find(all_connectors_of_type, {self.registry_primary_key_field: definition_id})
if not connector_entry:
return None

return get(connector_entry, "generated.metrics.cloud.usage")

def get_secret_manager(self, gsm_credentials: str):
return SecretsManager(connector_name=self.technical_name, gsm_credentials=gsm_credentials)

Expand Down
2 changes: 1 addition & 1 deletion airbyte-ci/connectors/connector_ops/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"

[tool.poetry]
name = "connector_ops"
version = "0.4.0"
version = "0.5.0"
description = "Packaged maintained by the connector operations team to perform CI for connectors"
authors = ["Airbyte <[email protected]>"]

Expand Down
4 changes: 4 additions & 0 deletions airbyte-ci/connectors/connectors_qa/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ poe lint

## Changelog

### 1.4.0

Added the `IntegrationTestsEnabledCheck` check that verifies if the integration tests are enabled for connectors with higher cloud usage.

### 1.3.2

Removed documentation checks in `MedatadaCheck` since it's already verified in `DocumentationCheck`.
Expand Down
2 changes: 1 addition & 1 deletion airbyte-ci/connectors/connectors_qa/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion airbyte-ci/connectors/connectors_qa/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "connectors-qa"
version = "1.3.2"
version = "1.4.0"
description = "A package to run QA checks on Airbyte connectors, generate reports and documentation."
authors = ["Airbyte <[email protected]>"]
readme = "README.md"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
from .metadata import ENABLED_CHECKS as METADATA_CORRECTNESS_CHECKS
from .packaging import ENABLED_CHECKS as PACKAGING_CHECKS
from .security import ENABLED_CHECKS as SECURITY_CHECKS
from .testing import ENABLED_CHECKS as TESTING_CHECKS

ENABLED_CHECKS = (
DOCUMENTATION_CHECKS
+ METADATA_CORRECTNESS_CHECKS
+ PACKAGING_CHECKS
+ ASSETS_CHECKS
+ SECURITY_CHECKS
+ TESTING_CHECKS
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.


from connector_ops.utils import Connector # type: ignore
from connectors_qa.models import Check, CheckCategory, CheckResult
from pydash.collections import find # type: ignore


class TestingCheck(Check):
category = CheckCategory.TESTING


class AcceptanceTestsEnabledCheck(TestingCheck):
applies_to_connector_cloud_usage = ["medium", "high"]
applies_to_connector_types = ["source"]
name = "Medium to High Use Connectors must enable acceptance tests"
description = "Medium to High Use Connectors must enable acceptance tests via the `connectorTestSuitesOptions.suite:acceptanceTests` in their respective metadata.yaml file to ensure that the connector is working as expected."
test_suite_name = "acceptanceTests"

def does_not_have_acceptance_tests_enabled(self, connector: Connector) -> bool:
metadata = connector.metadata
connector_test_suites_options = metadata.get("connectorTestSuitesOptions", [])
acceptance_tests_suite = find(connector_test_suites_options, {"suite": self.test_suite_name})
return bool(acceptance_tests_suite) is False

def _run(self, connector: Connector) -> CheckResult:
if self.does_not_have_acceptance_tests_enabled(connector):
return self.create_check_result(
connector=connector,
passed=False,
message=f"The {self.test_suite_name} test suite must be enabled for medium/high use connectors. Please enable this test suite in the connectorTestSuitesOptions field of the metadata.yaml file.",
)
return self.create_check_result(
connector=connector,
passed=True,
message=f"{connector.cloud_usage} cloud usage connector has enabled {self.test_suite_name}.",
)


ENABLED_CHECKS = [AcceptanceTestsEnabledCheck()]
15 changes: 15 additions & 0 deletions airbyte-ci/connectors/connectors_qa/src/connectors_qa/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class CheckCategory(Enum):
ASSETS = "💼 Assets"
SECURITY = "🔒 Security"
METADATA = "📝 Metadata"
TESTING = "🧪 Testing"


class CheckStatus(Enum):
Expand Down Expand Up @@ -144,6 +145,15 @@ def applies_to_connector_support_levels(self) -> Optional[List[str]]:
"""
return None

@property
def applies_to_connector_cloud_usage(self) -> Optional[List[str]]:
"""The connector's cloud usage level that the QA check applies to
Returns:
List[str]: None if connector's cloud usage levels that the QA check applies to is not specified
"""
return None

def run(self, connector: Connector) -> CheckResult:
if not self.runs_on_released_connectors and connector.is_released:
return self.skip(
Expand Down Expand Up @@ -172,6 +182,11 @@ def run(self, connector: Connector) -> CheckResult:
connector,
f"Check does not apply to {connector.support_level} connectors",
)
if self.applies_to_connector_cloud_usage and connector.cloud_usage not in self.applies_to_connector_cloud_usage:
return self.skip(
connector,
f"Check does not apply to {connector.cloud_usage} connectors",
)
return self._run(connector)

def _run(self, connector: Connector) -> CheckResult:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ They are by no mean replacing the need for a manual review of the connector code
_Applies to the following connector types: {{ ', '.join(check.applies_to_connector_types) }}_
_Applies to the following connector languages: {{ ', '.join(check.applies_to_connector_languages) }}_
_Applies to connector with {{ ', '.join(check.applies_to_connector_support_levels) if check.applies_to_connector_support_levels else 'any' }} support level_
_Applies to connector with {{ ', '.join(check.applies_to_connector_cloud_usage) if check.applies_to_connector_cloud_usage else 'any' }} Airbyte usage level_

{{ check.description }}
{% endfor %}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.

import pytest
from connector_ops.utils import ConnectorLanguage
from connectors_qa.checks import testing
from connectors_qa.models import CheckStatus

METADATA_CASE_NO_TEST_SUITE_OPTIONS = {
"foo": "bar",
}

METADATA_CASE_EMPTY_TEST_SUITE_OPTIONS = {
"connectorTestSuitesOptions": [],
}

METADATA_CASE_NONE_TEST_SUITE_OPTIONS = {
"connectorTestSuitesOptions": None,
}

METADATA_CASE_MISSING_ACCEPTANCE_TEST_SUITE_OPTIONS = {
"connectorTestSuitesOptions": [
{
"suite": "unit",
"testSecrets": {},
},
],
}

METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS_NO_SECRETS = {
"connectorTestSuitesOptions": [
{
"suite": testing.AcceptanceTestsEnabledCheck.test_suite_name,
},
],
}

METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS_EMPTY_SECRETS = {
"connectorTestSuitesOptions": [
{
"suite": testing.AcceptanceTestsEnabledCheck.test_suite_name,
"testSecrets": {},
},
],
}

METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS_NONE_SECRETS = {
"connectorTestSuitesOptions": [
{
"suite": testing.AcceptanceTestsEnabledCheck.test_suite_name,
"testSecrets": None,
},
],
}

METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS = {
"connectorTestSuitesOptions": [
{
"suite": testing.AcceptanceTestsEnabledCheck.test_suite_name,
"testSecrets": {
"testSecret": "test"
},
},
{
"suite": "unit",
"testSecrets": {},
},
],
}

THRESHOLD_USAGE_VALUES = ["high", "medium"]
OTHER_USAGE_VALUES = ["low", "none", "unknown", None, ""]

DYNAMIC_ACCEPTANCE_TESTS_ENABLED_CASES = [
METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS,
METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS_NONE_SECRETS,
METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS_EMPTY_SECRETS,
METADATA_CASE_WITH_ACCEPTANCE_TEST_SUITE_OPTIONS_NO_SECRETS,
]

DYNAMIC_ACCEPTANCE_TESTS_DISABLED_CASES = [
METADATA_CASE_NO_TEST_SUITE_OPTIONS,
METADATA_CASE_EMPTY_TEST_SUITE_OPTIONS,
METADATA_CASE_NONE_TEST_SUITE_OPTIONS,
METADATA_CASE_MISSING_ACCEPTANCE_TEST_SUITE_OPTIONS,
]


class TestAcceptanceTestsEnabledCheck:
@pytest.mark.parametrize(
"cases_to_test, usage_values_to_test, expected_result",
[
(
DYNAMIC_ACCEPTANCE_TESTS_DISABLED_CASES + DYNAMIC_ACCEPTANCE_TESTS_ENABLED_CASES,
OTHER_USAGE_VALUES,
CheckStatus.SKIPPED
),
(
DYNAMIC_ACCEPTANCE_TESTS_ENABLED_CASES,
THRESHOLD_USAGE_VALUES,
CheckStatus.PASSED
),
(
DYNAMIC_ACCEPTANCE_TESTS_DISABLED_CASES,
THRESHOLD_USAGE_VALUES,
CheckStatus.FAILED
)
],
)
def test_check_always_passes_when_usage_threshold_is_not_met(self, mocker, cases_to_test, usage_values_to_test, expected_result):
for usage_value in usage_values_to_test:
for metadata_case in cases_to_test:
# Arrange
connector = mocker.MagicMock(cloud_usage=usage_value, metadata=metadata_case, language=ConnectorLanguage.PYTHON, connector_type="source")

# Act
result = testing.AcceptanceTestsEnabledCheck().run(connector)

# Assert
assert result.status == expected_result, f"Usage value: {usage_value}, metadata case: {metadata_case}, expected result: {expected_result}"
Loading

0 comments on commit 6db379d

Please sign in to comment.