Spurious test failure: test_create_notification

[Thomas De Schampheleire]

Hi,


On Mon, Jun 22, 2015 at 9:59 AM, Thomas De Schampheleire
<patrickdepinguin at gmail.com> wrote:
> Hi,
>
> I'm seeing a spurious test failure on test_create_notification.
> Repeating the test makes it succeed.
>
> Is anyone seeing the same? Any ideas on the cause?
>
> I'm only starting to see this recently, so it must be caused by
> a. a test change
> b. switch to py.test (didn't try with nosetest yet)
>
>
> kallithea/tests/models/test_notifications.py:48: in test_create_notification
>     self.assertEqual([], Notification.query().all())
> E   AssertionError: Lists differ: [] != [<DB:Notification>]
> E
> E   Second list contains 1 additional elements.
> E   First extra element 0:
> E   <DB:Notification>
> E
> E   - []
> E   + [<DB:Notification>]
>

Upon deeper investigation, it turns out that this issue is not so
spurious at all. It looked like that because it really is a bleeding
of state from one test to the other, and repeating the failing test on
its own does not have this bleed.

The issue was introduced with my commit
9a23b444a7fefe5b67e57a42d26343787d992874 pullrequests: detect invalid
reviewers and raise HTTPBadRequest

that added more tests in test_pullrequests.py, and apparently these
tests cause a new notification for the test user.

My idea would be that this is not really a mistake of my tests, but
rather there should never be impact from one test to another. The
database before and after each test should be identical.

Am I naive in assuming this?
How should we fix this?

Thanks,
Thomas