Merge lp://qastaging/~indicator-applet-developers/unity8/indicator-power-autopilot-test into lp://qastaging/unity8

Proposed by Allan LeSage
Status: Work in progress
Proposed branch: lp://qastaging/~indicator-applet-developers/unity8/indicator-power-autopilot-test
Merge into: lp://qastaging/unity8
Diff against target: 302 lines (+196/-9)
5 files modified
debian/control (+2/-0)
tests/autopilot/unity8/indicators/__init__.py (+20/-0)
tests/autopilot/unity8/indicators/tests/__init__.py (+20/-5)
tests/autopilot/unity8/indicators/tests/test_indicator_power.py (+145/-0)
tests/autopilot/unity8/indicators/tests/test_indicators.py (+9/-4)
To merge this branch: bzr merge lp://qastaging/~indicator-applet-developers/unity8/indicator-power-autopilot-test
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) Needs Fixing
PS Jenkins bot (community) continuous-integration Needs Fixing
Leo Arias (community) Approve
Thomi Richards (community) Approve
Christopher Lee (community) Needs Fixing
Review via email: mp+247079@code.qastaging.launchpad.net

Commit message

Add discharging battery autopilot test.

Description of the change

This test simulates a discharging battery using a fake upower via python-dbusmock.

Offering for commentary with the UEQA team, charles wanting to absorb best practices for writing these autopilot tests.

Note that this requires a couple of related branches:

For a upower template tweak to enable signalling,
https://code.launchpad.net/~charlesk/python-dbusmock/add-upower-device-set-properties/+merge/246713

For an indicator-power which will listen on a special private bus for the fake upower:
https://code.launchpad.net/~charlesk/indicator-power/custom-bus-for-upower/+merge/246234

NOTE that we're chasing an intermittent failure, may need some pitti advice about, hence WIP.

To post a comment you must log in.
Revision history for this message
Charles Kerr (charlesk) :
Revision history for this message
Christopher Lee (veebers) wrote :

Looking good so far. Some questions and things that need fixing up.

review: Needs Fixing
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) :
review: Needs Fixing
Revision history for this message
Leo Arias (elopio) wrote :

I added some more comments.

review: Needs Fixing
Revision history for this message
Charles Kerr (charlesk) wrote :

I've nibbled around the edges of these NFs and have fixed most of the little pieces, but it's past my bedtime so Allan or I will need to finish the revisions tomorrow.

Remaining TODO:

 * device_emulation_scenarios + multiply_scenarios()

 * FakeUPower as a Fixture

 * moving the Indicator helper class to tests/autopilot/unity8/indicators/helpers/indicator_power.py -- not much payoff for this today, but scales if/when we add more indicator helpers

 * GRID_UNIT_PX: is it needed? if so, why?

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

Filed lp:1413390 for the GRID_UNIT_PX issue, will tag in source, also here's a relevant IRC snippet with elopio: http://pastebin.ubuntu.com/9808551/ . Don't think we need to solve here.

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

Corrected _get_device_emulation_scenarios() resolved GRID_UNIT_PX issue.

I see elopio's and veebers' questions about using a CPO for Indicator object--this isn't strictly CPO-able as it represents not the appearance of the Indicator but its D-Bus state. Please bring questions on this, resolved?

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

Some comments inline.

And one item for discussion:

<elopio> veebers: thomi: alesage: why are we putting the custom proxy object in a module named helpers, intead of putting them in the module named indicators?
<veebers> alesage, elopio: sounds good to me. I would imagine the apps have a similar layout for precedent?
<elopio> veebers: it's a mess with apps. But that's what I am aiming for. We will have the browser CPO in webbrowser.WebbrowserApp
<elopio> instead of webbrowser.helpers.WebbrowserApp
<veebers> ack, for the CPO no need to have it in helpers
<alesage> sounds decided, I'll make that change veebers, elopio

Thank you! I specially like the idea of starting the service with a different dbus address.

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

Question in comments. . . .

Revision history for this message
Charles Kerr (charlesk) wrote :

Responses to Leo inline

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

Much better, but a few issues still.

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

I replied to some replies. Will look again at the new version.

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

LGTM

review: Approve
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 :

Charles, Allan, you good with this?

1569. By Allan LeSage

Add python3-dbusmock dependency.

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

Please remove the unused

56 + def icon_matches(self, icon_name):

These must be reverted to work with python2. We are currently installing unity8-autopilot also on the py2 path:

78 + super().setUp()
149 + super().setUp()
199 + super().setUp()

1570. By Allan LeSage

Move PowerIndicator, flake8 correction, restore py2 supers, broken test fix.

1571. By Allan LeSage

Remove icon_matches.

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

Looks good. Thanks for the changes.

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)
1572. By Allan LeSage

Correct python-dbusmock build-deps.

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

Relocate python*-dbusmock dependencies.

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 :

Text conflict in tests/autopilot/unity8/indicators/__init__.py
Text conflict in tests/autopilot/unity8/indicators/tests/__init__.py
Text conflict in tests/autopilot/unity8/indicators/tests/test_indicators.py
3 conflicts encountered.

1574. By Allan LeSage

Merge trunk, resolving conflicts.

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

Hi guys, can you please add the "MP Submission Checklist Template" from https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8 as part of the "description Of the Change"?

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

This might sit for a bit, need to investigate a persistent Jenkins failure, will ping you Albert.

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

I'll put it as Work In Progress to remove it from my dashboard of things to review.

Unmerged revisions

1574. By Allan LeSage

Merge trunk, resolving conflicts.

1573. By Allan LeSage

Relocate python*-dbusmock dependencies.

1572. By Allan LeSage

Correct python-dbusmock build-deps.

1571. By Allan LeSage

Remove icon_matches.

1570. By Allan LeSage

Move PowerIndicator, flake8 correction, restore py2 supers, broken test fix.

1569. By Allan LeSage

Add python3-dbusmock dependency.

1568. By Charles Kerr

in __init__, make multiline import follow pep 328

1567. By Charles Kerr

in IndicatorPowerTestCase.setUp(), remove the self.main_window.wait_select_single(), since it's also encapsulated in the Indicator helper class

1566. By Charles Kerr

in IndicatorPowerTestCase.setUp(), don't create a temporary list for service_test_args.

1565. By Charles Kerr

in MockUPower.setUp(), clean up the OSError exception's message

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches