Skip to content

Commit 2a7cb4f

Browse files
authored
Merge pull request redpanda-data#27665 from travisdowns/td-full-promotion-check
Full promotion check
2 parents 116a8c8 + fd84086 commit 2a7cb4f

3 files changed

Lines changed: 90 additions & 34 deletions

File tree

tests/rptest/clients/installpack.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
import json
22
import tarfile
33
import tempfile
4+
from typing import Any
45

56
import requests
67

8+
from rptest.util import not_none
9+
710

811
class InstallPackClient:
9-
def __init__(self, baseURLTmpl, authType, auth):
12+
def __init__(self, baseURLTmpl: str, authType: str, auth: str) -> None:
1013
self._baseURLTmpl = baseURLTmpl
1114
self._authType = authType
1215
self._auth = auth
1316

14-
def getInstallPack(self, version):
17+
def getInstallPack(self, version: str) -> Any:
1518
headers = {"Authorization": "{} {}".format(self._authType, self._auth)}
1619
with requests.get(
1720
self._baseURLTmpl.format(install_pack_ver=version),
@@ -25,4 +28,4 @@ def getInstallPack(self, version):
2528
tmp_file.flush()
2629

2730
with tarfile.open(tmp_file.name, "r:gz") as tfile:
28-
return json.load(tfile.extractfile("install-pack.json"))
31+
return json.load(not_none(tfile.extractfile("install-pack.json")))

tests/type-checking/type-check-strictness.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,6 @@
278278
"rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2_connect.py",
279279
"rptest/clients/admin/proto/redpanda/core/admin/v2/internal/debug_pb2_connect.py",
280280
"rptest/clients/admin/proto/redpanda/core/admin/v2/shadow_link_pb2_connect.py",
281-
"rptest/clients/installpack.py",
282281
"rptest/clients/offline_log_viewer.py",
283282
"rptest/clients/ping_pong.py",
284283
"rptest/clients/rpk.py",

tests/type-checking/type-check.py

Lines changed: 84 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,22 @@ class Level(Enum):
2727
def __str__(self) -> str:
2828
return self.value
2929

30+
@classmethod
31+
def _level_order(cls) -> list["Level"]:
32+
"""Return levels in order from least to most strict."""
33+
return [cls.SKIP, cls.OFF, cls.BASIC, cls.STANDARD, cls.STRICT]
34+
35+
def get_stricter_levels(self) -> list["Level"]:
36+
"""Return all levels stricter than this one."""
37+
order = self._level_order()
38+
current_index = order.index(self)
39+
return order[current_index + 1 :]
40+
41+
def get_next_level(self) -> "Level | None":
42+
"""Return the next stricter level, or None if already at strictest."""
43+
stricter_levels = self.get_stricter_levels()
44+
return stricter_levels[0] if stricter_levels else None
45+
3046

3147
@dataclass
3248
class Diagnostic:
@@ -203,6 +219,27 @@ def error_count(item: Any):
203219

204220
return dict(sorted(dmap.items(), key=error_count))
205221

222+
def _test_files_at_level(self, files: list[Path], target_level: Level) -> DiagMap:
223+
"""Test a list of files at a specific strictness level.
224+
225+
Returns a map of file paths to their diagnostics at that level.
226+
"""
227+
if target_level == Level.SKIP:
228+
# SKIP level files always pass (no diagnostics)
229+
return {file_path: [] for file_path in files}
230+
231+
# Temporarily override force level to test at the target level
232+
original_force_level = self._force_level
233+
self._force_level = target_level
234+
235+
try:
236+
# Create a temporary strictness map with only these files at the target level
237+
temp_strictness_map: dict[Level, list[Path]] = {target_level: files}
238+
return self._run_diagnostics(input_files=temp_strictness_map)
239+
finally:
240+
# Restore original force level
241+
self._force_level = original_force_level
242+
206243
def check(self):
207244
"""Run pyright on the input set, printing errors to stdout."""
208245
strictness_map = self._get_input_files()
@@ -335,55 +372,73 @@ def _check_file_promotions(
335372
336373
Returns:
337374
Tuple of (promotable_files, files_by_next_level)
338-
- promotable_files: List of (file_path, current_level, target_level)
375+
- promotable_files: List of (file_path, current_level, strictest_passing_level)
339376
- files_by_next_level: Map from next stricter level to list of (file_path, diagnostics)
340377
"""
341-
next_level_map = {
342-
Level.SKIP: Level.OFF,
343-
Level.OFF: Level.BASIC,
344-
Level.BASIC: Level.STANDARD,
345-
Level.STANDARD: Level.STRICT,
346-
Level.STRICT: None, # Already at the strictest level
347-
}
348-
349378
promotable_files: list[tuple[Path, Level, Level]] = []
350379
files_by_next_level: dict[Level, list[tuple[Path, list[Diagnostic]]]] = (
351380
defaultdict(list)
352381
)
353382

354-
# Test each group at their next strictest level
383+
# First pass: batch test files at their next level to find initially promotable files
384+
candidates_by_level: dict[Level, list[Path]] = {}
385+
355386
for current_level, files in files_by_level.items():
356-
if (next_level := next_level_map[current_level]) is None:
387+
next_level = current_level.get_next_level()
388+
if next_level is None:
357389
continue # Already at strictest level, nothing to do
358390

359391
self.vprint(
360392
f"Testing {len(files)} files at {current_level.value} level for promotion to {next_level.value}"
361393
)
362394

363-
# Temporarily override force level to test at the next strictest level
364-
original_force_level = self._force_level
365-
self._force_level = next_level
395+
# Batch test all files at their next level
396+
dmap = self._test_files_at_level(files, next_level)
366397

367-
try:
368-
# Run diagnostics on files at the next level
369-
# Create a temporary strictness map with only these files at the target level
370-
temp_strictness_map: dict[Level, list[Path]] = {next_level: files}
398+
promotable_to_next: list[Path] = []
399+
for file_path in files:
400+
diagnostics = dmap.get(file_path, [])
401+
if len(diagnostics) == 0:
402+
# File passes at next level
403+
promotable_to_next.append(file_path)
404+
else:
405+
# File fails at next level, record diagnostics
406+
files_by_next_level[next_level].append((file_path, diagnostics))
371407

372-
dmap = self._run_diagnostics(input_files=temp_strictness_map)
408+
if promotable_to_next:
409+
candidates_by_level[next_level] = promotable_to_next
410+
self.vprint(
411+
f"Found {len(promotable_to_next)} files promotable to {next_level.value}"
412+
)
373413

374-
# Check which files pass at the next level
375-
for file_path in files:
414+
# Second pass: for files that can be promoted, find their strictest possible level
415+
for current_level, candidate_files in candidates_by_level.items():
416+
for file_path in candidate_files:
417+
strictest_passing_level = current_level
418+
419+
# Test each stricter level until we find one that fails
420+
for test_level in current_level.get_stricter_levels():
421+
dmap = self._test_files_at_level([file_path], test_level)
376422
diagnostics = dmap.get(file_path, [])
423+
377424
if len(diagnostics) == 0:
378-
# File passes at next level, can be promoted
379-
promotable_files.append((file_path, current_level, next_level))
425+
# File passes at this level
426+
strictest_passing_level = test_level
380427
else:
381-
# File fails at next level, record diagnostics for that level
382-
files_by_next_level[next_level].append((file_path, diagnostics))
383-
384-
finally:
385-
# Restore original force level
386-
self._force_level = original_force_level
428+
# File fails at this level, stop testing stricter levels
429+
break
430+
431+
# Find the original level for this file
432+
original_level = None
433+
for level, files in files_by_level.items():
434+
if file_path in files:
435+
original_level = level
436+
break
437+
438+
if original_level is not None:
439+
promotable_files.append(
440+
(file_path, original_level, strictest_passing_level)
441+
)
387442

388443
return promotable_files, dict(files_by_next_level)
389444

@@ -506,7 +561,6 @@ def _update_strictness_file(
506561
print(
507562
f"Updated {len(promotable_files)} file assignments in {strictness_config_path}"
508563
)
509-
print("Re-run the command to see if further promotions are possible.")
510564

511565
def _get_input_files(self):
512566
"""Return a map of strictness level to a list of files which are in that level."""

0 commit comments

Comments
 (0)