Giter VIP home page Giter VIP logo

Comments (9)

Ghostkeeper avatar Ghostkeeper commented on June 16, 2024

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 app.exec() on a QCoreApplication:

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.

from uranium.

gferon avatar gferon commented on June 16, 2024

I'm trying to package python-uranium for cura 4.5 and 4.6 for Fedora, and I'm getting the same segfault. I'm currently trying to investigate but came to the same conclusion as you did @Ghostkeeper (that it should never happen).

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.

from uranium.

Ghostkeeper avatar Ghostkeeper commented on June 16, 2024

Maybe something in PyQt then? It looks like it creates multiple QCoreApplication instances within the same run of Python now (due to other tests creating one too). Maybe they don't properly get destroyed afterwards, or they indeed alter some global state in Qt somewhere?

You could try to move that QCoreApplication to a singleton, maybe, but the tests call the function quit() on it too so that could spell trouble.

from uranium.

StefanBruens avatar StefanBruens commented on June 16, 2024

I would never expect to be able to create and destroy QCoreApplication multiple times.
One QCoreApplication seems the much better option, maybe with a QEventLoop for each test if multiple event loops are deemed necessary.

from uranium.

Ghostkeeper avatar Ghostkeeper commented on June 16, 2024

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.

from uranium.

StefanBruens avatar StefanBruens commented on June 16, 2024

The application is re-created for every test in order to prevent previous tests from influencing subsequent tests.

Then you should also restart the python runtime, because it may contain some leftover state. Recreating the HttpRequestsManager should be completely sufficient.

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.

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.

For me though and our CI server, this destroying and re-creating works fine. The Qt framework should allow that sort of thing.

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.)

from uranium.

Ghostkeeper avatar Ghostkeeper commented on June 16, 2024

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.

Which version of Qt are you using?

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).

from uranium.

StefanBruens avatar StefanBruens commented on June 16, 2024

So, how about:

  1. One QApplication fixture with session scope - resembles actual usage in cura
  2. Make sure the manager does not leak state, i.e. tests still pass.

I could spend a little bit of time on this, iff this is deemed a sensible route forward and able to be merged.

from uranium.

Ghostkeeper avatar Ghostkeeper commented on June 16, 2024

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.

from uranium.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.