Improve upfront validation in the Static Knockout Scheduler

sr/comp/knockout_scheduler/static_scheduler.py

@@ -15,6 +15,18 @@
 StaticMatchTeamReference = NewType('StaticMatchTeamReference', str)
 
 
+class InvalidSeedError(ValueError):
+    pass
+
+
+class InvalidReferenceError(ValueError):
+    pass
+
+
+class WrongNumberOfTeamsError(ValueError):
+    pass
+
+
 class StaticMatchInfo(TypedDict):
     arena: ArenaName
     start_time: datetime.datetime
@@ -42,15 +54,16 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
 
         # Collect a list of the teams eligible for the knockouts, in seeded order.
+        # TODO: deduplicate this vs similar logic in the automated scheduler.
         last_league_match_num = self.schedule.n_matches()
-        self._knockout_seeds = self._get_non_dropped_out_teams(
+        teams = self._get_non_dropped_out_teams(
             MatchNumber(last_league_match_num),
         )
-
-    def get_team(self, team_ref: StaticMatchTeamReference | None) -> TLA | None:
         if not self._played_all_league_matches():
-            return UNKNOWABLE_TEAM
+            teams = [UNKNOWABLE_TEAM] * len(teams)
+        self._knockout_seeds = teams
 
+    def get_team(self, team_ref: StaticMatchTeamReference | None) -> TLA | None:
         if team_ref is None:
             return None
 
@@ -59,35 +72,39 @@ def get_team(self, team_ref: StaticMatchTeamReference | None) -> TLA | None:
             pos = int(team_ref[1:])
             # seed numbers are 1 based
             if pos < 1:
-                raise ValueError(f"Invalid seed {team_ref!r} (seed numbers start at 1)")
+                raise InvalidSeedError(f"Invalid seed {team_ref!r} (seed numbers start at 1)")
             pos -= 1
             try:
                 return self._knockout_seeds[pos]
             except IndexError:
-                raise ValueError(
+                raise InvalidSeedError(
                     "Cannot reference seed {}, there are only {} eligible teams!".format(
                         team_ref,
                         len(self._knockout_seeds),
                     ),
                 ) from None
 
         # get a position from a match
-        assert len(team_ref) == 3
-        round_num, match_num, pos = (int(x) for x in team_ref)
+        try:
+            round_num, match_num, pos = (int(x) for x in team_ref)
+        except ValueError:
+            raise InvalidReferenceError(
+                f"Match references must be three digits, not {team_ref!r}",
+            ) from None
 
         try:
             match = self.knockout_rounds[round_num][match_num]
         except IndexError:
-            raise ValueError(
-                f"Reference '{team_ref}' to unscheduled match!",
+            raise InvalidReferenceError(
+                f"Reference {team_ref!r} to unscheduled match!",
             ) from None
 
         try:
             ranking = self.get_ranking(match)
             return ranking[pos]
         except IndexError:
-            raise ValueError(
-                f"Reference '{team_ref}' to invalid ranking!",
+            raise InvalidReferenceError(
+                f"Reference {team_ref!r} to invalid ranking!",
             ) from None
 
     def _add_match(
@@ -109,7 +126,7 @@ def _add_match(
         ]
 
         if len(teams) != self.num_teams_per_arena:
-            raise ValueError(
+            raise WrongNumberOfTeamsError(
                 f"Unexpected number of teams in match {num} (round {round_num}); "
                 f"got {len(teams)}, expecting {self.num_teams_per_arena}." + (
                     " Fill any expected empty places with `null`."

tests/test_static_knockout_scheduler.py

@@ -4,9 +4,16 @@
 from unittest import mock
 
 from sr.comp.knockout_scheduler import StaticScheduler, UNKNOWABLE_TEAM
+from sr.comp.knockout_scheduler.static_scheduler import (
+    InvalidReferenceError,
+    InvalidSeedError,
+    WrongNumberOfTeamsError,
+)
 from sr.comp.match_period import Match, MatchType
 from sr.comp.teams import Team
 
+from .factories import build_match
+
 TLAs = ['AAA', 'BBB', 'CCC', 'DDD', 'EEE', 'FFF', 'GGG', 'HHH', 'III', 'JJJ']
 
 
@@ -205,6 +212,29 @@ def assertMatches(self, expected_matches, **kwargs):
 
             self.assertEqual(e, a, f"Match {i} in the knockouts")
 
+    def assertInvalidReference(self, value, matches=()):
+        config = get_four_team_config()
+
+        config['matches'][1][0]['teams'][0] = value
+
+        self.assertInvalidSchedule(config, InvalidReferenceError, matches)
+
+    def assertInvalidSeed(self, value, matches=()):
+        config = get_four_team_config()
+
+        config['matches'][1][0]['teams'][0] = value
+
+        self.assertInvalidSchedule(config, InvalidSeedError, matches)
+
+    def assertInvalidSchedule(self, config, exception_type, matches=()):
+        with self.assertRaises(exception_type):
+            scheduler = get_scheduler(
+                matches_config=config,
+                matches=matches,
+            )
+
+            scheduler.add_knockouts()
+
     def test_four_teams_before(self):
         # Add an un-scored league match so that we don't appear to have played them all
         league_matches = [{'A': Match(
@@ -448,3 +478,88 @@ def test_two_teams_partial_2(self):
                 ]),
             },
         )
+
+    def test_improper_position_reference(self):
+        self.assertInvalidReference('00')
+
+    def test_invalid_position_reference(self):
+        self.assertInvalidReference('005')
+
+    def test_invalid_match_reference(self):
+        self.assertInvalidReference('050')
+
+    def test_invalid_round_reference(self):
+        self.assertInvalidReference('500')
+
+    def test_invalid_seed_reference_low(self):
+        self.assertInvalidSeed('S0')
+
+    def test_invalid_seed_reference_high(self):
+        self.assertInvalidSeed('S9999')
+
+    def test_invalid_position_reference_incomplete_league(self):
+        # Add an un-scored league match so that we don't appear to have played them all
+        league_matches = [{'A': build_match(arena='A')}]
+        self.assertInvalidReference('005', matches=league_matches)
+
+    def test_invalid_match_reference_incomplete_league(self):
+        # Add an un-scored league match so that we don't appear to have played them all
+        league_matches = [{'A': build_match(arena='A')}]
+        self.assertInvalidReference('050', matches=league_matches)
+
+    def test_invalid_round_reference_incomplete_league(self):
+        # Add an un-scored league match so that we don't appear to have played them all
+        league_matches = [{'A': build_match(arena='A')}]
+        self.assertInvalidReference('500', matches=league_matches)
+
+    def test_invalid_seed_reference_low_incomplete_league(self):
+        # Add an un-scored league match so that we don't appear to have played them all
+        league_matches = [{'A': build_match(arena='A')}]
+        self.assertInvalidSeed('S0', matches=league_matches)
+
+    def test_invalid_seed_reference_high_incomplete_league(self):
+        # Add an un-scored league match so that we don't appear to have played them all
+        league_matches = [{'A': build_match(arena='A')}]
+        self.assertInvalidSeed('S9999', matches=league_matches)
+
+    def test_too_few_teams_first_round(self):
+        config = get_four_team_config()
+
+        config['matches'][0][0]['teams'].pop()
+
+        self.assertInvalidSchedule(config, WrongNumberOfTeamsError)
+
+    def test_too_few_teams_second_round(self):
+        config = get_four_team_config()
+
+        config['matches'][1][0]['teams'].pop()
+
+        self.assertInvalidSchedule(config, WrongNumberOfTeamsError)
+
+    def test_too_few_teams_third_round(self):
+        config = get_four_team_config()
+
+        config['matches'][2][0]['teams'].pop()
+
+        self.assertInvalidSchedule(config, WrongNumberOfTeamsError)
+
+    def test_too_many_teams_first_round(self):
+        config = get_four_team_config()
+
+        config['matches'][0][0]['teams'].append('S1')
+
+        self.assertInvalidSchedule(config, WrongNumberOfTeamsError)
+
+    def test_too_many_teams_second_round(self):
+        config = get_four_team_config()
+
+        config['matches'][1][0]['teams'].append('S1')
+
+        self.assertInvalidSchedule(config, WrongNumberOfTeamsError)
+
+    def test_too_many_teams_third_round(self):
+        config = get_four_team_config()
+
+        config['matches'][2][0]['teams'].append('S1')
+
+        self.assertInvalidSchedule(config, WrongNumberOfTeamsError)