Code review comment for lp://staging/~coreygoldberg/window-mocker/py3

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Hi,

The latest revision includes several defects:

1) Running 'window-mocker' just produces an error and exits:

$ window-mocker
ERROR:windowmocker.plugins:No plugin was found with name '<function get_default_plugin_name at 0x1618398>'
Error: Unable to process request: Type name '<function get_default_plugin_name at 0x1618398>' is invalid

2) Running 'window-mocker3' produces similar errors:

$ window-mocker3
ERROR:windowmocker.plugins:No plugin was found with name '<function get_default_plugin_name at 0x7f90a55847a0>'
Error: Unable to process request: Type name '<function get_default_plugin_name at 0x7f90a55847a0>' is invalid

For both points 1) and 2), the desired behavior is that it should just open the window with the default plugin (qt4 in both cases).

3) Running 'window-mocker -h' lists the 'Qt5' plugin as an available option, but that's incorrect - Qt5 is not available in python 2. If you try and actually run it in this configuration, it crashes:

$ window-mocker -t Qt5
Traceback (most recent call last):
  File "/usr/bin/window-mocker", line 54, in <module>
    main()
  File "/usr/bin/window-mocker", line 25, in main
    application = create_application_from_data(app_type_name=args.type)
  File "/usr/lib/python2.7/dist-packages/windowmocker/__init__.py", line 88, in create_application_from_data
    app_instance = app_plugin(data)
  File "/usr/lib/python2.7/dist-packages/windowmocker/plugins/__init__.py", line 20, in create_qt5_plugin
    from windowmocker.plugins.qt5 import QtPlugin as Qt5Plugin
  File "/usr/lib/python2.7/dist-packages/windowmocker/plugins/qt5.py", line 10, in <module>
    from PyQt5 import QtCore, QtWidgets
ImportError: No module named PyQt5

The Qt4 plugin should not be listed as an option in python2.

4) Running 'window-mocker3 -t Qt5' does indeed produce a Qt5 window (yay!), but closing that window causes window-mocker3 to crash (segfault, in fact, which is impressive, for a python application).

Given that these regressed, it's probably a good idea to add functional tests for all these issues before fixing them.

review: Needs Fixing

« Back to merge proposal