Code review comment for lp://staging/~allanlesage/unity8/autopilot-indicator-page-title-matches-widget

Revision history for this message
Christopher Lee (veebers) wrote :

Hi Allan, just a couple of comments.

I would make a DefaultIndicatorWidget emulator,

  - to this add the swipe_to_open and swipe_to_close methods
  - perhaps also add get_center for now (although as mentioned there is a better
    place).
    -> Yes you're correct that you'll need to provide the coords, unlike
       click_object which determines the center itself.

Then there is no need for an IndicatorBaseTestCase class as the helpers are part
of the emulator/proxy that needs them.

line:58, 61 cleanup not needed as drag is a "finger down, move and finger up"
operation.
line:46-49: not needed (as per above)

line:120 -> should have the skip within the setup (not decorated).
i.e.
def setUp(self):
  if model() == "Desktop":
      self.skipTest("Test cannot be run on the desktop.")

line:126 -> Docstring
  The docstring should be descriptive not narrative.
  "Swiping open an indicator must show the correct title."

line:140
  This assert is not needed, if there is nothing found for the criteria
  (i.e. type and title text in this case) wait_select_single will raise an
  exception (thus failing the test.)

review: Needs Fixing

« Back to merge proposal