Test failures can trigger unrelated test failures

[ElDeveloper]

Since not all test classes cleanup the database, some tests follow a pattern where they first test the matter at hand, and then do any cleanup that may be needed. This is fine if the test passes, however if the test fails the cleanup won't happen and the database may be in an inconsistent state (or a different state from what other tests expect).

For example in qiita_db/test_study.py the TestStudy class does something along the lines of:

def test_get_info(self):
    # 1.- do some testing
    self.assert.....
    # 2.- create a new study
    Study.create(....)
    # 3.- do additional testing
    self.assert......
    # 4.- delete the additional study
    Study.delete(......)

Ideally, the created studies or created objects should be deleted in the tearDown class (to guarantee that the studies are cleaned up).

[antgonza]

I can see why this as something that will be desired, in theory. However, in praxis, not sure if actually needed. I mean, this will only fail if we make some change to the code and if we do, the chances are that other things will fail (which is not important); thus, the final cleanup of the tests will clean everything so we are ready to rerun more tests. In other words, we will never stay in an unstable state because the final cleanup and these deletes are just so they don't interfere with other tests within the same class.

Hope this makes, if not let me know.

[ElDeveloper]

I see, that makes sense. I've corrected the title to refer to the only outstanding issue. Which is that when this pattern is in use a failure in one test can lead to additional unrelated failures in other tests (within the same class).

[antgonza]

Thanks. However, I wonder if this is actually something that needs fixing, as it will only failed if we changed something in the code that we shouldn't have and the regular "testing" will fail due to this test ... which is what we expect, right?

[ElDeveloper]

I don't think its a fair expectation that when you run unit tests a failure in one isolated test case leads to N other failures. This pattern makes debugging harder.

[antgonza]

@ElDeveloper, during this release iteration there has been a lot of clean up from the tests so all of them clean after themself, keeping a tidy environment, and only fail/affect within a single test file. In other words, if you add new independent code/function, and an independent test for this new code; nothing will fail. However, if you modify behavior on a given function, let's say A, that is used by other methods, let's say X and Y, then that will trigger errors in X and Y if the results are changed based on the changes in A; resulting in having to change test for A, X and Y; which I think this is desirable. Does this sound like a good compromise?

[ElDeveloper]

Yeah, that seems reasonable. Thanks!

[antgonza]

Thank you!