diff --git a/tools/ci_connector_ops/README.md b/tools/ci_connector_ops/README.md index 4bde2fb76e1f..f2dfd02abead 100644 --- a/tools/ci_connector_ops/README.md +++ b/tools/ci_connector_ops/README.md @@ -19,3 +19,17 @@ pip install -e . # assuming you are in the ./tools/ci_connector_ops directory ``` pip will make binaries for all the commands in setup.py, so you can run `allowed-hosts-checks` directly from the virtual-env + +## Testing Locally + +To install requirements to run unit tests, use: + +``` +pip install -e ".[tests]" +``` + +Unit tests are currently configured to be run from the base `airbyte` directory. You can run the tests from that directory with the following command: + +``` +pytest -s tools/ci_connector_ops/tests +``` \ No newline at end of file diff --git a/tools/ci_connector_ops/ci_connector_ops/acceptance_test_config_checks.py b/tools/ci_connector_ops/ci_connector_ops/acceptance_test_config_checks.py index 8839005d1307..a9d1a042204c 100644 --- a/tools/ci_connector_ops/ci_connector_ops/acceptance_test_config_checks.py +++ b/tools/ci_connector_ops/ci_connector_ops/acceptance_test_config_checks.py @@ -4,7 +4,7 @@ import logging import sys -from typing import List, Dict, Union +from typing import List, Dict, Union, Set import yaml from ci_connector_ops import utils @@ -12,6 +12,7 @@ RELEASE_STAGE_TO_STRICTNESS_LEVEL_MAPPING = {"generally_available": "high"} BACKWARD_COMPATIBILITY_REVIEWERS = {"connector-operations", "connector-extensibility"} TEST_STRICTNESS_LEVEL_REVIEWERS = {"connector-operations"} +GA_BYPASS_REASON_REVIEWERS = {"connector-operations"} GA_CONNECTOR_REVIEWERS = {"gl-python"} REVIEW_REQUIREMENTS_FILE_PATH = ".github/connector_org_review_requirements.yaml" @@ -39,23 +40,36 @@ def find_connectors_with_bad_strictness_level() -> List[utils.Connector]: connectors_with_bad_strictness_level.append(connector) return connectors_with_bad_strictness_level -def find_changed_ga_connectors() -> List[utils.Connector]: +def find_changed_ga_connectors() -> Set[utils.Connector]: """Find GA connectors modified on the current branch. Returns: - List[utils.Connector]: The list of GA connectors that were modified on the current branch. + Set[utils.Connector]: The set of GA connectors that were modified on the current branch. """ changed_connectors = utils.get_changed_connectors() - return [connector for connector in changed_connectors if connector.release_stage == "generally_available"] + return {connector for connector in changed_connectors if connector.release_stage == "generally_available"} + +def get_ga_bypass_reason_changes() -> Set[utils.Connector]: + """Find GA connectors that have modified bypass_reasons. + + Returns: + Set[str]: Set of connector names e.g {"source-github"}: The set of GA connectors that have changed bypass_reasons. + """ + bypass_reason_changes = utils.get_changed_acceptance_test_config(diff_regex="bypass_reason") + return bypass_reason_changes.intersection(find_changed_ga_connectors()) def find_mandatory_reviewers() -> List[Union[str, Dict[str, List]]]: ga_connector_changes = find_changed_ga_connectors() backward_compatibility_changes = utils.get_changed_acceptance_test_config(diff_regex="disable_for_version") test_strictness_level_changes = utils.get_changed_acceptance_test_config(diff_regex="test_strictness_level") + ga_bypass_reason_changes = get_ga_bypass_reason_changes() + if backward_compatibility_changes: return [{"any-of": list(BACKWARD_COMPATIBILITY_REVIEWERS)}] if test_strictness_level_changes: return [{"any-of": list(TEST_STRICTNESS_LEVEL_REVIEWERS)}] + if ga_bypass_reason_changes: + return [{"any-of": list(GA_BYPASS_REASON_REVIEWERS)}] if ga_connector_changes: return list(GA_CONNECTOR_REVIEWERS) return [] diff --git a/tools/ci_connector_ops/ci_connector_ops/utils.py b/tools/ci_connector_ops/ci_connector_ops/utils.py index a7b14d5fcdb8..98d6a28f8340 100644 --- a/tools/ci_connector_ops/ci_connector_ops/utils.py +++ b/tools/ci_connector_ops/ci_connector_ops/utils.py @@ -41,13 +41,13 @@ def get_connector_name_from_path(path): return path.split("/")[2] def get_changed_acceptance_test_config(diff_regex: Optional[str]=None) -> Set[str]: - """Retrieve a list of connector names for which the acceptance_test_config file was changed in the current branch (compared to master). + """Retrieve the set of connectors for which the acceptance_test_config file was changed in the current branch (compared to master). Args: diff_regex (str): Find the edited files that contain the following regex in their change. Returns: - Set[str]: Set of connector names e.g {"source-pokeapi"} + Set[Connector]: Set of connectors that were changed """ if diff_regex is None: diff_command_args = ("--name-only", DIFFED_BRANCH) @@ -59,7 +59,7 @@ def get_changed_acceptance_test_config(diff_regex: Optional[str]=None) -> Set[st for file_path in AIRBYTE_REPO.git.diff(*diff_command_args).split("\n") if file_path.startswith(SOURCE_CONNECTOR_PATH_PREFIX) and file_path.endswith(ACCEPTANCE_TEST_CONFIG_FILE_NAME) } - return {get_connector_name_from_path(changed_file) for changed_file in changed_acceptance_test_config_paths} + return {Connector(get_connector_name_from_path(changed_file)) for changed_file in changed_acceptance_test_config_paths} @dataclass(frozen=True) @@ -152,7 +152,7 @@ def __repr__(self) -> str: return self.technical_name def get_changed_connectors() -> Set[Connector]: - """Retrieve a list of Connectors that were changed in the current branch (compared to master). + """Retrieve a set of Connectors that were changed in the current branch (compared to master). """ changed_source_connector_files = { file_path diff --git a/tools/ci_connector_ops/setup.py b/tools/ci_connector_ops/setup.py index a6cdda7d0e0d..f181d67c2038 100644 --- a/tools/ci_connector_ops/setup.py +++ b/tools/ci_connector_ops/setup.py @@ -24,7 +24,7 @@ ] setup( - version="0.1.15", + version="0.1.16", name="ci_connector_ops", description="Packaged maintained by the connector operations team to perform CI for connectors", author="Airbyte", diff --git a/tools/ci_connector_ops/tests/test_acceptance_test_config_checks.py b/tools/ci_connector_ops/tests/test_acceptance_test_config_checks.py index c03504b3ed27..4b3fc7e96415 100644 --- a/tools/ci_connector_ops/tests/test_acceptance_test_config_checks.py +++ b/tools/ci_connector_ops/tests/test_acceptance_test_config_checks.py @@ -3,7 +3,7 @@ # import shutil -from typing import List +from typing import List, Optional import yaml import pytest @@ -44,6 +44,16 @@ def not_ga_test_strictness_level_change_expected_team(tmp_path, pokeapi_acceptan yield expected_teams shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path) +@pytest.fixture +def not_ga_bypass_reason_file_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path): + expected_teams = [] + backup_path = tmp_path / "non_ga_acceptance_test_config.backup" + shutil.copyfile(pokeapi_acceptance_test_config_path, backup_path) + with open(pokeapi_acceptance_test_config_path, "a") as acceptance_test_config_file: + acceptance_test_config_file.write("bypass_reason:") + yield expected_teams + shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path) + @pytest.fixture def not_ga_not_tracked_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path): expected_teams = [] @@ -54,7 +64,6 @@ def not_ga_not_tracked_change_expected_team(tmp_path, pokeapi_acceptance_test_co yield expected_teams shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path) - @pytest.fixture def ga_connector_file_change_expected_team(tmp_path, ga_connector_file): expected_teams = list(acceptance_test_config_checks.GA_CONNECTOR_REVIEWERS) @@ -66,7 +75,7 @@ def ga_connector_file_change_expected_team(tmp_path, ga_connector_file): shutil.copyfile(backup_path, ga_connector_file) @pytest.fixture -def ga_connector_backward_compatibility_file_change(tmp_path, ga_connector_file): +def ga_connector_backward_compatibility_file_change_expected_team(tmp_path, ga_connector_file): expected_teams = [{"any-of": list(acceptance_test_config_checks.BACKWARD_COMPATIBILITY_REVIEWERS)}] backup_path = tmp_path / "ga_acceptance_test_config.backup" shutil.copyfile(ga_connector_file, backup_path) @@ -76,7 +85,17 @@ def ga_connector_backward_compatibility_file_change(tmp_path, ga_connector_file) shutil.copyfile(backup_path, ga_connector_file) @pytest.fixture -def ga_connector_test_strictness_level_file_change(tmp_path, ga_connector_file): +def ga_connector_bypass_reason_file_change_expected_team(tmp_path, ga_connector_file): + expected_teams = [{"any-of": list(acceptance_test_config_checks.GA_BYPASS_REASON_REVIEWERS)}] + backup_path = tmp_path / "ga_acceptance_test_config.backup" + shutil.copyfile(ga_connector_file, backup_path) + with open(ga_connector_file, "a") as ga_acceptance_test_config_file: + ga_acceptance_test_config_file.write("bypass_reason:") + yield expected_teams + shutil.copyfile(backup_path, ga_connector_file) + +@pytest.fixture +def ga_connector_test_strictness_level_file_change_expected_team(tmp_path, ga_connector_file): expected_teams = [{"any-of": list(acceptance_test_config_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)}] backup_path = tmp_path / "ga_acceptance_test_config.backup" shutil.copyfile(ga_connector_file, backup_path) @@ -85,33 +104,47 @@ def ga_connector_test_strictness_level_file_change(tmp_path, ga_connector_file): yield expected_teams shutil.copyfile(backup_path, ga_connector_file) -def check_review_requirements_file_contains_expected_teams(capsys, expected_teams: List): - acceptance_test_config_checks.write_review_requirements_file() - captured = capsys.readouterr() +def verify_no_requirements_file_was_generated(captured: str): + assert captured.out.split("\n")[0].split("=")[-1] == "false" + +def verify_requirements_file_was_generated(captured: str): assert captured.out.split("\n")[0].split("=")[-1] == "true" - requirements_file_path = acceptance_test_config_checks.REVIEW_REQUIREMENTS_FILE_PATH + +def verify_review_requirements_file_contains_expected_teams(requirements_file_path: str, expected_teams: List): with open(requirements_file_path, "r") as requirements_file: requirements = yaml.safe_load(requirements_file) assert requirements[0]["teams"] == expected_teams -def test_find_mandatory_reviewers_backward_compatibility(mock_diffed_branched, capsys, not_ga_backward_compatibility_change_expected_team): - check_review_requirements_file_contains_expected_teams(capsys, not_ga_backward_compatibility_change_expected_team) +def check_review_requirements_file(capsys, expected_teams: List): + acceptance_test_config_checks.write_review_requirements_file() + captured = capsys.readouterr() + if not expected_teams: + verify_no_requirements_file_was_generated(captured) + else: + verify_requirements_file_was_generated(captured) + requirements_file_path = acceptance_test_config_checks.REVIEW_REQUIREMENTS_FILE_PATH + verify_review_requirements_file_contains_expected_teams(requirements_file_path, expected_teams) +def test_find_mandatory_reviewers_backward_compatibility(mock_diffed_branched, capsys, not_ga_backward_compatibility_change_expected_team): + check_review_requirements_file(capsys, not_ga_backward_compatibility_change_expected_team) def test_find_mandatory_reviewers_test_strictness_level(mock_diffed_branched, capsys, not_ga_test_strictness_level_change_expected_team): - check_review_requirements_file_contains_expected_teams(capsys, not_ga_test_strictness_level_change_expected_team) + check_review_requirements_file(capsys, not_ga_test_strictness_level_change_expected_team) + +def test_find_mandatory_reviewers_not_ga_bypass_reason(mock_diffed_branched, capsys, not_ga_bypass_reason_file_change_expected_team): + check_review_requirements_file(capsys, not_ga_bypass_reason_file_change_expected_team) - def test_find_mandatory_reviewers_ga(mock_diffed_branched, capsys, ga_connector_file_change_expected_team): - check_review_requirements_file_contains_expected_teams(capsys, ga_connector_file_change_expected_team) + check_review_requirements_file(capsys, ga_connector_file_change_expected_team) + +def test_find_mandatory_reviewers_ga_backward_compatibility(mock_diffed_branched, capsys, ga_connector_backward_compatibility_file_change_expected_team): + check_review_requirements_file(capsys, ga_connector_backward_compatibility_file_change_expected_team) -def test_find_mandatory_reviewers_ga_backward_compatibility(mock_diffed_branched, capsys, ga_connector_backward_compatibility_file_change): - check_review_requirements_file_contains_expected_teams(capsys, ga_connector_backward_compatibility_file_change) +def test_find_mandatory_reviewers_ga_bypass_reason(mock_diffed_branched, capsys, ga_connector_bypass_reason_file_change_expected_team): + check_review_requirements_file(capsys, ga_connector_bypass_reason_file_change_expected_team) -def test_find_mandatory_reviewers_ga_test_strictness_level(mock_diffed_branched, capsys, ga_connector_test_strictness_level_file_change): - check_review_requirements_file_contains_expected_teams(capsys, ga_connector_test_strictness_level_file_change) +def test_find_mandatory_reviewers_ga_test_strictness_level(mock_diffed_branched, capsys, ga_connector_test_strictness_level_file_change_expected_team): + check_review_requirements_file(capsys, ga_connector_test_strictness_level_file_change_expected_team) def test_find_mandatory_reviewers_no_tracked_changed(mock_diffed_branched, capsys, not_ga_not_tracked_change_expected_team): - acceptance_test_config_checks.write_review_requirements_file() - captured = capsys.readouterr() - assert captured.out.split("\n")[0].split("=")[-1] == "false" + check_review_requirements_file(capsys, not_ga_not_tracked_change_expected_team)