Hi all. Raoul asked me to chime in on this conversation about moving to pytest 
fixtures. 

I ran into a whole bunch of segfaults when running the TestOpenLP test class. 
(tests/functional/openlp_core/test_app.py) Raoul's link #4 is a link to my 
merge request that ran into this issue. this gitlab comment 
[https://gitlab.com/openlp/openlp/merge_requests/19#note_224757584] and the 
containing thread have a more thorough explanation of the issue and some ideas 
about what might be causing the segfaults.

The main idea behind pytest fixtures is very similar to the existing setUp and 
tearDown method overrides on the TestCase class. They essentially provide a 
method for setting up and tearing down the environment and required objects for 
a particular test. The practical upshot of using pytest fixtures instead of the 
setUp and tearDown methods is that they are the 'official' pytest way of 
setting up and tearing down tests and I have found that pytest has far better 
failure messages when running a 'pytest' test versus a 'unittest' test.

I had a branch with the TestOpenLP class rewritten to use pytest fixtures that 
passed all tests locally. So it is completely doable given enough time. 
However, I did run into the segfaults when I started rewriting more of the 
tests to use pytest fixtures. The linked merge request touches on this issue a 
bit. 

The basic problem is that the QApplication object is only meant to be 
instantiated once for the lifetime of the application, which is what happens 
during the normal execution of OpenLP, but during testing, the QApplication is 
created and destroyed many times and references to the object may not always be 
torn down between tests. I personally suspect (though I have not had a chance 
to fully investigate) that the Settings object or some of the singleton objects 
may be keeping references to the QApplication object. (for instance, I know 
that the Registry singleton is explicitly given a reference to the QApplication 
object and I don't think the Registry object is destroyed between tests). This 
makes it very difficult to pinpoint the actual point of failure for a test 
because the point of failure may be caused as a side effect of another test 
failing to tear the environment down properly. (i.e. one that initialized the 
Registry and gave it a QApplication reference, but did not reset the Registry).

I'm not sure what to do about this underlying problem.

The migration to pytest fixtures, if done carefully, might be able to help 
solve the QApplication object's problems. (As an important note, the 
QAppplication problem only arises in the tests as a side effect of running 
multiple tests with new/shared QApplication object instances. It does not occur 
in the regular execution of OpenLP because only one QApplication object is ever 
created.)

A potential solution could be to manage some of the singleton classes 
differently. This would cut down on the potential for references to leak 
between tests.

Thanks and sorry about the long message.

Ben
On 10/8/2019 4:42:43 PM, Raoul Snyman via openlp-dev <[email protected]> 
wrote:
Hello everyone!

A while back we decided to switch to using pytest[0] for running our
tests. It's much faster than nose[1], and pretty actively developed. For
the most part, pytest can run tests written for the built-in unittest[2]
framework (which is what nose uses). Unfortunately there seem to be some
unintended consequences of this, and that is that sometimes the OpenLP
application object is not disposed of correctly, causing segfaults.
We've been skipping the tests for the OpenLP application object because
of this[3].

Benjamin was running into this issue[4], and being familiar with pytest,
figured he should use pytest fixtures (aka, The Right Way) to fix this
problem. Because of our history of nose -> nose2 -> pytest, we haven't
actually converted our tests to use pytest fixtures[5]. You can think of
a fixture as a way to run the setUp and tearDown methods without needing
a class.

I'd like to propose that we move to pytest fully. This, like all things,
will need to be an incremental change, but I reckon we can probably
start with the OpenLP application object tests.

What do you guys think?


[0] https://pytest.org/en/latest/
[1] https://docs.nose2.io/en/latest/
[2] https://docs.python.org/3/library/unittest.html
[3]
https://gitlab.com/openlp/openlp/blob/master/tests/functional/openlp_core/test_app.py#L161
[4] https://gitlab.com/openlp/openlp/merge_requests/19
[5] https://pytest.org/en/latest/fixture.html

--
Raoul Snyman
[email protected]
_______________________________________________
openlp-dev mailing list
[email protected]
https://lists.openlp.io/mailman/listinfo/openlp-dev
_______________________________________________
openlp-dev mailing list
[email protected]
https://lists.openlp.io/mailman/listinfo/openlp-dev

Reply via email to