GNOME Bugzilla – Bug 688219
Move pygtkcompat into sub-package or sibling package of gi
Last modified: 2012-11-19 13:28:54 UTC
I would like to include an implementation of GenericTreeModel along with the pygtkcompat module. However, dumping more stuff within the base gi package seems messy. Ideally we could make pygtkcompat a sibling of gi so we would have: pygobject/ gi/* pygtkcompat/* There are two benefits with this approach: 1) Makes it easier for pygtkcompat to become its own project at some point. 2) Allows separation of unittests. Currently enabling pygtkcompat will taint the Gtk namespace making testing of Gtk overrides more difficult and also allows for unknown override test dependencies. Alternatively we can move pygtkcompat from a sub-module to a sub-package: pygobject/ gi/ pygtkcompat/__init__.py If we can move pygtkcompat tests into a new tests folder within this, it can work just as well as the above approach: pygobject/gi/pygtkcompat/tests/test_pygtkcompat.py
Created attachment 229336 [details] [review] Work in progress This is a work in progress and I am mostly looking for feedback on the idea.
I like the idea of moving pygtkcompat into a separate namespace, although putting everything into __init__.py looks odd. Usually you would have a pygtkcompat/gtk.py, pygtkcompat/glib.py etc., and in the future pygtkcompat/treemodel.py, and __init__.py just imports/exports the exported API (such as the class names). What I don't like is splitting out the tests as well. It not only makes those tests harder to find, it also duplicates the test environment logic. I see no problem with keeping the pygtkcompat tests in tests/, what is the motivation for moving them? Thanks!
(In reply to comment #2) > I like the idea of moving pygtkcompat into a separate namespace, although > putting everything into __init__.py looks odd. Usually you would have a > pygtkcompat/gtk.py, pygtkcompat/glib.py etc., and in the future > pygtkcompat/treemodel.py, and __init__.py just imports/exports the exported API > (such as the class names). Ok, I will move stuff around a bit more. > What I don't like is splitting out the tests as well. It not only makes those > tests harder to find, it also duplicates the test environment logic. I see no > problem with keeping the pygtkcompat tests in tests/, what is the motivation > for moving them? The reason is pygtkcompat monkey patches overrides into Gdk and Gtk. This taints the namespaces and class objects so when test_overrides_gtk/gdk run later, they will be using the pygtkcompat version of some stuff. This depends on the order the tests are run (I'm seeing test_pygtkcompat as one of the first tests being run). However, this does not currently seem to be a problem. But I think it is a messy setup that will lead to either a cross dependency on a monkey patch and/or hard to debug situations like test_overrides_gtk working when running all tests but failing when run standalone. There might be way around this by using module level fixtures (setUpModule and tearDownModule) and unload all of what gi has loaded after test_pygtkcompat runs within tearDownModule. I will look into this a little deeper to see if it is possible.
Ah, thanks for the explanation. So could we perhaps change tests/Makefile.am to run them separately, but still keep them in tests/ ?
I think the issue with that is test discovery happens in runtests.py which is called from the Makefile. So the separate runs would need to pass additional information to runtests.py to include/exclude test_pygtkcompat explicitly.
Created attachment 229366 [details] [review] Move pygtkcompat into sibling package of gi Simplified patch which just does the move. We can break pygtkcompat up into smaller modules in a later commit. Also do not attempt to fix pygtkcompat tainting of modules as this can also be fixed in a later commit.
Thanks Simon! I pushed a change to run the pygtk compat tests separately: http://git.gnome.org/browse/pygobject/commit/?id=e617f76e5b0c301c3ae92e1091aa86792de4d8e8
With that some tests fail in Python 2 as they assumed the pygtkcompat behaviour of changing the default system encoding. So it's definitively worth running the tests separately, thanks for catching! I fix the tests for python 2 now.