Issue/#721 non ascii in matchmaker game names (#722)

* Move all strategies to their own file

* Rewrite start_game tests to use hypothesis

Note that test_start_game_with_teams will fail.

* Bypass the ascii check when setting matchmaker game names

Since the name was generated internally, we can trust it to be ok. The 
ascii
check is really meant to prevent encoding errors accross different 
system
configurations and to encourage the use of English titles. Clan names 
will
be guaranteed to be valid utf8, and the rest of the game title will be 
in
English since we created it that way.

* Refactor player_factory to generate connection mocks with spec or autospec

* Update codecov to get statuses to post

codecov.yml → .github/codecov.yml

@@ -1,4 +1,5 @@
-comment: false
+comment:
+  layout: "files"
 
 coverage:
   status:

server/games/game.py

@@ -136,12 +136,19 @@ def name(self):
     @name.setter
     def name(self, value: str):
         """
-        Avoids the game name to crash the mysql INSERT query by being longer
-        than the column's max size.
+        Verifies that names only contain ascii characters.
         """
         if not value.isascii():
             raise ValueError("Name must be ascii!")
 
+        self.set_name_unchecked(value)
+
+    def set_name_unchecked(self, value: str):
+        """
+        Sets the game name without doing any validity checks.
+
+        Truncates the game name to avoid crashing mysql INSERT statements.
+        """
         max_len = game_stats.c.gameName.type.length
         self._name = value[:max_len]
 

server/ladder_service.py

@@ -373,13 +373,14 @@ async def start_game(
                 game_class=LadderGame,
                 game_mode=queue.featured_mod,
                 host=host,
-                name=game_name(team1, team2),
+                name="Matchmaker Game",
                 matchmaker_queue_id=queue.id,
                 rating_type=queue.rating_type,
                 max_players=len(all_players)
             )
             game.init_mode = InitMode.AUTO_LOBBY
             game.map_file_path = map_path
+            game.set_name_unchecked(game_name(team1, team2))
 
             def get_player_mean(player):
                 return player.ratings[queue.rating_type][0]

server/players.py

@@ -166,7 +166,9 @@ def __str__(self) -> str:
                 f"{self.ratings[RatingType.LADDER_1V1]})")
 
     def __repr__(self) -> str:
-        return self.__str__()
+        return (f"Player(login={self.login}, session={self.session}, "
+                f"id={self.id}, ratings={dict(self.ratings)}, "
+                f"clan={self.clan}, game_count={dict(self.game_count)})")
 
     def __hash__(self) -> int:
         return self.id

tests/conftest.py

@@ -180,11 +180,13 @@ def make_game(database, uid, players):
 
 
 def make_player(
+    login=None,
     state=PlayerState.IDLE,
     global_rating=None,
     ladder_rating=None,
     global_games=0,
     ladder_games=0,
+    lobby_connection_spec=None,
     **kwargs
 ):
     ratings = {k: v for k, v in {
@@ -197,42 +199,31 @@ def make_player(
         RatingType.LADDER_1V1: ladder_games
     }
 
-    p = Player(ratings=ratings, game_count=games, **kwargs)
+    p = Player(login=login, ratings=ratings, game_count=games, **kwargs)
     p.state = state
+
+    if lobby_connection_spec:
+        if not isinstance(lobby_connection_spec, str):
+            conn = mock.Mock(spec=lobby_connection_spec)
+        elif lobby_connection_spec == "mock":
+            conn = mock.Mock(spec=LobbyConnection)
+        elif lobby_connection_spec == "auto":
+            conn = asynctest.create_autospec(LobbyConnection)
+        else:
+            raise ValueError(f"Unknown spec type '{lobby_connection_spec}'")
+
+        # lobby_connection is a weak reference, but we want the mock
+        # to live for the full lifetime of the player object
+        p.__owned_lobby_connection = conn
+        p.lobby_connection = p.__owned_lobby_connection
+        p.lobby_connection.player = p
+
     return p
 
 
 @pytest.fixture(scope="session")
 def player_factory():
-    def make(
-        login=None,
-        state=PlayerState.IDLE,
-        global_rating=None,
-        ladder_rating=None,
-        global_games=0,
-        ladder_games=0,
-        with_lobby_connection=False,
-        **kwargs
-    ):
-        p = make_player(
-            state=state,
-            global_rating=global_rating,
-            ladder_rating=ladder_rating,
-            global_games=global_games,
-            ladder_games=ladder_games,
-            login=login,
-            **kwargs
-        )
-
-        if with_lobby_connection:
-            # lobby_connection is a weak reference, but we want the mock
-            # to live for the full lifetime of the player object
-            p.__owned_lobby_connection = asynctest.create_autospec(LobbyConnection)
-            p.lobby_connection = p.__owned_lobby_connection
-            p.lobby_connection.player = p
-        return p
-
-    return make
+    return make_player
 
 
 @pytest.fixture

tests/unit_tests/conftest.py

@@ -149,7 +149,7 @@ def add(gameobj: Game, n: int, team: int = None):
                 player_id=i+1,
                 login=f"Player {i + 1}",
                 global_rating=(1500, 500),
-                with_lobby_connection=False
+                lobby_connection_spec=None
             )
             players.append(p)
 

tests/unit_tests/strategies.py

@@ -0,0 +1,56 @@
+# Custom hypothesis strategies
+
+import string
+
+from hypothesis import strategies as st
+
+from server.matchmaker import Search
+from tests.conftest import make_player
+
+
+@st.composite
+def st_rating(draw):
+    """Strategy for generating rating tuples"""
+    return (
+        draw(st.floats(min_value=-100., max_value=2500.)),
+        draw(st.floats(min_value=0.001, max_value=500.))
+    )
+
+
+@st.composite
+def st_players(draw, name=None, **kwargs):
+    """Strategy for generating Player objects"""
+    return make_player(
+        ladder_rating=(draw(st_rating())),
+        ladder_games=draw(st.integers(0, 1000)),
+        login=draw(st.text(
+            alphabet=list(string.ascii_letters + string.digits + "_-"),
+            min_size=1,
+            max_size=42
+        )) if name is None else name,
+        clan=draw(st.text(min_size=1, max_size=3)),
+        **kwargs
+    )
+
+
+@st.composite
+def st_searches(draw, num_players=1):
+    """Strategy for generating Search objects"""
+    return Search([
+        draw(st_players(f"p{i}")) for i in range(num_players)
+    ])
+
+
+@st.composite
+def st_searches_list(draw, min_players=1, max_players=10, max_size=30):
+    """Strategy for generating a list of Search objects"""
+    return draw(
+        st.lists(
+            st_searches(
+                num_players=draw(
+                    st.integers(min_value=min_players, max_value=max_players)
+                )
+            ),
+            max_size=max_size
+        )
+    )

tests/unit_tests/test_game_rating.py

@@ -107,7 +107,6 @@ def add_players_with_rating(player_factory, game, ratings, teams):
                 player_id=i,
                 global_rating=rating,
                 ladder_rating=rating,
-                with_lobby_connection=False,
             ),
             team,
         )
@@ -857,7 +856,6 @@ async def test_single_wrong_report_still_rated_correctly(game: Game, player_fact
             login=f"{player_id}",
             player_id=player_id,
             global_rating=Rating(old_rating, 250),
-            with_lobby_connection=False,
         )
         for team in log_dict["teams"].values()
         for player_id in team