From 9479c92c9763cdd1634287cf9ccc5ae8e0b520e9 Mon Sep 17 00:00:00 2001 From: Erik Vroon Date: Sat, 10 Feb 2024 20:59:36 +0100 Subject: [PATCH] Increase code coverage (#466) Remove unused code and add some tests Also fix detection of running pytest --- backend/bracket/app.py | 2 +- backend/bracket/config.py | 3 +- backend/bracket/logic/planning/matches.py | 113 ------------- backend/bracket/logic/scheduling/builder.py | 16 +- .../bracket/logic/scheduling/elimination.py | 6 +- .../bracket/logic/scheduling/round_robin.py | 4 +- .../logic/scheduling/upcoming_matches.py | 2 +- backend/bracket/models/db/round.py | 1 - backend/bracket/routes/matches.py | 4 +- backend/bracket/routes/rounds.py | 11 +- backend/bracket/routes/stage_items.py | 2 +- backend/bracket/sql/rounds.py | 19 +++ .../api/auto_scheduling_matches_test.py | 155 ++++++++++++++++++ .../tests/integration_tests/api/users_test.py | 2 + 14 files changed, 195 insertions(+), 145 deletions(-) create mode 100644 backend/tests/integration_tests/api/auto_scheduling_matches_test.py diff --git a/backend/bracket/app.py b/backend/bracket/app.py index 07e0c446b..9b4dc3a15 100644 --- a/backend/bracket/app.py +++ b/backend/bracket/app.py @@ -58,7 +58,7 @@ async def lifespan(_: FastAPI) -> AsyncIterator[None]: yield - if environment != Environment.CI: + if environment is not Environment.CI: await database.disconnect() await AsyncioTasksManager.gather() diff --git a/backend/bracket/config.py b/backend/bracket/config.py index 1856fece4..05d1dadc4 100644 --- a/backend/bracket/config.py +++ b/backend/bracket/config.py @@ -1,5 +1,6 @@ import logging import os +import sys from enum import auto from typing import Annotated @@ -65,7 +66,7 @@ class DemoConfig(Config): def currently_testing() -> bool: - return "PYTEST_CURRENT_TEST" in os.environ + return "pytest" in sys.modules environment = Environment( diff --git a/backend/bracket/logic/planning/matches.py b/backend/bracket/logic/planning/matches.py index 63a7e58ff..ac451a98b 100644 --- a/backend/bracket/logic/planning/matches.py +++ b/backend/bracket/logic/planning/matches.py @@ -1,22 +1,17 @@ -import random from collections import defaultdict from typing import NamedTuple from heliclockter import timedelta from bracket.models.db.match import ( - Match, - MatchCreateBody, MatchRescheduleBody, MatchWithDetails, MatchWithDetailsDefinitive, ) -from bracket.models.db.stage_item_inputs import StageItemInputGeneric from bracket.models.db.tournament import Tournament from bracket.models.db.util import StageWithStageItems from bracket.sql.courts import get_all_courts_in_tournament from bracket.sql.matches import ( - sql_create_match, sql_reschedule_match_and_determine_duration_and_margin, ) from bracket.sql.stages import get_full_tournament_details @@ -24,13 +19,6 @@ from bracket.utils.types import assert_some -async def create_match_and_assign_free_court( - tournament_id: int, - match_body: MatchCreateBody, -) -> Match: - return await sql_create_match(match_body) - - async def schedule_all_unscheduled_matches(tournament_id: int) -> None: tournament = await sql_get_tournament(tournament_id) stages = await get_full_tournament_details(tournament_id) @@ -85,107 +73,6 @@ async def schedule_all_unscheduled_matches(tournament_id: int) -> None: await update_start_times_of_matches(tournament_id) -def has_conflict( - match: MatchWithDetailsDefinitive | MatchWithDetails, - team_defs: set[int | None], - matches_per_team: dict[int | None, list[Match]], -) -> bool: - for team in team_defs: - for existing_match in matches_per_team[team]: - if existing_match.start_time == match.start_time: - return True - - return False - - -async def todo_schedule_all_matches(tournament_id: int) -> None: - tournament = await sql_get_tournament(tournament_id) - stages = await get_full_tournament_details(tournament_id) - courts = await get_all_courts_in_tournament(tournament_id) - - match_count_per_court: dict[int, int] = {assert_some(court.id): 0 for court in courts} - matches_per_court: dict[int, list[Match]] = {assert_some(court.id): [] for court in courts} - matches_per_team: dict[int | None, list[Match]] = defaultdict(list) - - matches_to_schedule = [ - match.model_copy(update={"court_id": None, "position_in_schedule": None}) - for stage in stages - for stage_item in stage.stage_items - for round_ in stage_item.rounds - for match in round_.matches - ] - await iterative_scheduling( - match_count_per_court, - matches_per_court, - matches_per_team, - matches_to_schedule, - tournament, - ) - - -async def iterative_scheduling( - match_count_per_court: dict[int, int], - matches_per_court: dict[int, list[Match]], - matches_per_team: dict[int | None, list[Match]], - matches_to_schedule: list[MatchWithDetailsDefinitive | MatchWithDetails], - tournament: Tournament, -) -> None: - attempts_since_last_write = 0 - - while len(matches_to_schedule) > 0: - attempts_since_last_write += 1 - match = matches_to_schedule[0] - - StageItemInputGeneric( - team_id=match.team1_id, - winner_from_stage_item_id=match.team1_winner_from_stage_item_id, - winner_position=match.team1_winner_position, - winner_from_match_id=match.team1_winner_from_match_id, - ) - StageItemInputGeneric( - team_id=match.team2_id, - winner_from_stage_item_id=match.team2_winner_from_stage_item_id, - winner_position=match.team2_winner_position, - winner_from_match_id=match.team2_winner_from_match_id, - ) - team_defs = {match.team1_id, match.team2_id} - court_id = sorted(match_count_per_court.items(), key=lambda x: x[1])[0][0] - - try: - position_in_schedule = len(matches_per_court[court_id]) - last_match = matches_per_court[court_id][-1] - start_time = assert_some(last_match.start_time) + timedelta( - minutes=match.duration_minutes - ) - except IndexError: - start_time = tournament.start_time - position_in_schedule = 0 - - updated_match = match.model_copy( - update={ - "start_time": start_time, - "position_in_schedule": position_in_schedule, - "court_id": court_id, - } - ) - - match_has_conflict = has_conflict(updated_match, team_defs, matches_per_team) - if match_has_conflict and attempts_since_last_write < 100: - continue - - match_count_per_court[court_id] += 1 - matches_per_court[court_id].append(updated_match) - matches_per_team[match.team1_id].append(updated_match) - matches_per_team[match.team2_id].append(updated_match) - matches_to_schedule.remove(match) - attempts_since_last_write = 0 - random.shuffle(matches_to_schedule) - - await sql_reschedule_match_and_determine_duration_and_margin( - assert_some(match.id), court_id, start_time, position_in_schedule, match, tournament - ) - - class MatchPosition(NamedTuple): match: MatchWithDetailsDefinitive | MatchWithDetails position: float diff --git a/backend/bracket/logic/scheduling/builder.py b/backend/bracket/logic/scheduling/builder.py index fa42f6fe3..116d6a264 100644 --- a/backend/bracket/logic/scheduling/builder.py +++ b/backend/bracket/logic/scheduling/builder.py @@ -1,7 +1,5 @@ from fastapi import HTTPException -from heliclockter import datetime_utc -from bracket.database import database from bracket.logic.scheduling.elimination import ( build_single_elimination_stage_item, get_number_of_rounds_to_create_single_elimination, @@ -18,7 +16,7 @@ ) from bracket.models.db.team import FullTeamWithPlayers from bracket.models.db.util import StageWithStageItems -from bracket.sql.rounds import get_next_round_name +from bracket.sql.rounds import get_next_round_name, sql_create_round from bracket.sql.stage_items import get_stage_item from bracket.utils.types import assert_some @@ -35,18 +33,12 @@ async def create_rounds_for_new_stage_item(tournament_id: int, stage_item: Stage case other: raise NotImplementedError(f"No round creation implementation for {other}") - now = datetime_utc.now() for _ in range(rounds_count): - await database.execute( - query=""" - INSERT INTO rounds (created, stage_item_id, name, is_draft, is_active) - VALUES (:created, :stage_item_id, :name, :is_draft, :is_active) - """, - values=RoundToInsert( - created=now, + await sql_create_round( + RoundToInsert( stage_item_id=assert_some(stage_item.id), name=await get_next_round_name(tournament_id, assert_some(stage_item.id)), - ).model_dump(), + ), ) diff --git a/backend/bracket/logic/scheduling/elimination.py b/backend/bracket/logic/scheduling/elimination.py index e43eb229e..fc32cc1ac 100644 --- a/backend/bracket/logic/scheduling/elimination.py +++ b/backend/bracket/logic/scheduling/elimination.py @@ -1,7 +1,7 @@ -from bracket.logic.planning.matches import create_match_and_assign_free_court from bracket.models.db.match import Match, MatchCreateBody from bracket.models.db.tournament import Tournament from bracket.models.db.util import RoundWithMatches, StageItemWithRounds +from bracket.sql.matches import sql_create_match from bracket.sql.rounds import get_rounds_for_stage_item from bracket.sql.tournaments import sql_get_tournament from bracket.utils.types import assert_some @@ -79,13 +79,13 @@ async def build_single_elimination_stage_item( first_round = rounds[0] prev_matches = [ - await create_match_and_assign_free_court(tournament_id, match) + await sql_create_match(match) for match in determine_matches_first_round(first_round, stage_item, tournament) ] for round_ in rounds[1:]: prev_matches = [ - await create_match_and_assign_free_court(tournament_id, match) + await sql_create_match(match) for match in determine_matches_subsequent_round(prev_matches, round_, tournament) ] diff --git a/backend/bracket/logic/scheduling/round_robin.py b/backend/bracket/logic/scheduling/round_robin.py index acc77b6f9..c18604e34 100644 --- a/backend/bracket/logic/scheduling/round_robin.py +++ b/backend/bracket/logic/scheduling/round_robin.py @@ -1,8 +1,8 @@ -from bracket.logic.planning.matches import create_match_and_assign_free_court from bracket.models.db.match import ( MatchCreateBody, ) from bracket.models.db.util import StageItemWithRounds +from bracket.sql.matches import sql_create_match from bracket.sql.tournaments import sql_get_tournament from bracket.utils.types import assert_some @@ -60,7 +60,7 @@ async def build_round_robin_stage_item(tournament_id: int, stage_item: StageItem custom_duration_minutes=None, custom_margin_minutes=None, ) - await create_match_and_assign_free_court(tournament_id, match) + await sql_create_match(match) def get_number_of_rounds_to_create_round_robin(team_count: int) -> int: diff --git a/backend/bracket/logic/scheduling/upcoming_matches.py b/backend/bracket/logic/scheduling/upcoming_matches.py index 7432fb116..1cc2716a0 100644 --- a/backend/bracket/logic/scheduling/upcoming_matches.py +++ b/backend/bracket/logic/scheduling/upcoming_matches.py @@ -18,7 +18,7 @@ async def get_upcoming_matches_for_swiss_round( [stage_item] = stage.stage_items if stage_item.type is not StageType.SWISS: - raise HTTPException(400, "There is no draft round, so no matches can be scheduled.") + raise HTTPException(400, "Expected stage item to be of type SWISS.") rounds = await get_rounds_for_stage_item(tournament_id, assert_some(stage_item.id)) teams = await get_teams_with_members(tournament_id, only_active_teams=True) diff --git a/backend/bracket/models/db/round.py b/backend/bracket/models/db/round.py index 7976fe134..c75efd00b 100644 --- a/backend/bracket/models/db/round.py +++ b/backend/bracket/models/db/round.py @@ -24,7 +24,6 @@ class RoundCreateBody(BaseModelORM): class RoundToInsert(RoundUpdateBody): - created: datetime_utc stage_item_id: int is_draft: bool = False is_active: bool = False diff --git a/backend/bracket/routes/matches.py b/backend/bracket/routes/matches.py index 0d1d4dfdf..8313378b6 100644 --- a/backend/bracket/routes/matches.py +++ b/backend/bracket/routes/matches.py @@ -1,7 +1,6 @@ from fastapi import APIRouter, Depends, HTTPException from bracket.logic.planning.matches import ( - create_match_and_assign_free_court, handle_match_reschedule, schedule_all_unscheduled_matches, ) @@ -146,8 +145,7 @@ async def create_matches_automatically( assert isinstance(match, SuggestedMatch) assert round_.id and match.team1.id and match.team2.id - await create_match_and_assign_free_court( - tournament_id, + await sql_create_match( MatchCreateBody( round_id=round_.id, team1_id=match.team1.id, diff --git a/backend/bracket/routes/rounds.py b/backend/bracket/routes/rounds.py index aa6570ca9..549a82c6c 100644 --- a/backend/bracket/routes/rounds.py +++ b/backend/bracket/routes/rounds.py @@ -1,5 +1,4 @@ from fastapi import APIRouter, Depends, HTTPException -from heliclockter import datetime_utc from starlette import status from bracket.database import database @@ -20,7 +19,7 @@ round_with_matches_dependency, ) from bracket.schema import rounds -from bracket.sql.rounds import get_next_round_name, set_round_active_or_draft +from bracket.sql.rounds import get_next_round_name, set_round_active_or_draft, sql_create_round from bracket.sql.stage_items import get_stage_item from bracket.sql.stages import get_full_tournament_details @@ -78,13 +77,11 @@ async def create_round( detail=f"Stage type {stage_item.type} doesn't support manual creation of rounds", ) - round_id = await database.execute( - query=rounds.insert(), - values=RoundToInsert( - created=datetime_utc.now(), + round_id = await sql_create_round( + RoundToInsert( stage_item_id=round_body.stage_item_id, name=await get_next_round_name(tournament_id, round_body.stage_item_id), - ).model_dump(), + ), ) await set_round_active_or_draft(round_id, tournament_id, is_active=False, is_draft=True) diff --git a/backend/bracket/routes/stage_items.py b/backend/bracket/routes/stage_items.py index 7dea432d1..4beed76b8 100644 --- a/backend/bracket/routes/stage_items.py +++ b/backend/bracket/routes/stage_items.py @@ -123,6 +123,6 @@ async def start_next_round( ) from exc assert next_round.id is not None - await set_round_active_or_draft(next_round.id, tournament_id, is_active=False, is_draft=False) + await set_round_active_or_draft(next_round.id, tournament_id, is_active=True, is_draft=False) return SuccessResponse() diff --git a/backend/bracket/sql/rounds.py b/backend/bracket/sql/rounds.py index 7a8fa9c78..c60eb84cc 100644 --- a/backend/bracket/sql/rounds.py +++ b/backend/bracket/sql/rounds.py @@ -1,9 +1,28 @@ from bracket.database import database +from bracket.models.db.round import RoundToInsert from bracket.models.db.util import RoundWithMatches from bracket.sql.stage_items import get_stage_item from bracket.sql.stages import get_full_tournament_details +async def sql_create_round(round_: RoundToInsert) -> int: + query = """ + INSERT INTO rounds (created, is_draft, is_active, name, stage_item_id) + VALUES (NOW(), :is_draft, :is_active, :name, :stage_item_id) + RETURNING id + """ + result: int = await database.fetch_val( + query=query, + values={ + "name": round_.name, + "is_draft": round_.is_draft, + "is_active": round_.is_active, + "stage_item_id": round_.stage_item_id, + }, + ) + return result + + async def get_rounds_for_stage_item( tournament_id: int, stage_item_id: int ) -> list[RoundWithMatches]: diff --git a/backend/tests/integration_tests/api/auto_scheduling_matches_test.py b/backend/tests/integration_tests/api/auto_scheduling_matches_test.py new file mode 100644 index 000000000..6d46b1cd1 --- /dev/null +++ b/backend/tests/integration_tests/api/auto_scheduling_matches_test.py @@ -0,0 +1,155 @@ +from heliclockter import datetime_utc + +from bracket.models.db.round import RoundToInsert +from bracket.models.db.stage_item import StageItemCreateBody, StageType +from bracket.models.db.stage_item_inputs import ( + StageItemInputCreateBodyFinal, +) +from bracket.sql.rounds import get_round_by_id, sql_create_round +from bracket.sql.shared import sql_delete_stage_item_with_foreign_keys +from bracket.sql.stage_items import sql_create_stage_item +from bracket.sql.stages import get_full_tournament_details +from bracket.utils.dummy_records import ( + DUMMY_COURT1, + DUMMY_STAGE2, + DUMMY_STAGE_ITEM1, + DUMMY_TEAM1, +) +from bracket.utils.http import HTTPMethod +from bracket.utils.types import assert_some +from tests.integration_tests.api.shared import ( + SUCCESS_RESPONSE, + send_tournament_request, +) +from tests.integration_tests.models import AuthContext +from tests.integration_tests.sql import ( + inserted_court, + inserted_stage, + inserted_team, +) + + +async def test_schedule_matches_auto( + startup_and_shutdown_uvicorn_server: None, auth_context: AuthContext +) -> None: + async with ( + inserted_court( + DUMMY_COURT1.model_copy(update={"tournament_id": auth_context.tournament.id}) + ), + inserted_stage( + DUMMY_STAGE2.model_copy(update={"tournament_id": auth_context.tournament.id}) + ) as stage_inserted_1, + inserted_team( + DUMMY_TEAM1.model_copy(update={"tournament_id": auth_context.tournament.id}) + ) as team_inserted_1, + inserted_team( + DUMMY_TEAM1.model_copy(update={"tournament_id": auth_context.tournament.id}) + ) as team_inserted_2, + ): + tournament_id = assert_some(auth_context.tournament.id) + stage_item_1 = await sql_create_stage_item( + tournament_id, + StageItemCreateBody( + stage_id=assert_some(stage_inserted_1.id), + name=DUMMY_STAGE_ITEM1.name, + team_count=2, + type=StageType.SWISS, + inputs=[ + StageItemInputCreateBodyFinal( + slot=1, + team_id=assert_some(team_inserted_1.id), + ), + StageItemInputCreateBodyFinal( + slot=2, + team_id=assert_some(team_inserted_2.id), + ), + ], + ), + ) + round_1_id = await sql_create_round( + RoundToInsert(stage_item_id=stage_item_1.id, name="", is_draft=True, is_active=False), + ) + + response = await send_tournament_request( + HTTPMethod.POST, + f"rounds/{round_1_id}/schedule_auto", + auth_context, + ) + stages = await get_full_tournament_details(tournament_id) + + await sql_delete_stage_item_with_foreign_keys(stage_item_1.id) + + assert response == SUCCESS_RESPONSE + + stage_item = stages[0].stage_items[0] + assert len(stage_item.rounds) == 1 + assert len(stage_item.rounds[0].matches) == 1 + + +async def test_start_next_round( + startup_and_shutdown_uvicorn_server: None, auth_context: AuthContext +) -> None: + async with ( + inserted_court( + DUMMY_COURT1.model_copy(update={"tournament_id": auth_context.tournament.id}) + ), + inserted_stage( + DUMMY_STAGE2.model_copy(update={"tournament_id": auth_context.tournament.id}) + ) as stage_inserted_1, + inserted_team( + DUMMY_TEAM1.model_copy(update={"tournament_id": auth_context.tournament.id}) + ) as team_inserted_1, + inserted_team( + DUMMY_TEAM1.model_copy(update={"tournament_id": auth_context.tournament.id}) + ) as team_inserted_2, + ): + tournament_id = assert_some(auth_context.tournament.id) + stage_item_1 = await sql_create_stage_item( + tournament_id, + StageItemCreateBody( + stage_id=assert_some(stage_inserted_1.id), + name=DUMMY_STAGE_ITEM1.name, + team_count=2, + type=StageType.SWISS, + inputs=[ + StageItemInputCreateBodyFinal( + slot=1, + team_id=assert_some(team_inserted_1.id), + ), + StageItemInputCreateBodyFinal( + slot=2, + team_id=assert_some(team_inserted_2.id), + ), + ], + ), + ) + round_1_id = await sql_create_round( + RoundToInsert(stage_item_id=stage_item_1.id, name="", is_draft=True, is_active=False), + ) + round_2_id = await sql_create_round( + RoundToInsert(stage_item_id=stage_item_1.id, name="", is_draft=True, is_active=False), + ) + + try: + response = await send_tournament_request( + HTTPMethod.POST, + f"stage_items/{stage_item_1.id}/start_next_round", + auth_context, + json={}, + ) + + assert response == SUCCESS_RESPONSE + round_1 = await get_round_by_id(tournament_id, round_1_id) + assert assert_some(round_1).is_active + + response = await send_tournament_request( + HTTPMethod.POST, + f"stage_items/{stage_item_1.id}/start_next_round", + auth_context, + json={"adjust_to_time": datetime_utc.now().isoformat()}, + ) + assert response == SUCCESS_RESPONSE + round_2 = await get_round_by_id(tournament_id, round_2_id) + assert assert_some(round_2).is_active + finally: + await sql_delete_stage_item_with_foreign_keys(stage_item_1.id) diff --git a/backend/tests/integration_tests/api/users_test.py b/backend/tests/integration_tests/api/users_test.py index 0e1aedd2d..b24cd49b1 100644 --- a/backend/tests/integration_tests/api/users_test.py +++ b/backend/tests/integration_tests/api/users_test.py @@ -36,6 +36,7 @@ async def test_create_user( "captcha_token": "my token", } response = await send_request(HTTPMethod.POST, "users/register", None, body) + assert "data" in response, response assert response["data"]["token_type"] == "bearer" assert response["data"]["user_id"] await delete_user(response["data"]["user_id"]) @@ -46,6 +47,7 @@ async def test_create_demo_user( ) -> None: body = {"captcha_token": "my token"} response = await send_request(HTTPMethod.POST, "users/register_demo", None, body) + assert "data" in response, response assert response["data"]["token_type"] == "bearer" assert response["data"]["user_id"] await delete_user(response["data"]["user_id"])