Opened 5 years ago

Closed 5 years ago

#5494 defect closed fixed (fixed)

If gi can't be imported, gtk2 reactor still fails (and vice versa)

Reported by: itamar Owned by: itamar
Priority: normal Milestone: Twisted-12.1
Component: core Keywords:
Cc: jesstess Branch: branches/gtk2-vs-gtk3-5494
branch-diff, diff-cov, branch-cov, buildbot
Author: itamar


#4558 made it impossible to import twisted.internet.gtk2reactor if sys.modules contains a "gi" key. This seems to overlook the fact that even a *failed* import of the gi module adds such a key to that dictionary. Since the test suite tries to test gireactor, there will always be such a key, so gtk2reactor can never be imported by the test suite. The same issue would operate, only in reverse, if gtk2reactor was imported first.

Change History (11)

comment:1 Changed 5 years ago by itamarst

  • Author set to itamarst
  • Branch set to branches/gtk2-vs-gtk3-5494

(In [33588]) Branching to 'gtk2-vs-gtk3-5494'

comment:2 Changed 5 years ago by itamar

  • Keywords review added
  • Owner itamar deleted

comment:3 Changed 5 years ago by itamar

  • Keywords review removed
  • Owner set to itamar

There's an effort to make a pygtk2 compatible wrapper around gi, and the current checks break that:

So, we should make sure we support that too in this branch. Removing from review to ensure that.

comment:4 Changed 5 years ago by itamar

  • Keywords review added
  • Owner itamar deleted

JP wants this in ASAP so we can have gtk2 tests running again, I'll do the compat layer fix separately.

comment:5 Changed 5 years ago by jdahlin

The patch looks good, but I'd switch the order of the parameters preventImports/errorMessage and always pass in the former by keyword, True/False are not really useful at the callsites.

I'd like a way to disable these checks, for gireactor/gtk3reactor, but that's probably another bug as discussed on irc.

comment:6 Changed 5 years ago by exarkun

  • Keywords review removed
  • Owner set to itamar

Sort of an improvement. However, gtk2reactor tests still run on no build slaves (in other words, the issue has not been resolved).

The ones which might possibly run it seem to have a too-old version of gi?

First they skip the gtk3 tests because 'gi.repository.GLib' object has no attribute 'threads_init' then they skip the gtk2 tests because No module named gobject (presumably due to the early behavior of twisted/internet/

I feel as though our buildbot configuration is not doing us any favors here. Do we need an alternate behavior for skipping or skip reporting?

comment:7 Changed 5 years ago by itamar

We are going to have to either setup a specific reactor buildstep for glib2 and gtk2, or setup a specific reactor buildstep for gi+gtk3. Technically we have the former but it has issues because it's too old to run gtk2 correctly!

I reordered things to do the latter; since we don't have a sufficiently new buildslave to use gi anyway, may as well have gtk2 tests running. We will need to setup a buildslave for gi once we have one on a sufficiently modern OS. I will open a ticket for that and jdahlin's pygtk2 compat mode support when this ticket is merged.

comment:8 Changed 5 years ago by itamar

  • Author changed from itamarst to itamar
  • Keywords review added
  • Owner itamar deleted

comment:9 Changed 5 years ago by jesstess

  • Cc jesstess added
  • Owner set to jesstess

comment:10 Changed 5 years ago by jesstess

  • Keywords review removed
  • Owner changed from jesstess to itamar

Thanks for working on this, itamar. The code changes look good to me. A couple of mostly cosmetic comments:

  • preventImports defaults to a tuple but is described as a list in the docstring and is passed a list in the tests:
    +def ensureNotImported(moduleNames, errorMessage, preventImports=()):
  • Can you add @type markup to the parameters in ensureNotImported?
  • missage => message in:
    +    @raises: C{ImportError} with given error missage if a given module name
  • None could get epytext markup in:
    +        If the specified modules have been set to None in C{sys.modules},
  • raise => raises in:
    +        L{ensureNotImported} raise an exception.

comment:11 Changed 5 years ago by itamarst

  • Resolution set to fixed
  • Status changed from new to closed

(In [33732]) Merge gtk2-vs-gtk3-5494: Make sure gtk2 tests run.

Author: itamar Review: exarkun, jesstess Fixes: #5494

If gtk3 was missing, gtk2 tests still wouldn't run. This makes sure they will run. To run gtk3 or gi tests you will need to explicitly choose the applicable reactor for trial.

Note: See TracTickets for help on using tickets.