Skip to content

Update required reviewer tests #37724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 1, 2024
Merged

Update required reviewer tests #37724

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2df705a
update tests
erohmensing Apr 30, 2024
ed75624
format
erohmensing Apr 30, 2024
7998407
remove unnecessary fixture
erohmensing Apr 30, 2024
343f741
format
erohmensing Apr 30, 2024
c27ad14
ok maybe mock diffed branch is actually important
erohmensing Apr 30, 2024
12bde21
clarify
erohmensing Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 23 additions & 22 deletions airbyte-ci/connectors/connector_ops/tests/test_required_reviewer_checks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from connector_ops import required_reviewer_checks


@pytest.fixture
# This fixture ensure that the remote CI works the same way local CI does
@pytest.fixture(autouse=True)
def mock_diffed_branched(mocker):
airbyte_repo = git.Repo(search_parent_directories=True)
mocker.patch.object(required_reviewer_checks.utils, "DIFFED_BRANCH", airbyte_repo.active_branch)
Expand Down Expand Up @@ -57,6 +58,7 @@ def not_strategic_test_strictness_level_change_expected_team(tmp_path, pokeapi_a

@pytest.fixture
def not_strategic_bypass_reason_file_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path):
# The bypass reason logic explicitly only checks for bypass reasons on strategic connectors
expected_teams = []
backup_path = tmp_path / "non_strategic_acceptance_test_config.backup"
shutil.copyfile(pokeapi_acceptance_test_config_path, backup_path)
Expand Down Expand Up @@ -90,7 +92,9 @@ def strategic_connector_file_change_expected_team(tmp_path, strategic_connector_

@pytest.fixture
def strategic_connector_backward_compatibility_file_change_expected_team(tmp_path, strategic_connector_file):
expected_teams = list(required_reviewer_checks.BACKWARD_COMPATIBILITY_REVIEWERS)
expected_teams = list(
required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS.union(required_reviewer_checks.BACKWARD_COMPATIBILITY_REVIEWERS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

)
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
shutil.copyfile(strategic_connector_file, backup_path)
with open(strategic_connector_file, "a") as strategic_acceptance_test_config_file:
Expand All @@ -101,7 +105,9 @@ def strategic_connector_backward_compatibility_file_change_expected_team(tmp_pat

@pytest.fixture
def strategic_connector_bypass_reason_file_change_expected_team(tmp_path, strategic_connector_file):
expected_teams = list(required_reviewer_checks.BYPASS_REASON_REVIEWERS)
expected_teams = list(
required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS.union(required_reviewer_checks.BYPASS_REASON_REVIEWERS)
)
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
shutil.copyfile(strategic_connector_file, backup_path)
with open(strategic_connector_file, "a") as strategic_acceptance_test_config_file:
Expand All @@ -112,7 +118,9 @@ def strategic_connector_bypass_reason_file_change_expected_team(tmp_path, strate

@pytest.fixture
def strategic_connector_test_strictness_level_file_change_expected_team(tmp_path, strategic_connector_file):
expected_teams = list(required_reviewer_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)
expected_teams = list(
required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS.union(required_reviewer_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)
)
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
shutil.copyfile(strategic_connector_file, backup_path)
with open(strategic_connector_file, "a") as strategic_acceptance_test_config_file:
Expand Down Expand Up @@ -145,7 +153,8 @@ def verify_requirements_file_was_generated(captured: str):
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 any([r["teams"] == expected_teams for r in requirements])
all_required_teams = set().union(*(r["teams"] for r in requirements))
assert all_required_teams == set(expected_teams)


def check_review_requirements_file(capsys, expected_teams: List):
Expand All @@ -159,49 +168,41 @@ def check_review_requirements_file(capsys, expected_teams: List):
verify_review_requirements_file_contains_expected_teams(requirements_file_path, expected_teams)


def test_find_mandatory_reviewers_backward_compatibility(
mock_diffed_branched, capsys, not_strategic_backward_compatibility_change_expected_team
):
def test_find_mandatory_reviewers_backward_compatibility(capsys, not_strategic_backward_compatibility_change_expected_team):
check_review_requirements_file(capsys, not_strategic_backward_compatibility_change_expected_team)


def test_find_mandatory_reviewers_test_strictness_level(
mock_diffed_branched, capsys, not_strategic_test_strictness_level_change_expected_team
):
def test_find_mandatory_reviewers_test_strictness_level(capsys, not_strategic_test_strictness_level_change_expected_team):
check_review_requirements_file(capsys, not_strategic_test_strictness_level_change_expected_team)


def test_find_mandatory_reviewers_not_strategic_bypass_reason(
mock_diffed_branched, capsys, not_strategic_bypass_reason_file_change_expected_team
):
def test_find_mandatory_reviewers_not_strategic_bypass_reason(capsys, not_strategic_bypass_reason_file_change_expected_team):
check_review_requirements_file(capsys, not_strategic_bypass_reason_file_change_expected_team)


def test_find_mandatory_reviewers_ga(mock_diffed_branched, capsys, strategic_connector_file_change_expected_team):
def test_find_mandatory_reviewers_ga(capsys, strategic_connector_file_change_expected_team):
check_review_requirements_file(capsys, strategic_connector_file_change_expected_team)


def test_find_mandatory_reviewers_strategic_backward_compatibility(
mock_diffed_branched, capsys, strategic_connector_backward_compatibility_file_change_expected_team
capsys, strategic_connector_backward_compatibility_file_change_expected_team
):
check_review_requirements_file(capsys, strategic_connector_backward_compatibility_file_change_expected_team)


def test_find_mandatory_reviewers_strategic_bypass_reason(
mock_diffed_branched, capsys, strategic_connector_bypass_reason_file_change_expected_team
):
def test_find_mandatory_reviewers_strategic_bypass_reason(capsys, strategic_connector_bypass_reason_file_change_expected_team):
check_review_requirements_file(capsys, strategic_connector_bypass_reason_file_change_expected_team)


def test_find_mandatory_reviewers_strategic_test_strictness_level(
mock_diffed_branched, capsys, strategic_connector_test_strictness_level_file_change_expected_team
capsys, strategic_connector_test_strictness_level_file_change_expected_team
):
check_review_requirements_file(capsys, strategic_connector_test_strictness_level_file_change_expected_team)


def test_find_mandatory_reviewers_breaking_change_release(mock_diffed_branched, capsys, test_breaking_change_release_expected_team):
def test_find_mandatory_reviewers_breaking_change_release(capsys, test_breaking_change_release_expected_team):
check_review_requirements_file(capsys, test_breaking_change_release_expected_team)


def test_find_mandatory_reviewers_no_tracked_changed(mock_diffed_branched, capsys, not_strategic_not_tracked_change_expected_team):
def test_find_mandatory_reviewers_no_tracked_changed(capsys, not_strategic_not_tracked_change_expected_team):
check_review_requirements_file(capsys, not_strategic_not_tracked_change_expected_team)
Loading