Merge lp:~allanlesage/unity8/autopilot-indicator-page-title-matches-widget into lp:unity8

Proposed by Allan LeSage
Status: Merged
Approved by: Michał Sawicz
Approved revision: 527
Merged at revision: 826
Proposed branch: lp:~allanlesage/unity8/autopilot-indicator-page-title-matches-widget
Merge into: lp:unity8
Prerequisite: lp:~allanlesage/unity8/indicator-stubs
Diff against target: 185 lines (+109/-15)
3 files modified
tests/autopilot/unity8/indicators/tests/__init__.py (+17/-0)
tests/autopilot/unity8/indicators/tests/test_indicators.py (+69/-13)
tests/autopilot/unity8/shell/emulators/main_window.py (+23/-2)
To merge this branch: bzr merge lp:~allanlesage/unity8/autopilot-indicator-page-title-matches-widget
Reviewer Review Type Date Requested Status
Michał Sawicz Approve
PS Jenkins bot (community) continuous-integration Approve
Leo Arias (community) code review Approve
Nick Dedekind (community) Approve
Albert Astals Cid (community) Needs Information
Christopher Lee (community) Approve
Review via email: mp+196991@code.launchpad.net

Commit message

Swiping open an indicator must show its correct title--protect against lp:1253804 .

Description of the change

When I swipe open an indicator, the correct indicator page opens. This should protect against lp:1253804 .

To post a comment you must log in.
Revision history for this message
Allan LeSage (allanlesage) wrote :

Some questions for reviewers:

dednick, can you advise on the best way to assert an indicator page is open? Concerns ll. 59 and 69.

veebers, does the 'scenario' form of this added test make sense, or should it be mashed into the original IndicatorTestCase ?

veebers, can you comment on the necessity of get_center utility function when using a Touch Pointer object? I see that there are utilities for pressing the centers of buttons, etc., but in any case I'll need an origin point to perform a drag, correct?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:503
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~allanlesage/unity8/autopilot-indicator-page-title-matches-widget/+merge/196991/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity8-ci/1763/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1224/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1198/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/454
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/286
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/287
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/287/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/286
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1088/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1224
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1224/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1198
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1198/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3754/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1872

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1763/rebuild

review: Needs Fixing (continuous-integration)
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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:503
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~allanlesage/unity8/autopilot-indicator-page-title-matches-widget/+merge/196991/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity8-ci/1774/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1257
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1231/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/465/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/297
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/298
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/298/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/297
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1257
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1257/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1231
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1231/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3784/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1904

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1774/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:503
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~allanlesage/unity8/autopilot-indicator-page-title-matches-widget/+merge/196991/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity8-ci/1781/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1278
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1252/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/478
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/304
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/305
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/305/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/304
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1134
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1278
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1278/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1252
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1252/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3800/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1920

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1781/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:506
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~allanlesage/unity8/autopilot-indicator-page-title-matches-widget/+merge/196991/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity8-ci/1809/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1402
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1371/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/543
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/332
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/333
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/333/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/332
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1247
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1402
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1402/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1371
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1371/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3905/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2034

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1809/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Christopher Lee (veebers) wrote :

The acceptance tests look good to me.

Not happy about the "# TODO: assert that the indicator page closed" but as they're not required for this I will approve.

Please file a bug against the tests to fix the TODOs.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

/tmp/buildd/unity8-7.84+14.04.20131128.2/tests/autopilot/unity8/indicators/emulators/widget.py: bad whitespace in line 31

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:511
http://jenkins.qa.ubuntu.com/job/unity8-ci/1836/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1485
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1447/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/587
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/359
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/360
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/360/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/359
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1322
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1485
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1485/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1447
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1447/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3977/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2132

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1836/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Leo Arias (elopio) wrote :

53 +

A small pita comment, I don't like that space :)

64 + self.globalRect[0]+int(self.globalRect[2]/2),
65 + self.globalRect[1]+int(self.globalRect[3]/2)

You now can do globalRect.x, globalRect.y, globalRect.height, globalRect.width. That makes it clearer.

77 + # TODO: assert that the indicator page opened

A nice thing that might be useful in the future is signing the comments like that, using -- alesage - 2013-12-06.

153 + self.assertIsNotNone(widget)
189 + self.assertIsNotNone(widget)

Please notice that select single will throw an exception now if it doesn't find the object, instead of returning None as it did before. So I think that line is not necessary, if the widget is not present, an exception will be raised and the test will fail.

The test reads nicely. I'm just wondering if this can be run faster as a QtTest. But I'm not sure how the indicators are implemented, if this involves several projects, autopilot is probably the best option.

Marking as needs fixing only for the assertIsNotNone.
Thank you!

review: Needs Fixing (code review)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:512
http://jenkins.qa.ubuntu.com/job/unity8-ci/1844/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1513
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1471/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/601
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/367
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/368
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/368/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/367
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1345
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1513
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1513/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1471
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1471/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3998/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2154

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1844/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Allan LeSage (allanlesage) wrote :

Leo's on holiday but I've implemented his suggested changes; Saviq, are the unstable tests holding us back? (Note that dednick is also on holidays.)

Revision history for this message
Albert Astals Cid (aacid) wrote :

FWIW dednick is not on holidays anymore (or at least was in yesterday)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:512
http://jenkins.qa.ubuntu.com/job/unity8-ci/1901/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1729
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1653/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/687
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/424
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/425
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/425/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/424
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1528
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1729
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1729/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1653
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1653/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4160/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2366

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1901/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

What's the status of this?

Is it supposed to be held only by failing in CI?

I tried to rebuild the last rev but CI doesn't know about the job anymore.

Allan maybe you can merge master and push to get a rebuild?

review: Needs Information
Revision history for this message
Michał Sawicz (saviq) wrote :

This is held by the issues with the prerequisite: https://code.launchpad.net/~allanlesage/unity8/indicator-stubs/+merge/192059/comments/464049

Bug #1238417 needs to be fixed for this to continue :/

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:623
http://jenkins.qa.ubuntu.com/job/unity8-ci/2051/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2225
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2110/console
    None: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/890/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/573
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/575
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/575/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/573
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1945
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2227
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2227/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2110
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2110/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4572/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2970

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2051/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:512
http://jenkins.qa.ubuntu.com/job/unity8-ci/2053/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2247
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2132
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/897
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/575
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/577
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/577/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/575
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1970
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2249
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2249/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2132
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2132/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4591
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2990

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2053/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

ATM the above basically says that the job passed, otto has issues with running u8 tests.

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

While it all looks ok codewise, I think the opening/closing of the indicators should be provided by a higher level component than the widget itself, such as an Indicators emulator object, which could provide an open_indicator(identifier) function.

Marking as approved; emulators can be expanded later.

review: Approve
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Just waiting on test suite run before global approve :)

Revision history for this message
Allan LeSage (allanlesage) wrote :

Apologies for the delay; merge of master forthcoming. . . .

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:512
http://jenkins.qa.ubuntu.com/job/unity8-ci/2060/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2332
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2209/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/907
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/582
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/584
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/584/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/582
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2045
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2334
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2334/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2209
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2209/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4666/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3089

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2060/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Allan LeSage (allanlesage) wrote :

The new tests are failing; while running the new tests on my mako I'm seeing these failures as authentic: there's a new "shuffling" behavior which may be confusing the simple "does the header string match" test. Nick would you run these tests and give your opinion?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:512
http://jenkins.qa.ubuntu.com/job/unity8-ci/2065/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2353
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2226/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/915
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/587
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/589
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/589/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/587
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2065
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2355
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2355/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2226
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2226/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4683/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3109

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2065/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Allan LeSage (allanlesage) wrote :

Correction: an objectName for the IndicatorPage on which I was depending has been eliminated, need to discuss with dednick about how to get hold of that page again, marking this as "in progress" for the moment.

Revision history for this message
Allan LeSage (allanlesage) wrote :

Dusting off this ancient MP, some minor changes from its prior incarnation. Recall that this was blocked on getting 'indicator stubs' through, now we'd like to land to enable some indicator-network autopilot tests for Wellark.

Revision history for this message
Leo Arias (elopio) wrote :

9 +# Copyright (C) 2013 Canonical
32 +# Copyright (C) 2013 Canonical
94 +# Copyright (C) 2013 Canonical

This now got to 2014, so please add the new year.

57 + # TODO: submit to autopilot.introspection.types.Rectangle [alesage 2013-12-06]

Can you please report it as a bug in autopilot and put the link here on this comment?
That way we will know when we can remove the TODO.

65 + def swipe_to_open_indicator(self, window):
76 + def swipe_to_close_indicator(self, window):

These two are good places to put the autopilot logging decorator.
I'm not sure if you have used it before. If not, take a look at dash.py or ping me.

Also, as the class name is DefaultIndicatorWidget, maybe we can make the names shorter:
swipe_to_open
swipe_to_close

I leave that decision to you, if you think it's less clear, just leave them as they are now.

74 + # TODO: assert that the indicator page opened [alesage 2013-12-06]
85 + # TODO: assert that the indicator page closed [alesage 2013-12-06]

I think this is pretty important for the stability of the test. Are we missing something to be able to add a wait_for here?

143 + if platform.model() == "Desktop":
144 + self.skipTest("Test cannot be run on the desktop.")

Not a big deal, but after some discussion QAs seem to like more the single quotes, so I'm trying to convince people of this everywhere. Please change the double quotes for single.

206 + unity_proxy = self.launch_unity()
207 + unlock_unity(unity_proxy)

I would put these two statements on the setUp.

208 + widget = self.main_window.get_indicator_widget(self.indicator_name)
209 + widget.swipe_to_open_indicator(self.main_window)
210 + indicator_page = self.main_window.get_indicator_page(
211 + indicator_title=self.title
212 + )
213 + self.assertTrue(indicator_page.visible)

We could make this more page-object compliant changing the signatures of the methods.
get_indicator_widget and get_indicator_page are returning autopilot objects. That's the main clue that tells you they should be private and never accessed by a test.
swipe_to_open_indicator doesn't need you to pass the main_window as a parameter, because you can select it again doing something like self.get_root_instance().select_single(MainWindow)

So I would try to rewrite the test to something like:

indicator_page = self.main_window.open_indicator(self.indicator_name)
self.assertTrue(indicator_page.visible)
self.assertEqual(indicator_page.title, self.title)

That's of course pseudocode and I'm not sure if it's doable at all. Let me know if you need a hand and I'll dig into the vis, or we can pair program some time.

ps: Feel free to disagree with me on any of the things I've written. If you think that the current way it's better for some reason, I'm always glad to discuss.

review: Needs Fixing (code review)
518. By Allan LeSage

Remove emulator, preferring main_window opening of indicator.

Revision history for this message
Allan LeSage (allanlesage) wrote :

The need for a get_center has been served in the interim, thanks veebers for directing me to. I've adapted elopio's suggested style changes, eliminating the widget emulator--really the 'swipe' action belongs to an object that spans a few UI elements, e.g. the Tab, Widget, Page, and I'm not sure this fits the autopilot regime. But it's ok to just have that 'indicator' in the main_view for the moment IMO.

I'm blocked on getting the feedback for 'is the page open', will ask dednick about in the AM.

519. By Allan LeSage

Restore platform import (cursing pyflakes), sphinxify a return in main_window emulator.

520. By Allan LeSage

Autopilot wait for indicator to be fullyOpened.

Revision history for this message
Allan LeSage (allanlesage) wrote :

Ok the property was suitably named 'fullyOpened', elopio if you could offer a final review please.

521. By Allan LeSage

Year amendment for indicators autopilot tests.

522. By Allan LeSage

Removing an unused/tested 'close_indicator_page' function.

523. By Allan LeSage

Privatize get_indicator_widget and get_indicator_page.

524. By Allan LeSage

Adjust input import.

Revision history for this message
Leo Arias (elopio) :
review: Approve (code review)
Revision history for this message
Allan LeSage (allanlesage) wrote :

 * Are there any related MPs required for this MP to build/function as expected? Please list.

No.

 * Did you perform an exploratory manual test run of your code change and any related functionality?

This concerns adding an autopilot test and not new functionality; I've tested on mako and using the desktop emulator.

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

Packaging has not been changed.

 * If you changed the UI, has there been a design review?

UI has not been changed.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:524
http://jenkins.qa.ubuntu.com/job/unity8-ci/2756/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-trusty-touch/100
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4621
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1626
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1277
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1281
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1281/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1277
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/97
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4214
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4214/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5755
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/3978
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4745
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4745/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2756/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Could we get a skip for Nexus10 here? It doesn't have bluetooth...

review: Needs Information
Revision history for this message
Michał Sawicz (saviq) wrote :

Basically something like what I did in https://code.launchpad.net/~saviq/unity8/skip-bluetooth-on-manta/+merge/211834 - but better? :)

525. By Allan LeSage

Merged trunk.

526. By Allan LeSage

Skip indicator-bluetooth on manta.

527. By Allan LeSage

Correct super invocation oops.

Revision history for this message
Allan LeSage (allanlesage) wrote :

Made that change, tested on manta, appears to work as expected :) .

Revision history for this message
Michał Sawicz (saviq) wrote :

+1

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes.

 * Did CI run pass? If not, please explain why.
Yup.

review: Approve
Revision history for this message
Michał Sawicz (saviq) wrote :

One thing to remember, this is locale-dependent, maybe we should export LANG=C somewhere?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/autopilot/unity8/indicators/tests/__init__.py'
2--- tests/autopilot/unity8/indicators/tests/__init__.py 2013-10-22 02:23:12 +0000
3+++ tests/autopilot/unity8/indicators/tests/__init__.py 2014-04-09 15:28:58 +0000
4@@ -0,0 +1,17 @@
5+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
6+#
7+# Unity - Indicators Autopilot Test Suite
8+# Copyright (C) 2013, 2014 Canonical
9+#
10+# This program is free software: you can redistribute it and/or modify
11+# it under the terms of the GNU General Public License as published by
12+# the Free Software Foundation, either version 3 of the License, or
13+# (at your option) any later version.
14+#
15+# This program is distributed in the hope that it will be useful,
16+# but WITHOUT ANY WARRANTY; without even the implied warranty of
17+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18+# GNU General Public License for more details.
19+#
20+# You should have received a copy of the GNU General Public License
21+# along with this program. If not, see <http://www.gnu.org/licenses/>.
22
23=== modified file 'tests/autopilot/unity8/indicators/tests/test_indicators.py'
24--- tests/autopilot/unity8/indicators/tests/test_indicators.py 2013-12-05 20:58:30 +0000
25+++ tests/autopilot/unity8/indicators/tests/test_indicators.py 2014-04-09 15:28:58 +0000
26@@ -1,7 +1,7 @@
27 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
28 #
29-# Unity - Indicators Autopilot Test Suite
30-# Copyright (C) 2013 Canonical
31+# Unity Indicators Autopilot Test Suite
32+# Copyright (C) 2013, 2014 Canonical
33 #
34 # This program is free software: you can redistribute it and/or modify
35 # it under the terms of the GNU General Public License as published by
36@@ -15,20 +15,32 @@
37 #
38 # You should have received a copy of the GNU General Public License
39 # along with this program. If not, see <http://www.gnu.org/licenses/>.
40-#
41
42 from __future__ import absolute_import
43
44+from testscenarios import multiply_scenarios
45+
46 from autopilot import platform
47-from testtools import skipIf
48
49 from unity8.process_helpers import unlock_unity
50-from unity8.shell.tests import UnityTestCase
51+from unity8.shell.tests import UnityTestCase, _get_device_emulation_scenarios
52
53
54 class IndicatorTestCase(UnityTestCase):
55
56- scenarios = [
57+ device_emulation_scenarios = _get_device_emulation_scenarios()
58+
59+ def setUp(self):
60+ if platform.model() == 'Desktop':
61+ self.skipTest('Test cannot be run on the desktop.')
62+ super(IndicatorTestCase, self).setUp()
63+ self.unity_proxy = self.launch_unity()
64+ unlock_unity(self.unity_proxy)
65+
66+
67+class IndicatorExistsTestCase(IndicatorTestCase):
68+
69+ indicator_scenarios = [
70 ('Bluetooth', dict(indicator_name='indicator-bluetooth')),
71 ('Datetime', dict(indicator_name='indicator-datetime')),
72 ('Location', dict(indicator_name='indicator-location')),
73@@ -37,14 +49,58 @@
74 ('Power', dict(indicator_name='indicator-power')),
75 ('Sound', dict(indicator_name='indicator-sound')),
76 ]
77+ scenarios = multiply_scenarios(
78+ indicator_scenarios,
79+ IndicatorTestCase.device_emulation_scenarios
80+ )
81
82- @skipIf(platform.image_codename() == 'Desktop',
83- "Unity8/phablet-only test!")
84 def setUp(self):
85- super(IndicatorTestCase, self).setUp()
86+ super(IndicatorExistsTestCase, self).setUp()
87+ if (platform.model() == 'Nexus 10' and
88+ self.indicator_name == 'indicator-bluetooth'):
89+ self.skipTest('Nexus 10 does not have bluetooth at the moment.')
90
91 def test_indicator_exists(self):
92- unity_proxy = self.launch_unity()
93- unlock_unity(unity_proxy)
94- indicator = self.main_window.get_indicator(self.indicator_name)
95- self.assertIsNotNone(indicator)
96+ self.main_window._get_indicator_widget(
97+ self.indicator_name
98+ )
99+
100+
101+class IndicatorPageTitleMatchesWidgetTestCase(IndicatorTestCase):
102+
103+ indicator_scenarios = [
104+ ('Bluetooth', dict(indicator_name='indicator-bluetooth',
105+ title='Bluetooth')),
106+ ('Datetime', dict(indicator_name='indicator-datetime',
107+ title='Upcoming')),
108+ ('Location', dict(indicator_name='indicator-location',
109+ title='Location')),
110+ ('Messaging', dict(indicator_name='indicator-messages',
111+ title='Incoming')),
112+ ('Network', dict(indicator_name='indicator-network',
113+ title='Network')),
114+ ('Power', dict(indicator_name='indicator-power',
115+ title='Battery')),
116+ ('Sound', dict(indicator_name='indicator-sound',
117+ title='Sound')),
118+ ]
119+ scenarios = multiply_scenarios(
120+ indicator_scenarios,
121+ IndicatorTestCase.device_emulation_scenarios
122+ )
123+
124+ def setUp(self):
125+ super(IndicatorPageTitleMatchesWidgetTestCase, self).setUp()
126+ if (platform.model() == 'Nexus 10' and
127+ self.indicator_name == 'indicator-bluetooth'):
128+ self.skipTest('Nexus 10 does not have bluetooth at the moment.')
129+
130+ def test_indicator_page_title_matches_widget(self):
131+ """Swiping open an indicator must show its correct title.
132+
133+ See https://bugs.launchpad.net/ubuntu-ux/+bug/1253804 .
134+ """
135+ indicator_page = self.main_window.open_indicator_page(
136+ self.indicator_name)
137+ self.assertTrue(indicator_page.visible)
138+ self.assertEqual(indicator_page.title, self.title)
139
140=== modified file 'tests/autopilot/unity8/shell/emulators/main_window.py'
141--- tests/autopilot/unity8/shell/emulators/main_window.py 2014-03-25 11:34:34 +0000
142+++ tests/autopilot/unity8/shell/emulators/main_window.py 2014-04-09 15:28:58 +0000
143@@ -20,6 +20,7 @@
144 import logging
145
146 from autopilot import logging as autopilot_logging
147+from autopilot import input
148
149 from unity8.shell import emulators
150 from unity8.shell.emulators.greeter import Greeter
151@@ -87,12 +88,32 @@
152 def get_pinentryField(self):
153 return self.select_single(objectName="pinentryField")
154
155- def get_indicator(self, indicator_name):
156- return self.wait_select_single(
157+ def _get_indicator_widget(self, indicator_name):
158+ return self.select_single(
159 'DefaultIndicatorWidget',
160 objectName=indicator_name+'-widget'
161 )
162
163+ def _get_indicator_page(self, indicator_name):
164+ return self.select_single(
165+ 'DefaultIndicatorPage',
166+ objectName=indicator_name+'-page'
167+ )
168+
169+ @autopilot_logging.log_action(logger.info)
170+ def open_indicator_page(self, indicator_name):
171+ """Swipe to open the indicator, wait until it's open.
172+
173+ :returns: The indicator page.
174+ """
175+ widget = self._get_indicator_widget(indicator_name)
176+ start_x, start_y = input.get_center_point(widget)
177+ end_x = start_x
178+ end_y = self.height
179+ self.pointing_device.drag(start_x, start_y, end_x, end_y)
180+ self.wait_select_single('Indicators', fullyOpened=True)
181+ return self._get_indicator_page(indicator_name)
182+
183 def get_shell_background(self):
184 return self.select_single(
185 "CrossFadeImage", objectName="backgroundImage")

Subscribers

People subscribed via source and target branches