Fix a bug for Player.__init__ and Player.__del__ where, if a bad strategy name is supplied, the object doesn't teardown correctly

Author: marcharper

src/axelrod_fortran/player.py

@@ -42,11 +42,15 @@ def __init__(self, original_name):
             A instance of an axelrod Game
         """
         super().__init__()
+        # The order of the next 4 lines is important. We must first check that
+        # the player name is valid, then grab a copy of the shared library,
+        # and then setup the actual strategy function.
+        self.original_name = original_name
         self.index, self.shared_library_filename = \
-            shared_library_manager.get_filename_for_player(original_name)
+            shared_library_manager.get_filename_for_player(self.original_name)
         self.shared_library = load_library(self.shared_library_filename)
-        self.__original_name = original_name
         self.original_function = self.original_name
+
         is_stochastic = characteristics[self.original_name]['stochastic']
         if is_stochastic is not None:
             self.classifier['stochastic'] = is_stochastic
@@ -62,11 +66,11 @@ def original_name(self):
         return self.__original_name
 
     @original_name.setter
-    def original_name(self, value):
-        if value in characteristics:
-            self.__original_name = value
+    def original_name(self, key):
+        if key in characteristics:
+            self.__original_name = key
         else:
-            raise ValueError('{} is not a valid Fortran function'.format(value))
+            raise ValueError('{} is not a valid Fortran function'.format(key))
 
     @property
     def original_function(self):
@@ -118,7 +122,16 @@ def _release_shared_library(self):
         # thread closes before the player class is garbage collected, which
         # tends to happen at the end of a script.
         try:
-            shared_library_manager.release(self.original_name, self.index)
+            name = self.original_name
+            index = self.index
+        except AttributeError:
+            # If the Player does finish __init__, because the name of a
+            # non-existent strategy is supplied, a copy of the shared library
+            # won't be loaded, nor will self.original_name or self.index
+            # exist. In that case there's nothing to do.
+            return
+        try:
+            shared_library_manager.release(name, index)
         except FileNotFoundError:
             pass
 

tests/test_player.py

@@ -22,7 +22,7 @@ def test_init():
             POINTER(c_int), POINTER(c_int), POINTER(c_int), POINTER(c_int),
             POINTER(c_float))
         assert player.original_function.restype == c_int
-        with pytest.raises(AttributeError):
+        with pytest.raises(ValueError):
             player = Player('test')
 
 
@@ -186,19 +186,21 @@ def test_multiple_copies(recwarn):
 
 
 def test_match_reproducibility():
-    for _ in range(10):
+    for _ in range(100):
         rng = RandomGenerator()
         seed = rng.random_seed_int()
         strategies = rng.choice(all_strategies, size=2)
-        print(strategies)
         players1 = [Player(strategy) for strategy in strategies]
-        # players1 = (p() for p in strategies)
         match1 = Match(players1, turns=200, noise=0.1, seed=seed)
         results1 = match1.play()
+
         players2 = [Player(strategy) for strategy in strategies]
-        # players2 = (p() for p in strategies)
         match2 = Match(players2, turns=200, noise=0.1, seed=seed)
         results2 = match2.play()
+        if results1 != results2:
+            print(strategies)
+            print(results1)
+            print(results2)
         assert (results1 == results2)
 
 
@@ -221,6 +223,6 @@ def test_tournament_reproducibility():
     test_init()
     test_matches()
     test_noisy_matches()
-    test_implemented_strategies()
+    # test_implemented_strategies()
     test_match_reproducibility()
     test_tournament_reproducibility()