Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Update unit tests to not use instance level variables to prevent state presisting between tests #33

Open
SebDuff opened this issue Oct 15, 2020 · 0 comments
Labels
bug Something isn't working

Comments

@SebDuff
Copy link
Collaborator

SebDuff commented Oct 15, 2020

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: #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))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant