Currently, we are creating the default data in the class method (@classmetod). This creates a single object instance for each of the variables, which are then referenced by all of the subsequent tests. That means that any data which references these variables will effectively mean the test is no longer self-contained and running in isolation.
For example:
https://github.com/FootyStats/footy/blob/master/tests/domain/test_Competition.py
class MyTestCase(unittest.TestCase):
@classmethod
def setUpClass(cls):
cls.CODE = 'Test'
cls.NAME = 'Test League'
cls.TEAMS = [Team('Arsenal', 64, 36, 18, 19, 69),
Team('Stoke', 37, 51, 19, 18, 45)]
cls.START_DATE = '2020-09-25T15:00:00Z'
cls.END_DATE = '2021-02-13T21:30:00Z'
cls.STAGE = 'Group'
cls.FIXTURES = []
def competition_under_test_producer(self):
# Creates a new Competition object which uses a reference to the class level cls.TEAMS collection
return Competition(self.CODE, self.NAME, self.TEAMS, self.START_DATE, self.END_DATE,
self.STAGE, self.FIXTURES)
def test_add_team_returns_new_team(self):
team = Team('A', 1, 2, 3, 4, 5)
# Create a new Competition object, but that object has a reference to class level TEAMS collection
# len(self.TEAMS) == 2
competition = self.competition_under_test_producer()
# Add a team to the class level TEAMS collection
competition.add_team(team)
# len(self.TEAMS) == 3
self.assertTrue(team in competition.teams)
def test_add_team_only_adds_once(self):
team = Team('A', 1, 2, 3, 4, 5)
# Create a new Competition object, but that object has a reference to class level TEAMS collection
# len(self.TEAMS) == 3 NOT 2 as we would expect
competition = self.competition_under_test_producer()
num_teams = len(competition.teams)
# When competition.add_team(team) tests for presence of a team by reference, this adds another team.
# When testing by value of Team, this does nothing
competition.add_team(team)
# Reference: len(self.TEAMS) == 4
# Value: len(self.TEAMS) == 3
competition.add_team(team)
# Reference: len(self.TEAMS) == 4
# Value: len(self.TEAMS) == 3
self.assertTrue(team in competition.teams)
# Hence, this assertion is true when add_team tests for reference, and false when testing for value
self.assertEqual(num_teams + 1, len(competition.teams))
This test was only working because the Competition.add_team(Team) method on was checking for the presence of an object with the same reference. Because the Team object had a different reference (but the same data), it was allowing another "duplicate" team to be added, but only because the object reference was different. after overriding the __eq__ method on Team to compare by data (rather than reference), Competition.add_team(Team) started (correctly) not adding the team because there was already an existing team with the same data.
This may be the only place which we have been stung by this for now, but we should update all of the other tests to also only use the class level references when asserting or not changing values.
My initial fix isn't the most elegant, but as there are no true constants in Python, I'm not sure what the best approach is (maybe not using class level expected values at all?
Fix for test_competition: https://github.com/FootyStats/footy/pull/34
Example of fixed https://github.com/FootyStats/footy/blob/master/tests/domain/test_Competition.py:
class TestCompetition(unittest.TestCase):
@classmethod
def setUpClass(cls):
# only use for assertions and when the values of competition will not change
cls.EXPECTED_COMPETITION = Competition('Test', 'Test Competition', [Team('Arsenal', 64, 36, 18, 19, 69),
Team('Stoke', 37, 51, 19, 18, 45)],
'2020-09-25T15:00:00Z', '2021-02-13T21:30:00Z', 'Group', [])
def competition_under_test_producer(self):
return Competition('Test', 'Test Competition', [Team('Arsenal', 64, 36, 18, 19, 69),
Team('Stoke', 37, 51, 19, 18, 45)],
'2020-09-25T15:00:00Z', '2021-02-13T21:30:00Z', 'Group', [])
def test_add_team_adds_new_team(self):
team = Team('A', 1, 2, 3, 4, 5)
competition = self.competition_under_test_producer()
competition.add_team(team)
self.assertTrue(team in competition.teams)
def test_add_team_does_not_add_duplicates(self):
team = Team('A', 1, 2, 3, 4, 5)
competition = self.competition_under_test_producer()
num_teams = len(competition.teams)
competition.add_team(team)
competition.add_team(team)
self.assertTrue(team in competition.teams)
self.assertEqual(num_teams + 1, len(competition.teams))
Currently, we are creating the default data in the class method (
@classmetod
). This creates a single object instance for each of the variables, which are then referenced by all of the subsequent tests. That means that any data which references these variables will effectively mean the test is no longer self-contained and running in isolation.For example:
https://github.com/FootyStats/footy/blob/master/tests/domain/test_Competition.py
This test was only working because the
Competition.add_team(Team)
method on was checking for the presence of an object with the same reference. Because theTeam
object had a different reference (but the same data), it was allowing another "duplicate" team to be added, but only because the object reference was different. after overriding the__eq__
method onTeam
to compare by data (rather than reference),Competition.add_team(Team)
started (correctly) not adding the team because there was already an existing team with the same data.This may be the only place which we have been stung by this for now, but we should update all of the other tests to also only use the class level references when asserting or not changing values.
My initial fix isn't the most elegant, but as there are no true constants in Python, I'm not sure what the best approach is (maybe not using class level expected values at all? Fix for test_competition: https://github.com/FootyStats/footy/pull/34
Example of fixed
https://github.com/FootyStats/footy/blob/master/tests/domain/test_Competition.py
: