Segmentation fault on tests/TaskManagement/TestHttpRequestManager.py::test_getBasicAuthSuccess #594
Comments
|
I'm unable to reproduce this. The tests succeed for me, apart from the typing check which fails on Trust.py for me locally (as I have a more modern version of Mypy than our CI server). The line in question is So that means that somewhere Qt is getting a segfault. That should never happen. We may need to bump this up to Qt or Riverbank. |
|
I'm trying to package EDIT: what's suspicious is the test pass when executed one by one, which makes me think somewhere there's global state that makes them failing when running the entire suite. |
|
Maybe something in PyQt then? It looks like it creates multiple You could try to move that |
|
I would never expect to be able to create and destroy QCoreApplication multiple times. |
|
The application is re-created for every test in order to prevent previous tests from influencing subsequent tests. If they did, that would create some particularly nasty bugs in the tests that are hard to track down. Those bugs would also be exclusive to the tests, so fixing them is not making the application any better. For me though and our CI server, this destroying and re-creating works fine. The Qt framework should allow that sort of thing. |
Then you should also restart the python runtime, because it may contain some leftover state. Recreating the HttpRequestsManager should be completely sufficient.
You do not recreate the QApplication in the real application, so why do you do in the tests? If deleting the application were a requirement to make requests independent, then the tests would succeed, but the same requests in the real application would fail because there is some leftover state.
Which version of Qt are you using? Have you tested Qt 5.12 and Qt 5.15, which are the current LTS releases? (And Qt 5.15 will be the last Qt5 version ever.) |
|
Yeah, we should restart the Python runtime, but that's not how PyTest works. Understandably, most unit tests wouldn't need this sort of thing and they'd claim that tests are expected to clean up after themselves.
We're currently on Qt 5.10.2. This is necessary to support some old MacOS versions that we still have a lot of users on. As that is dwindling though, we have a mind to upgrade to 5.12, which would solve a few open bugs (in particular a nasty one that crashes Cura if people use different GPUs for different screens). |
|
So, how about:
I could spend a little bit of time on this, iff this is deemed a sensible route forward and able to be merged. |
|
Yeah that is probably the only way to fix this. The difficulty is probably in the 2nd point there. The HttpRequestManager in particular uses multithreading so you could end up with nondeterministic tests pretty quickly. |
With Uranium 4.5.0, I get the following test failure:
The text was updated successfully, but these errors were encountered: