spv-dev team mailing list archive

["F.-A. Bourbonnais" <fab.dev@xxxxxxxxxxx>]

Review: Needs Fixing
  - The field name "screen_information_provider" is confusing. It's too similar to ScreenProviderQt. I think that we should call it qt_desktop_widget since the code in the __init__ will only work with a QDesktopWidget.

  - The name of the Mock class in "preferred_projectors_steps.py" should be renamed too. I really have difficulties to follow what is mocked (ScreenProviderQt or the QDesktopWidget)?

  - At the system test level, I don't see anything that could test that PreferredProjectors is really notified when a screen is plugged. 

    For example, if the event is renamed in screen.py, no test will fail if the method in PreferredProjectors is not renamed too. 

    IMO, in preferred_projectors_steps.py the only thing that should be mocked is the QDesktopWidget not the ScreenProviderQt. Because if not, there is nothing to test the integration.

  - Why screen.py is in the widget package?

  - tests_screen: _notify_screen_changed is not tested. 
      a) Should be triggered on the QDesktopWidget event
      b) Should notify observers

  - tests_screen: convert_to_qt_screen_number is not tested.
-- 
https://code.launchpad.net/~claude-bolduc/slidepresenterview/choose-projector-screens/+merge/45326
Your team SlidePresenterView Development Team is subscribed to branch lp:slidepresenterview.