Create new dummy webview backend, fix unit-test launching #3938
Conversation
- The mode= keyword arguments to OpenShotApp and MainWindow are removed - Instead, a variable info.LAUNCH_MODE is created, defaulting to "gui" - Many more parts of the startup (font and color settings, backup recovery) are made dependent on the value of info.LAUNCH_MODE - The settings object from OpenShotApp is also passed in to ui_util.load_theme(), reducing dependencies on classes.settings Configure application in launch.py, not app class Fix indentation of configure_gui Import correct log object
The test runner will need to be run with '-platform minimal' or '-platform offscreen' on systems where the default platform can't be used (e.g. Linux CI containers, where the default is xcb and an X server connection is required)
| # Set Font for any theme | ||
| if self.settings.get("theme") != "No Theme": | ||
| # Load embedded font | ||
| try: | ||
| log.info("Setting font to %s" % os.path.join(info.IMAGES_PATH, "fonts", "Ubuntu-R.ttf")) | ||
| log.info("Setting font to %s", os.path.join(info.IMAGES_PATH, "fonts", "Ubuntu-R.ttf")) |
There was a problem hiding this comment.
Does this still need the %s placeholder?
There was a problem hiding this comment.
It does. logging's formatter processes template strings with % escapes, the remaining arguments are the inputs to the formatting.
There's one key difference between passing the format string and inputs as arguments, and doing the string interpolation up front: logging calls benefit from lazy evaluation. If logging is disabled (or the logging.INFO level isn't configured for output), not only will the final, formatted string not be generated, but the os.path.join() won't even be executed, since its output isn't needed.
"Setting font to %s" % os.path.join(info.IMAGES_PATH, "fonts", "Ubuntu-R.ttf") always has to do the os.path.join() and generate the string, in order to pass it to the log.info() call — whether or not it will ultimately be logged.
| if callback: | ||
| callback("{}") | ||
| else: | ||
| return None |
There was a problem hiding this comment.
nitpick, empty line at end of file 😉
|
Thanks @ferdnyc ! This looks really nice! I had 1 question, but otherwise, I say merge away once you are ready! |
|
I'm trying to merge in things that look very critical / and not too scary. I want to create a release branch soon. 🤞 |
|
Merge conflicts have been detected on this PR, please resolve. |
|
I believe I have a better way to do this (see ferdnyc/app-launch), so I'm going to close this PR. |
This PR changes the application startup substantially to make it more resilient. Specifically, it:
webview_backend.dummy, containing aDummyWebViewclass derived fromQWidgetand providing a stubrun_jsmethod. This webview can be used in place of the existing WebKit and WebEngine options in situations where the webview is not expected to display anything.mode="unittest"argument toOpenShotAppandMainWindowwith an info variable,info.LAUNCH_MODE. The default value is"gui", and many more things in the startup are made dependent on the value ofinfo.LAUNCH_MODE.QApplicationconfiguration previously being done at the top ofclasses.app, and instead does it at the top oflaunch.py. This ensures that the necessary settings/imports will be completed before instantiating theQApplication-derivedOpenShotAppobject.query_tests.pyto setinfo.LAUNCH_MODE="unittest"andinfo.WEB_BACKEND="dummy", as well asinfo.LOG_LEVEL_CONSOLE="WARNING", so that startup will be constrained and logging output will be kept to a minimum during the unit test runs. This prevents crashing due to unnecessarily loaded GUI elements (especially the QML-based QWebEngineView) and keeps the unit test runs streamlined for CI use.query_tests.pyto process Qt standard command line arguments, and adds-platform minimalto the arguments when the unit tests are run in the Github Actions CI. This is necessary when executing the code in an environment without an X server, and is a lighter alternative to thexvfbvirtual-server environment that was previously used.This PR also incorporates the change from #3937, eliminating the attempts to communicate with Ubuntu Unity.