Skip to content

Commit af975f4

Browse files
authored
Update required reviewer tests (#37724)
## What <!-- * Describe what the change is solving. Link all GitHub issues related to this change. --> * Updates the tests to be explicit about exact matching the teams that come out of the requiements file instead of just checking whether any of the expected teams were in it. Makes it easier to know which groups will be tagged for a given test. * change usage of `mock_diffed_branches` because it looks unused in the methods, and i tried removing it, but it does need to run before every test 😅 * Update test expectations accordingly to list which groups are tagged in requirements ## Can this PR be safely reverted and rolled back? <!-- * If unsure, leave it blank. --> - [x] YES 💚 - [ ] NO ❌
1 parent 914f044 commit af975f4

File tree

1 file changed

+23
-22
lines changed

1 file changed

+23
-22
lines changed

airbyte-ci/connectors/connector_ops/tests/test_required_reviewer_checks.py

+23-22
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
from connector_ops import required_reviewer_checks
1212

1313

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

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

9193
@pytest.fixture
9294
def strategic_connector_backward_compatibility_file_change_expected_team(tmp_path, strategic_connector_file):
93-
expected_teams = list(required_reviewer_checks.BACKWARD_COMPATIBILITY_REVIEWERS)
95+
expected_teams = list(
96+
required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS.union(required_reviewer_checks.BACKWARD_COMPATIBILITY_REVIEWERS)
97+
)
9498
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
9599
shutil.copyfile(strategic_connector_file, backup_path)
96100
with open(strategic_connector_file, "a") as strategic_acceptance_test_config_file:
@@ -101,7 +105,9 @@ def strategic_connector_backward_compatibility_file_change_expected_team(tmp_pat
101105

102106
@pytest.fixture
103107
def strategic_connector_bypass_reason_file_change_expected_team(tmp_path, strategic_connector_file):
104-
expected_teams = list(required_reviewer_checks.BYPASS_REASON_REVIEWERS)
108+
expected_teams = list(
109+
required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS.union(required_reviewer_checks.BYPASS_REASON_REVIEWERS)
110+
)
105111
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
106112
shutil.copyfile(strategic_connector_file, backup_path)
107113
with open(strategic_connector_file, "a") as strategic_acceptance_test_config_file:
@@ -112,7 +118,9 @@ def strategic_connector_bypass_reason_file_change_expected_team(tmp_path, strate
112118

113119
@pytest.fixture
114120
def strategic_connector_test_strictness_level_file_change_expected_team(tmp_path, strategic_connector_file):
115-
expected_teams = list(required_reviewer_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)
121+
expected_teams = list(
122+
required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS.union(required_reviewer_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)
123+
)
116124
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
117125
shutil.copyfile(strategic_connector_file, backup_path)
118126
with open(strategic_connector_file, "a") as strategic_acceptance_test_config_file:
@@ -145,7 +153,8 @@ def verify_requirements_file_was_generated(captured: str):
145153
def verify_review_requirements_file_contains_expected_teams(requirements_file_path: str, expected_teams: List):
146154
with open(requirements_file_path, "r") as requirements_file:
147155
requirements = yaml.safe_load(requirements_file)
148-
assert any([r["teams"] == expected_teams for r in requirements])
156+
all_required_teams = set().union(*(r["teams"] for r in requirements))
157+
assert all_required_teams == set(expected_teams)
149158

150159

151160
def check_review_requirements_file(capsys, expected_teams: List):
@@ -159,49 +168,41 @@ def check_review_requirements_file(capsys, expected_teams: List):
159168
verify_review_requirements_file_contains_expected_teams(requirements_file_path, expected_teams)
160169

161170

162-
def test_find_mandatory_reviewers_backward_compatibility(
163-
mock_diffed_branched, capsys, not_strategic_backward_compatibility_change_expected_team
164-
):
171+
def test_find_mandatory_reviewers_backward_compatibility(capsys, not_strategic_backward_compatibility_change_expected_team):
165172
check_review_requirements_file(capsys, not_strategic_backward_compatibility_change_expected_team)
166173

167174

168-
def test_find_mandatory_reviewers_test_strictness_level(
169-
mock_diffed_branched, capsys, not_strategic_test_strictness_level_change_expected_team
170-
):
175+
def test_find_mandatory_reviewers_test_strictness_level(capsys, not_strategic_test_strictness_level_change_expected_team):
171176
check_review_requirements_file(capsys, not_strategic_test_strictness_level_change_expected_team)
172177

173178

174-
def test_find_mandatory_reviewers_not_strategic_bypass_reason(
175-
mock_diffed_branched, capsys, not_strategic_bypass_reason_file_change_expected_team
176-
):
179+
def test_find_mandatory_reviewers_not_strategic_bypass_reason(capsys, not_strategic_bypass_reason_file_change_expected_team):
177180
check_review_requirements_file(capsys, not_strategic_bypass_reason_file_change_expected_team)
178181

179182

180-
def test_find_mandatory_reviewers_ga(mock_diffed_branched, capsys, strategic_connector_file_change_expected_team):
183+
def test_find_mandatory_reviewers_ga(capsys, strategic_connector_file_change_expected_team):
181184
check_review_requirements_file(capsys, strategic_connector_file_change_expected_team)
182185

183186

184187
def test_find_mandatory_reviewers_strategic_backward_compatibility(
185-
mock_diffed_branched, capsys, strategic_connector_backward_compatibility_file_change_expected_team
188+
capsys, strategic_connector_backward_compatibility_file_change_expected_team
186189
):
187190
check_review_requirements_file(capsys, strategic_connector_backward_compatibility_file_change_expected_team)
188191

189192

190-
def test_find_mandatory_reviewers_strategic_bypass_reason(
191-
mock_diffed_branched, capsys, strategic_connector_bypass_reason_file_change_expected_team
192-
):
193+
def test_find_mandatory_reviewers_strategic_bypass_reason(capsys, strategic_connector_bypass_reason_file_change_expected_team):
193194
check_review_requirements_file(capsys, strategic_connector_bypass_reason_file_change_expected_team)
194195

195196

196197
def test_find_mandatory_reviewers_strategic_test_strictness_level(
197-
mock_diffed_branched, capsys, strategic_connector_test_strictness_level_file_change_expected_team
198+
capsys, strategic_connector_test_strictness_level_file_change_expected_team
198199
):
199200
check_review_requirements_file(capsys, strategic_connector_test_strictness_level_file_change_expected_team)
200201

201202

202-
def test_find_mandatory_reviewers_breaking_change_release(mock_diffed_branched, capsys, test_breaking_change_release_expected_team):
203+
def test_find_mandatory_reviewers_breaking_change_release(capsys, test_breaking_change_release_expected_team):
203204
check_review_requirements_file(capsys, test_breaking_change_release_expected_team)
204205

205206

206-
def test_find_mandatory_reviewers_no_tracked_changed(mock_diffed_branched, capsys, not_strategic_not_tracked_change_expected_team):
207+
def test_find_mandatory_reviewers_no_tracked_changed(capsys, not_strategic_not_tracked_change_expected_team):
207208
check_review_requirements_file(capsys, not_strategic_not_tracked_change_expected_team)

0 commit comments

Comments
 (0)