Skip to content

Commit

Permalink
ensure widgets can be garbage collected
Browse files Browse the repository at this point in the history
To be more test friendly.

Widgets (qargparse widgets in this case) were defined
as class attributes (not in __init__), so it won't be
garbage collected after main window closed, and signals
are still connected. And somehow they still able to
trigger the model which is created in next test session
to do model reset. So how many tests has ran, how many
reset signal will be emitted, to the same model, hence
the test can easily got segfault (especially Linux).
  • Loading branch information
davidlatwe committed Nov 14, 2020
1 parent b0f3224 commit feed9c3
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 118 deletions.
175 changes: 89 additions & 86 deletions allzpark/dock.py
Original file line number Diff line number Diff line change
Expand Up @@ -1060,97 +1060,100 @@ class Preferences(AbstractDockWidget):

icon = "Action_GoHome_32"

options = [
qargparse.Info("startupProfile", help=(
"Load this profile on startup"
)),
qargparse.Info("startupApplication", help=(
"Load this application on startup"
)),

qargparse.Separator("Appearance"),

qargparse.Enum("theme", items=res.theme_names(), help=(
"GUI skin. May need to restart Allzpark after changed."
)),

qargparse.Button("resetLayout", help=(
"Reset stored layout to their defaults"
)),

qargparse.Separator("Settings"),

qargparse.Boolean("smallIcons", enabled=False, help=(
"Draw small icons"
)),
qargparse.Boolean("allowMultipleDocks", help=(
"Allow more than one dock to exist at a time"
)),
qargparse.Boolean("showAdvancedControls", help=(
"Show developer-centric controls"
)),
qargparse.Boolean("showAllApps", help=(
"List everything from allzparkconfig:applications\n"
"not just the ones specified for a given profile."
)),
qargparse.Boolean("showHiddenApps", help=(
"Show apps with metadata['hidden'] = True"
)),
qargparse.Boolean("showAllVersions", help=(
"Show all package versions.\n"
"Profile requested application version range will still be \n"
"respected, but all versions of each dependency package will \n"
"be shown."
)),
qargparse.Boolean("patchWithFilter", help=(
"Use the current exclusion filter when patching.\n"
"This enables patching of packages outside of a filter, \n"
"such as *.beta packages, with every other package still \n"
"qualifying for that filter."
)),
qargparse.Integer("clearCacheTimeout", min=1, default=10, help=(
"Clear package repository cache at this interval, in seconds. \n\n"

"Default 10. (Requires restart)\n\n"

"Normally, filesystem calls like `os.listdir` are cached \n"
"so as to avoid unnecessary calls. However, whenever a new \n"
"version of a package is released, it will remain invisible \n"
"until this cache is cleared. \n\n"

"Clearing ths cache should have a very small impact on \n"
"performance and is safe to do frequently. It has no effect \n"
"on memcached which has a much greater impact on performanc."
)),

qargparse.String(
"exclusionFilter",
default=allzparkconfig.exclude_filter,
help="Exclude versions that match this expression"),

qargparse.Separator("System"),

# Provided by controller
qargparse.Info("pythonExe"),
qargparse.Info("pythonVersion"),
qargparse.Info("qtVersion"),
qargparse.Info("qtBinding"),
qargparse.Info("qtBindingVersion"),
qargparse.Info("rezLocation"),
qargparse.Info("rezVersion"),
qargparse.Info("rezConfigFile"),
qargparse.Info("memcachedURI"),
qargparse.InfoList("rezPackagesPath"),
qargparse.InfoList("rezLocalPath"),
qargparse.InfoList("rezReleasePath"),
qargparse.Info("settingsPath"),
]

def __init__(self, window, ctrl, parent=None):
super(Preferences, self).__init__("Preferences", parent)
self.setAttribute(QtCore.Qt.WA_StyledBackground)
self.setObjectName("Preferences")

self.options = [
qargparse.Info("startupProfile", help=(
"Load this profile on startup"
)),
qargparse.Info("startupApplication", help=(
"Load this application on startup"
)),

qargparse.Separator("Appearance"),

qargparse.Enum("theme", items=res.theme_names(), help=(
"GUI skin. May need to restart Allzpark after changed."
)),

qargparse.Button("resetLayout", help=(
"Reset stored layout to their defaults"
)),

qargparse.Separator("Settings"),

qargparse.Boolean("smallIcons", enabled=False, help=(
"Draw small icons"
)),
qargparse.Boolean("allowMultipleDocks", help=(
"Allow more than one dock to exist at a time"
)),
qargparse.Boolean("showAdvancedControls", help=(
"Show developer-centric controls"
)),
qargparse.Boolean("showAllApps", help=(
"List everything from allzparkconfig:applications\n"
"not just the ones specified for a given profile."
)),
qargparse.Boolean("showHiddenApps", help=(
"Show apps with metadata['hidden'] = True"
)),
qargparse.Boolean("showAllVersions", help=(
"Show all package versions.\n"
"Profile requested application version range will \n"
"still be respected, but all versions of each \n"
"dependency package will be shown."
)),
qargparse.Boolean("patchWithFilter", help=(
"Use the current exclusion filter when patching.\n"
"This enables patching of packages outside of a \n"
"filter, such as *.beta packages, with every other \n"
"package still qualifying for that filter."
)),
qargparse.Integer("clearCacheTimeout", min=1, default=10, help=(
"Clear package repository cache at this interval, in \n"
"seconds.\n\n"

"Default 10. (Requires restart)\n\n"

"Normally, filesystem calls like `os.listdir` are \n"
"cached so as to avoid unnecessary calls. However, \n"
"whenever a new version of a package is released, \n"
"it will remain invisible until this cache is \n"
"cleared. \n\n"

"Clearing ths cache should have a very small impact \n"
"on performance and is safe to do frequently. It has \n"
"no effect on memcached which has a much greater \n"
"impact on performance."
)),

qargparse.String(
"exclusionFilter",
default=allzparkconfig.exclude_filter,
help="Exclude versions that match this expression"),

qargparse.Separator("System"),

# Provided by controller
qargparse.Info("pythonExe"),
qargparse.Info("pythonVersion"),
qargparse.Info("qtVersion"),
qargparse.Info("qtBinding"),
qargparse.Info("qtBindingVersion"),
qargparse.Info("rezLocation"),
qargparse.Info("rezVersion"),
qargparse.Info("rezConfigFile"),
qargparse.Info("memcachedURI"),
qargparse.InfoList("rezPackagesPath"),
qargparse.InfoList("rezLocalPath"),
qargparse.InfoList("rezReleasePath"),
qargparse.Info("settingsPath"),
]

protected = allzparkconfig.protected_preferences()
for name, value in protected.items():
arg = next((a for a in self.options if a["name"] == name), None)
Expand Down
12 changes: 7 additions & 5 deletions tests/test_docks.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ class TestDocks(util.TestBase):

def test_feature_blocked_on_failed_app(self):
"""Test feature blocked if application is broken"""
self.set_preference("showAdvancedControls", True)

util.memory_repository({
"foo": {
"1.0.0": {
Expand All @@ -23,6 +21,8 @@ def test_feature_blocked_on_failed_app(self):
})
self.ctrl_reset(["foo"])

self.set_preference("showAdvancedControls", True)

context_a = self.ctrl.state["rezContexts"]["app_A==None"]
context_b = self.ctrl.state["rezContexts"]["app_B==1"]

Expand Down Expand Up @@ -50,9 +50,6 @@ def test_version_editable_on_not_show_all_versions(self):
self._test_version_editable(show_all_version=False)

def _test_version_editable(self, show_all_version):
self.set_preference("showAdvancedControls", True)
self.set_preference("showAllVersions", show_all_version)

util.memory_repository({
"foo": {
"1": {"name": "foo", "version": "1",
Expand All @@ -67,6 +64,11 @@ def _test_version_editable(self, show_all_version):
"2": {"name": "bar", "version": "2"}}
})
self.ctrl_reset(["foo"])

self.set_preference("showAdvancedControls", True)
self.set_preference("showAllVersions", show_all_version)
self.wait(200) # wait for reset

self.select_application("app_B==1")

dock = self.show_dock("packages")
Expand Down
28 changes: 1 addition & 27 deletions tests/util.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

import os
import time
import unittest
import contextlib

Expand Down Expand Up @@ -43,45 +42,20 @@ def setUp(self):
self.wait(timeout=50)

def tearDown(self):
self.wait(timeout=500)
self.wait(timeout=200)
self.window.close()
time.sleep(0.1)

def set_preference(self, name, value):
"""Setup preference
This should be called before ctrl reset.
(NOTE) Some preference change may calling ctrl.reset, so ctrl.reset
will be monkey-patched to do nothing, this is to prevent error
raised from resting without profile.
If not doing this, and setup preference after reset with test
profiles, calling reset again on preference changed may leads
to some weird profile model reset error. (item.internalPointer
returning random object and AttributeError raised)
Args:
name: preference name
value: preference value
Returns:
None
"""
preferences = self.window._docks["preferences"]
arg = next((opt for opt in preferences.options
if opt["name"] == name), None)
if not arg:
self.fail("Preference doesn't have this setting: %s" % name)

origin_reset = getattr(self.ctrl, "reset")
setattr(self.ctrl, "reset", lambda: None)
try:
arg.write(value)
except Exception as e:
self.fail("Preference '%s' set failed: %s" % (name, str(e)))
finally:
setattr(self.ctrl, "reset", origin_reset)

def show_dock(self, name, on_page=None):
dock = self.window._docks[name]
Expand Down

0 comments on commit feed9c3

Please sign in to comment.