Tests: Clean up some of the fill test helpers a bit (#2935)

* Tests: Clean up some of the fill test helpers a bit

* fix some formatting

---------

Co-authored-by: NewSoupVi <57900059+NewSoupVi@users.noreply.github.com>

test/general/__init__.py

@@ -1,7 +1,7 @@
 from argparse import Namespace
 from typing import List, Optional, Tuple, Type, Union
 
-from BaseClasses import CollectionState, MultiWorld
+from BaseClasses import CollectionState, Item, ItemClassification, Location, MultiWorld, Region
 from worlds.AutoWorld import World, call_all
 
 gen_steps = ("generate_early", "create_regions", "create_items", "set_rules", "generate_basic", "pre_fill")
@@ -17,19 +17,21 @@ def setup_solo_multiworld(
     :param steps: The gen steps that should be called on the generated multiworld before returning. Default calls
     steps through pre_fill
     :param seed: The seed to be used when creating this multiworld
+    :return: The generated multiworld
     """
     return setup_multiworld(world_type, steps, seed)
 
 
 def setup_multiworld(worlds: Union[List[Type[World]], Type[World]], steps: Tuple[str, ...] = gen_steps,
-    seed: Optional[int] = None) -> MultiWorld:
+                     seed: Optional[int] = None) -> MultiWorld:
     """
     Creates a multiworld with a player for each provided world type, allowing duplicates, setting default options, and
     calling the provided gen steps.
     
-    :param worlds: type/s of worlds to generate a multiworld for
+    :param worlds: Type/s of worlds to generate a multiworld for
-    :param steps: gen steps that should be called before returning. Default calls through pre_fill
+    :param steps: Gen steps that should be called before returning. Default calls through pre_fill
     :param seed: The seed to be used when creating this multiworld
+    :return: The generated multiworld
     """
     if not isinstance(worlds, list):
         worlds = [worlds]
@@ -49,3 +51,59 @@ def setup_multiworld(worlds: Union[List[Type[World]], Type[World]], steps: Tuple
     for step in steps:
         call_all(multiworld, step)
     return multiworld
+
+
+class TestWorld(World):
+    game = f"Test Game"
+    item_name_to_id = {}
+    location_name_to_id = {}
+    hidden = True
+
+
+def generate_test_multiworld(players: int = 1) -> MultiWorld:
+    """
+    Generates a multiworld using a special Test Case World class, and seed of 0.
+
+    :param players: Number of players to generate the multiworld for
+    :return: The generated test multiworld
+    """
+    multiworld = setup_multiworld([TestWorld] * players, seed=0)
+    multiworld.regions += [Region("Menu", player_id + 1, multiworld) for player_id in range(players)]
+
+    return multiworld
+
+
+def generate_locations(count: int, player_id: int, region: Region, address: Optional[int] = None,
+                       tag: str = "") -> List[Location]:
+    """
+    Generates the specified amount of locations for the player and adds them to the specified region.
+
+    :param count: Number of locations to create
+    :param player_id: ID of the player to create the locations for
+    :param address: Address for the specified locations. They will all share the same address if multiple are created
+    :param region: Parent region to add these locations to
+    :param tag: Tag to add to the name of the generated locations
+    :return: List containing the created locations
+    """
+    prefix = f"player{player_id}{tag}_location"
+
+    locations = [Location(player_id, f"{prefix}{i}", address, region) for i in range(count)]
+    region.locations += locations
+    return locations
+
+
+def generate_items(count: int, player_id: int, advancement: bool = False, code: int = None) -> List[Item]:
+    """
+    Generates the specified amount of items for the target player.
+
+    :param count: The amount of items to create
+    :param player_id: ID of the player to create the items for
+    :param advancement: Whether the created items should be advancement
+    :param code: The code the items should be created with
+    :return: List containing the created items
+    """
+    item_type = "prog" if advancement else ""
+    classification = ItemClassification.progression if advancement else ItemClassification.filler
+
+    items = [Item(f"player{player_id}_{item_type}item{i}", classification, code, player_id) for i in range(count)]
+    return items

test/general/test_fill.py

@@ -1,41 +1,15 @@
 from typing import List, Iterable
 import unittest
 
-import Options
 from Options import Accessibility
-from worlds.AutoWorld import World
+from test.general import generate_items, generate_locations, generate_test_multiworld
 from Fill import FillError, balance_multiworld_progression, fill_restrictive, \
     distribute_early_items, distribute_items_restrictive
 from BaseClasses import Entrance, LocationProgressType, MultiWorld, Region, Item, Location, \
-    ItemClassification, CollectionState
+    ItemClassification
 from worlds.generic.Rules import CollectionRule, add_item_rule, locality_rules, set_rule
 
 
-def generate_multiworld(players: int = 1) -> MultiWorld:
-    multiworld = MultiWorld(players)
-    multiworld.set_seed(0)
-    multiworld.player_name = {}
-    multiworld.state = CollectionState(multiworld)
-    for i in range(players):
-        player_id = i+1
-        world = World(multiworld, player_id)
-        multiworld.game[player_id] = f"Game {player_id}"
-        multiworld.worlds[player_id] = world
-        multiworld.player_name[player_id] = "Test Player " + str(player_id)
-        region = Region("Menu", player_id, multiworld, "Menu Region Hint")
-        multiworld.regions.append(region)
-        for option_key, option in Options.PerGameCommonOptions.type_hints.items():
-            if hasattr(multiworld, option_key):
-                getattr(multiworld, option_key).setdefault(player_id, option.from_any(getattr(option, "default")))
-            else:
-                setattr(multiworld, option_key, {player_id: option.from_any(getattr(option, "default"))})
-        # TODO - remove this loop once all worlds use options dataclasses
-        world.options = world.options_dataclass(**{option_key: getattr(multiworld, option_key)[player_id]
-                                                   for option_key in world.options_dataclass.type_hints})
-
-    return multiworld
-
-
 class PlayerDefinition(object):
     multiworld: MultiWorld
     id: int
@@ -55,12 +29,12 @@ class PlayerDefinition(object):
         self.regions = [menu]
 
     def generate_region(self, parent: Region, size: int, access_rule: CollectionRule = lambda state: True) -> Region:
-        region_tag = "_region" + str(len(self.regions))
+        region_tag = f"_region{len(self.regions)}"
-        region_name = "player" + str(self.id) + region_tag
+        region_name = f"player{self.id}{region_tag}"
-        region = Region("player" + str(self.id) + region_tag, self.id, self.multiworld)
+        region = Region(f"player{self.id}{region_tag}", self.id, self.multiworld)
-        self.locations += generate_locations(size, self.id, None, region, region_tag)
+        self.locations += generate_locations(size, self.id, region, None, region_tag)
 
-        entrance = Entrance(self.id, region_name + "_entrance", parent)
+        entrance = Entrance(self.id, f"{region_name}_entrance", parent)
         parent.exits.append(entrance)
         entrance.connect(region)
         entrance.access_rule = access_rule
@@ -94,7 +68,7 @@ def region_contains(region: Region, item: Item) -> bool:
 
 def generate_player_data(multiworld: MultiWorld, player_id: int, location_count: int = 0, prog_item_count: int = 0, basic_item_count: int = 0) -> PlayerDefinition:
     menu = multiworld.get_region("Menu", player_id)
-    locations = generate_locations(location_count, player_id, None, menu)
+    locations = generate_locations(location_count, player_id, menu, None)
     prog_items = generate_items(prog_item_count, player_id, True)
     multiworld.itempool += prog_items
     basic_items = generate_items(basic_item_count, player_id, False)
@@ -103,28 +77,6 @@ def generate_player_data(multiworld: MultiWorld, player_id: int, location_count:
     return PlayerDefinition(multiworld, player_id, menu, locations, prog_items, basic_items)
 
 
-def generate_locations(count: int, player_id: int, address: int = None, region: Region = None, tag: str = "") -> List[Location]:
-    locations = []
-    prefix = "player" + str(player_id) + tag + "_location"
-    for i in range(count):
-        name = prefix + str(i)
-        location = Location(player_id, name, address, region)
-        locations.append(location)
-        region.locations.append(location)
-    return locations
-
-
-def generate_items(count: int, player_id: int, advancement: bool = False, code: int = None) -> List[Item]:
-    items = []
-    item_type = "prog" if advancement else ""
-    for i in range(count):
-        name = "player" + str(player_id) + "_" + item_type + "item" + str(i)
-        items.append(Item(name,
-                          ItemClassification.progression if advancement else ItemClassification.filler,
-                          code, player_id))
-    return items
-
-
 def names(objs: list) -> Iterable[str]:
     return map(lambda o: o.name, objs)
 
@@ -132,7 +84,7 @@ def names(objs: list) -> Iterable[str]:
 class TestFillRestrictive(unittest.TestCase):
     def test_basic_fill(self):
         """Tests `fill_restrictive` fills and removes the locations and items from their respective lists"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 2, 2)
 
         item0 = player1.prog_items[0]
@@ -150,7 +102,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_ordered_fill(self):
         """Tests `fill_restrictive` fulfills set rules"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 2, 2)
         items = player1.prog_items
         locations = player1.locations
@@ -167,7 +119,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_partial_fill(self):
         """Tests that `fill_restrictive` returns unfilled locations"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 3, 2)
 
         item0 = player1.prog_items[0]
@@ -193,7 +145,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_minimal_fill(self):
         """Test that fill for minimal player can have unreachable items"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 2, 2)
 
         items = player1.prog_items
@@ -218,7 +170,7 @@ class TestFillRestrictive(unittest.TestCase):
         the non-minimal player get all items.
         """
 
-        multiworld = generate_multiworld(2)
+        multiworld = generate_test_multiworld(2)
         player1 = generate_player_data(multiworld, 1, 3, 3)
         player2 = generate_player_data(multiworld, 2, 3, 3)
 
@@ -245,11 +197,11 @@ class TestFillRestrictive(unittest.TestCase):
         # all of player2's locations and items should be accessible (not all of player1's)
         for item in player2.prog_items:
             self.assertTrue(multiworld.state.has(item.name, player2.id),
-                            f'{item} is unreachable in {item.location}')
+                            f"{item} is unreachable in {item.location}")
 
     def test_reversed_fill(self):
         """Test a different set of rules can be satisfied"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 2, 2)
 
         item0 = player1.prog_items[0]
@@ -268,7 +220,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_multi_step_fill(self):
         """Test that fill is able to satisfy multiple spheres"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 4, 4)
 
         items = player1.prog_items
@@ -293,7 +245,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_impossible_fill(self):
         """Test that fill raises an error when it can't place any items"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 2, 2)
         items = player1.prog_items
         locations = player1.locations
@@ -310,7 +262,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_circular_fill(self):
         """Test that fill raises an error when it can't place all items"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 3, 3)
 
         item0 = player1.prog_items[0]
@@ -331,7 +283,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_competing_fill(self):
         """Test that fill raises an error when it can't place items in a way to satisfy the conditions"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 2, 2)
 
         item0 = player1.prog_items[0]
@@ -348,7 +300,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_multiplayer_fill(self):
         """Test that items can be placed across worlds"""
-        multiworld = generate_multiworld(2)
+        multiworld = generate_test_multiworld(2)
         player1 = generate_player_data(multiworld, 1, 2, 2)
         player2 = generate_player_data(multiworld, 2, 2, 2)
 
@@ -369,7 +321,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_multiplayer_rules_fill(self):
         """Test that fill across worlds satisfies the rules"""
-        multiworld = generate_multiworld(2)
+        multiworld = generate_test_multiworld(2)
         player1 = generate_player_data(multiworld, 1, 2, 2)
         player2 = generate_player_data(multiworld, 2, 2, 2)
 
@@ -393,7 +345,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_restrictive_progress(self):
         """Test that various spheres with different requirements can be filled"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, prog_item_count=25)
         items = player1.prog_items.copy()
         multiworld.completion_condition[player1.id] = lambda state: state.has_all(
@@ -417,7 +369,7 @@ class TestFillRestrictive(unittest.TestCase):
     def test_swap_to_earlier_location_with_item_rule(self):
         """Test that item swap happens and works as intended"""
         # test for PR#1109
-        multiworld = generate_multiworld(1)
+        multiworld = generate_test_multiworld(1)
         player1 = generate_player_data(multiworld, 1, 4, 4)
         locations = player1.locations[:]  # copy required
         items = player1.prog_items[:]  # copy required
@@ -442,7 +394,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_swap_to_earlier_location_with_item_rule2(self):
         """Test that swap works before all items are placed"""
-        multiworld = generate_multiworld(1)
+        multiworld = generate_test_multiworld(1)
         player1 = generate_player_data(multiworld, 1, 5, 5)
         locations = player1.locations[:]  # copy required
         items = player1.prog_items[:]  # copy required
@@ -484,7 +436,7 @@ class TestFillRestrictive(unittest.TestCase):
     def test_double_sweep(self):
         """Test that sweep doesn't duplicate Event items when sweeping"""
         # test for PR1114
-        multiworld = generate_multiworld(1)
+        multiworld = generate_test_multiworld(1)
         player1 = generate_player_data(multiworld, 1, 1, 1)
         location = player1.locations[0]
         location.address = None
@@ -498,7 +450,7 @@ class TestFillRestrictive(unittest.TestCase):
 
     def test_correct_item_instance_removed_from_pool(self):
         """Test that a placed item gets removed from the submitted pool"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(multiworld, 1, 2, 2)
 
         player1.prog_items[0].name = "Different_item_instance_but_same_item_name"
@@ -515,7 +467,7 @@ class TestFillRestrictive(unittest.TestCase):
 class TestDistributeItemsRestrictive(unittest.TestCase):
     def test_basic_distribute(self):
         """Test that distribute_items_restrictive is deterministic"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         locations = player1.locations
@@ -535,7 +487,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_excluded_distribute(self):
         """Test that distribute_items_restrictive doesn't put advancement items on excluded locations"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         locations = player1.locations
@@ -550,7 +502,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_non_excluded_item_distribute(self):
         """Test that useful items aren't placed on excluded locations"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         locations = player1.locations
@@ -565,7 +517,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_too_many_excluded_distribute(self):
         """Test that fill fails if it can't place all progression items due to too many excluded locations"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         locations = player1.locations
@@ -578,7 +530,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_non_excluded_item_must_distribute(self):
         """Test that fill fails if it can't place useful items due to too many excluded locations"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         locations = player1.locations
@@ -593,7 +545,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_priority_distribute(self):
         """Test that priority locations receive advancement items"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         locations = player1.locations
@@ -608,7 +560,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_excess_priority_distribute(self):
         """Test that if there's more priority locations than advancement items, they can still fill"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         locations = player1.locations
@@ -623,7 +575,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_multiple_world_priority_distribute(self):
         """Test that priority fill can be satisfied for multiple worlds"""
-        multiworld = generate_multiworld(3)
+        multiworld = generate_test_multiworld(3)
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
         player2 = generate_player_data(
@@ -653,7 +605,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_can_remove_locations_in_fill_hook(self):
         """Test that distribute_items_restrictive calls the fill hook and allows for item and location removal"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, 4, prog_item_count=2, basic_item_count=2)
 
@@ -673,12 +625,12 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_seed_robust_to_item_order(self):
         """Test deterministic fill"""
-        mw1 = generate_multiworld()
+        mw1 = generate_test_multiworld()
         gen1 = generate_player_data(
             mw1, 1, 4, prog_item_count=2, basic_item_count=2)
         distribute_items_restrictive(mw1)
 
-        mw2 = generate_multiworld()
+        mw2 = generate_test_multiworld()
         gen2 = generate_player_data(
             mw2, 1, 4, prog_item_count=2, basic_item_count=2)
         mw2.itempool.append(mw2.itempool.pop(0))
@@ -691,12 +643,12 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_seed_robust_to_location_order(self):
         """Test deterministic fill even if locations in a region are reordered"""
-        mw1 = generate_multiworld()
+        mw1 = generate_test_multiworld()
         gen1 = generate_player_data(
             mw1, 1, 4, prog_item_count=2, basic_item_count=2)
         distribute_items_restrictive(mw1)
 
-        mw2 = generate_multiworld()
+        mw2 = generate_test_multiworld()
         gen2 = generate_player_data(
             mw2, 1, 4, prog_item_count=2, basic_item_count=2)
         reg = mw2.get_region("Menu", gen2.id)
@@ -710,7 +662,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_can_reserve_advancement_items_for_general_fill(self):
         """Test that priority locations fill still satisfies item rules"""
-        multiworld = generate_multiworld()
+        multiworld = generate_test_multiworld()
         player1 = generate_player_data(
             multiworld, 1, location_count=5, prog_item_count=5)
         items = player1.prog_items
@@ -727,7 +679,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_non_excluded_local_items(self):
         """Test that local items get placed locally in a multiworld"""
-        multiworld = generate_multiworld(2)
+        multiworld = generate_test_multiworld(2)
         player1 = generate_player_data(
             multiworld, 1, location_count=5, basic_item_count=5)
         player2 = generate_player_data(
@@ -748,7 +700,7 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
 
     def test_early_items(self) -> None:
         """Test that the early items API successfully places items early"""
-        mw = generate_multiworld(2)
+        mw = generate_test_multiworld(2)
         player1 = generate_player_data(mw, 1, location_count=5, basic_item_count=5)
         player2 = generate_player_data(mw, 2, location_count=5, basic_item_count=5)
         mw.early_items[1][player1.basic_items[0].name] = 1
@@ -803,11 +755,11 @@ class TestBalanceMultiworldProgression(unittest.TestCase):
             if location.item and location.item == item:
                 return True
 
-        self.fail("Expected " + region.name + " to contain " + item.name +
+        self.fail(f"Expected {region.name} to contain {item.name}.\n"
-                  "\n Contains" + str(list(map(lambda location: location.item, region.locations))))
+                  f"Contains{list(map(lambda location: location.item, region.locations))}")
 
     def setUp(self) -> None:
-        multiworld = generate_multiworld(2)
+        multiworld = generate_test_multiworld(2)
         self.multiworld = multiworld
         player1 = generate_player_data(
             multiworld, 1, prog_item_count=2, basic_item_count=40)