From b454ffa90fa58a2c33b245a775116d67af8a20d5 Mon Sep 17 00:00:00 2001 From: erohmensing Date: Fri, 3 Mar 2023 12:48:58 -0600 Subject: [PATCH 1/8] Add instructions to install reqs and run tests locally --- tools/ci_connector_ops/README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 From 2136b153390bd63f182cea2a880206aa892d7e26 Mon Sep 17 00:00:00 2001 From: erohmensing Date: Fri, 3 Mar 2023 15:24:40 -0600 Subject: [PATCH 2/8] add passing tests with old behavior --- .../acceptance_test_config_checks.py | 1 + .../test_acceptance_test_config_checks.py | 32 +++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) 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..5bca2956ffbc 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 @@ -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" 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..f69989be9d53 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 @@ -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_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("not_tracked:") + 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) @@ -75,6 +84,16 @@ def ga_connector_backward_compatibility_file_change(tmp_path, ga_connector_file) yield expected_teams shutil.copyfile(backup_path, ga_connector_file) +@pytest.fixture +def ga_connector_bypass_reason_file_change(tmp_path, ga_connector_file): + expected_teams = list(acceptance_test_config_checks.GA_CONNECTOR_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(tmp_path, ga_connector_file): expected_teams = [{"any-of": list(acceptance_test_config_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)}] @@ -96,18 +115,25 @@ def check_review_requirements_file_contains_expected_teams(capsys, expected_team 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 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) - +def test_find_mandatory_reviewers_bypass_reason(mock_diffed_branched, capsys, not_ga_bypass_reason_change_expected_team): + # check_review_requirements_file_contains_expected_teams(capsys, not_ga_bypass_reason_change_expected_team) + acceptance_test_config_checks.write_review_requirements_file() + captured = capsys.readouterr() + assert captured.out.split("\n")[0].split("=")[-1] == "false" + 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) 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): + check_review_requirements_file_contains_expected_teams(capsys, ga_connector_bypass_reason_file_change) + 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) From d41077f9b86a2c20dbfdca029c57ae2ed488feec Mon Sep 17 00:00:00 2001 From: erohmensing Date: Fri, 3 Mar 2023 15:54:20 -0600 Subject: [PATCH 3/8] Make it like test strictness level --- .../ci_connector_ops/acceptance_test_config_checks.py | 3 +++ .../tests/test_acceptance_test_config_checks.py | 11 ++++------- 2 files changed, 7 insertions(+), 7 deletions(-) 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 5bca2956ffbc..c02edf4d2283 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 @@ -53,10 +53,13 @@ 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") + bypass_reason_changes = utils.get_changed_acceptance_test_config(diff_regex="bypass_reason") 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 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/tests/test_acceptance_test_config_checks.py b/tools/ci_connector_ops/tests/test_acceptance_test_config_checks.py index f69989be9d53..bf2488e2e9a9 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 @@ -46,7 +46,7 @@ def not_ga_test_strictness_level_change_expected_team(tmp_path, pokeapi_acceptan @pytest.fixture def not_ga_bypass_reason_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path): - expected_teams = [] + expected_teams = [{"any-of": list(acceptance_test_config_checks.GA_BYPASS_REASON_REVIEWERS)}] 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: @@ -86,7 +86,7 @@ def ga_connector_backward_compatibility_file_change(tmp_path, ga_connector_file) @pytest.fixture def ga_connector_bypass_reason_file_change(tmp_path, ga_connector_file): - expected_teams = list(acceptance_test_config_checks.GA_CONNECTOR_REVIEWERS) + 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: @@ -119,11 +119,8 @@ def test_find_mandatory_reviewers_backward_compatibility(mock_diffed_branched, c 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) -def test_find_mandatory_reviewers_bypass_reason(mock_diffed_branched, capsys, not_ga_bypass_reason_change_expected_team): - # check_review_requirements_file_contains_expected_teams(capsys, not_ga_bypass_reason_change_expected_team) - acceptance_test_config_checks.write_review_requirements_file() - captured = capsys.readouterr() - assert captured.out.split("\n")[0].split("=")[-1] == "false" +def test_find_mandatory_reviewers_ga_bypass_reason(mock_diffed_branched, capsys, not_ga_bypass_reason_change_expected_team): + check_review_requirements_file_contains_expected_teams(capsys, not_ga_bypass_reason_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) From 71429d5a4b03208fcb037c3585dd78b6e783b15d Mon Sep 17 00:00:00 2001 From: erohmensing Date: Fri, 3 Mar 2023 16:02:11 -0600 Subject: [PATCH 4/8] Remove expected teams from non-tracked change test since they are not asserted against --- .../tests/test_acceptance_test_config_checks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 bf2488e2e9a9..7894ad6a1681 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 @@ -56,12 +56,11 @@ def not_ga_bypass_reason_change_expected_team(tmp_path, pokeapi_acceptance_test_ @pytest.fixture def not_ga_not_tracked_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("not_tracked") - yield expected_teams + yield shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path) @pytest.fixture From 6783d8901075845fdafb41e910d55fd4975958cd Mon Sep 17 00:00:00 2001 From: erohmensing Date: Fri, 3 Mar 2023 16:57:48 -0600 Subject: [PATCH 5/8] Make changed GA connector list object same object format as regex checker to be able to compare them. Fix tests --- .../acceptance_test_config_checks.py | 22 ++++++++++++++----- .../test_acceptance_test_config_checks.py | 18 ++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) 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 c02edf4d2283..af5715376635 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 @@ -40,25 +40,35 @@ 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[str]: """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[str]: Set of connector names e.g {"source-github"}: 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.technical_name for connector in changed_connectors if connector.release_stage == "generally_available"} + +def get_ga_bypass_reason_changes() -> Set[str]: + """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") - bypass_reason_changes = utils.get_changed_acceptance_test_config(diff_regex="bypass_reason") + 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 bypass_reason_changes: + if ga_bypass_reason_changes: return [{"any-of": list(GA_BYPASS_REASON_REVIEWERS)}] if ga_connector_changes: return list(GA_CONNECTOR_REVIEWERS) 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 7894ad6a1681..47080688f55a 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 @@ -46,12 +46,11 @@ def not_ga_test_strictness_level_change_expected_team(tmp_path, pokeapi_acceptan @pytest.fixture def not_ga_bypass_reason_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path): - expected_teams = [{"any-of": list(acceptance_test_config_checks.GA_BYPASS_REASON_REVIEWERS)}] 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("not_tracked:") - yield expected_teams + acceptance_test_config_file.write("bypass_reason:") + yield shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path) @pytest.fixture @@ -112,14 +111,19 @@ def check_review_requirements_file_contains_expected_teams(capsys, expected_team requirements = yaml.safe_load(requirements_file) assert requirements[0]["teams"] == expected_teams +def verify_no_requirements_file_was_generated(capsys): + acceptance_test_config_checks.write_review_requirements_file() + captured = capsys.readouterr() + assert captured.out.split("\n")[0].split("=")[-1] == "false" + 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 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) -def test_find_mandatory_reviewers_ga_bypass_reason(mock_diffed_branched, capsys, not_ga_bypass_reason_change_expected_team): - check_review_requirements_file_contains_expected_teams(capsys, not_ga_bypass_reason_change_expected_team) +def test_find_mandatory_reviewers_not_ga_bypass_reason(mock_diffed_branched, capsys, not_ga_bypass_reason_change_expected_team): + verify_no_requirements_file_was_generated(capsys) 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) @@ -134,6 +138,4 @@ def test_find_mandatory_reviewers_ga_test_strictness_level(mock_diffed_branched, check_review_requirements_file_contains_expected_teams(capsys, ga_connector_test_strictness_level_file_change) 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" + verify_no_requirements_file_was_generated(capsys) From 21ed1064317937d354ae8eca2478db620a5dbee8 Mon Sep 17 00:00:00 2001 From: erohmensing Date: Fri, 3 Mar 2023 17:15:29 -0600 Subject: [PATCH 6/8] Refactor and align naming --- .../test_acceptance_test_config_checks.py | 59 +++++++++++-------- 1 file changed, 34 insertions(+), 25 deletions(-) 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 47080688f55a..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 @@ -45,21 +45,23 @@ def not_ga_test_strictness_level_change_expected_team(tmp_path, pokeapi_acceptan shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path) @pytest.fixture -def not_ga_bypass_reason_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path): +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 + 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 = [] 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("not_tracked") - yield + yield expected_teams shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path) @pytest.fixture @@ -73,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) @@ -83,7 +85,7 @@ def ga_connector_backward_compatibility_file_change(tmp_path, ga_connector_file) shutil.copyfile(backup_path, ga_connector_file) @pytest.fixture -def ga_connector_bypass_reason_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) @@ -93,7 +95,7 @@ def ga_connector_bypass_reason_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_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) @@ -102,40 +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 verify_no_requirements_file_was_generated(capsys): +def check_review_requirements_file(capsys, expected_teams: List): acceptance_test_config_checks.write_review_requirements_file() captured = capsys.readouterr() - assert captured.out.split("\n")[0].split("=")[-1] == "false" + 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_contains_expected_teams(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_change_expected_team): - verify_no_requirements_file_was_generated(capsys) +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): - check_review_requirements_file_contains_expected_teams(capsys, ga_connector_backward_compatibility_file_change) +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_bypass_reason(mock_diffed_branched, capsys, ga_connector_bypass_reason_file_change): - check_review_requirements_file_contains_expected_teams(capsys, ga_connector_bypass_reason_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): - verify_no_requirements_file_was_generated(capsys) + check_review_requirements_file(capsys, not_ga_not_tracked_change_expected_team) From 4deef062c01167837cfd5c21f3601694ebb68e3e Mon Sep 17 00:00:00 2001 From: erohmensing Date: Mon, 6 Mar 2023 15:08:34 -0600 Subject: [PATCH 7/8] Update all find methods to return Connector objects --- .../ci_connector_ops/acceptance_test_config_checks.py | 8 ++++---- tools/ci_connector_ops/ci_connector_ops/utils.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) 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 af5715376635..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 @@ -40,16 +40,16 @@ 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() -> Set[str]: +def find_changed_ga_connectors() -> Set[utils.Connector]: """Find GA connectors modified on the current branch. Returns: - Set[str]: Set of connector names e.g {"source-github"}: The set 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.technical_name 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[str]: +def get_ga_bypass_reason_changes() -> Set[utils.Connector]: """Find GA connectors that have modified bypass_reasons. Returns: 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 From 89575cd5e3051a471e3fc5c707ccd546c5d6c7ff Mon Sep 17 00:00:00 2001 From: erohmensing Date: Mon, 6 Mar 2023 15:10:10 -0600 Subject: [PATCH 8/8] Bump setup.py --- tools/ci_connector_ops/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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",