Unity 8

Merge lp:~aacid/unity8/fix_indicators_update_state into lp:unity8

  1. fix_indicators_update_state
  2. Merge into trunk
Proposed by Albert Astals Cid on 2015-05-27
Status: Merged
Approved by: Nick Dedekind on 2015-07-03
Approved revision: 1797
Merged at revision: 1871
Proposed branch: lp:~aacid/unity8/fix_indicators_update_state
Merge into: lp:unity8
Prerequisite: lp:~aacid/unity8/oxide_regression_workaround
Diff against target: 46 lines (+8/-1)
3 files modified
qml/Panel/IndicatorsMenu.qml (+1/-1)
tests/autopilot/unity8/indicators/__init__.py (+1/-0)
tests/qmltests/Panel/tst_IndicatorsMenu.qml (+6/-0)
To merge this branch: bzr merge lp:~aacid/unity8/fix_indicators_update_state
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration 2015-05-27 Needs Fixing on 2015-07-15
Nick Dedekind (community) 2015-06-19 Approve on 2015-07-03
Unity Team 2015-05-27 Pending
Review via email: mp+260291@code.launchpad.net

This proposal supersedes a proposal from 2015-05-26.

Commit Message

Update state of indicators bar only when the user is actively dragging himself

Fixes race with timer causing bug 1457044

Description of the Change

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

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

 * Did you make sure that your branch does not contain spurious tags?
Yes

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

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

 * Did you have a look at the warnings when running tests? Can they be reduced?
Yes, not introducing any new warning

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1793
http://jenkins.qa.ubuntu.com/job/unity8-ci/5765/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/44
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-wily/28/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/34
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/44
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/44/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20734

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

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1794
http://jenkins.qa.ubuntu.com/job/unity8-ci/5766/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/45
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-wily/29/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/35
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/45
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/45/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20740

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

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1795
http://jenkins.qa.ubuntu.com/job/unity8-ci/5775/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/54
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-wily/38
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/53
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/53
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/42
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/54
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/54/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20767

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

review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/fix_indicators_update_state updated on 2015-06-22
1796. By Albert Astals Cid on 2015-06-22

Merge unity8

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1796
http://jenkins.qa.ubuntu.com/job/unity8-ci/5844/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/139/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/124
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/125
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/91/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/139
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/139/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21350

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

review: Needs Fixing (continuous-integration)
Nick Dedekind (nick-dedekind) wrote :

The change to the tests doesn't test the issue. If succeeds whether the change in IndicatorsMenu exists or not. You can test by updating unitProgress without having an active bar.

indicatorsMenu.unitProgress = 0;
compare(indicatorsMenu.state, "initial");
indicatorsMenu.unitProgress = 1
compare(indicatorsMenu.state, "initial");

Nick Dedekind (nick-dedekind) :
review: Needs Fixing
Nick Dedekind (nick-dedekind) wrote :

although unitProgess is a readonly.
can use:
indicatorsMenu.height = indicatorsMenu.minimizedPanelHeight
indicatorsMenu.height = indicatorsMenu.openedHeight

lp:~aacid/unity8/fix_indicators_update_state updated on 2015-07-03
1797. By Albert Astals Cid on 2015-07-03

Merge unit8

Albert Astals Cid (aacid) wrote :

> The change to the tests doesn't test the issue. If succeeds whether the change
> in IndicatorsMenu exists or not. You can test by updating unitProgress without
> having an active bar.

You are right, the change in the test is not to test the change, is to make it so that the test still works, did not feel the need to add a qmltest to check this is working since the autopilot test was already failing sometimes without this so we actually already have something that makes sure this is needed.

Nick Dedekind (nick-dedekind) wrote :

> > The change to the tests doesn't test the issue. If succeeds whether the
> change
> > in IndicatorsMenu exists or not. You can test by updating unitProgress
> without
> > having an active bar.
>
> You are right, the change in the test is not to test the change, is to make it
> so that the test still works, did not feel the need to add a qmltest to check
> this is working since the autopilot test was already failing sometimes without
> this so we actually already have something that makes sure this is needed.

ok, fine.

review: Approve
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1797
http://jenkins.qa.ubuntu.com/job/unity8-ci/5933/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/255
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/213
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/214
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/175
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/255
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/255/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21644

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

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1797
http://jenkins.qa.ubuntu.com/job/unity8-ci/6036/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/338
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-wily/66
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/274
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/275
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/243
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/338
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/338/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21856

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

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Panel/IndicatorsMenu.qml'
2--- qml/Panel/IndicatorsMenu.qml 2015-05-18 23:04:12 +0000
3+++ qml/Panel/IndicatorsMenu.qml 2015-07-03 10:45:30 +0000
4@@ -270,7 +270,7 @@
5 }
6
7 function updateState() {
8- if (!showAnimation.running && !hideAnimation.running) {
9+ if (!showAnimation.running && !hideAnimation.running && d.activeDragHandle) {
10 if (unitProgress <= 0) {
11 root.state = "initial";
12 // lock indicator if we've been committed and aren't moving too much laterally or too fast up.
13
14=== modified file 'tests/autopilot/unity8/indicators/__init__.py'
15--- tests/autopilot/unity8/indicators/__init__.py 2015-04-28 15:20:13 +0000
16+++ tests/autopilot/unity8/indicators/__init__.py 2015-07-03 10:45:30 +0000
17@@ -74,6 +74,7 @@
18 # XXX this should be implemented as a general horizontal swiping in
19 # the toolkit custom proxy object. -- elopio - 2015-01-20
20 if not flickable.atXEnd:
21+ flickable.interactive.wait_for(True)
22 while not flickable.atXEnd:
23 start_y = stop_y = (
24 flickable.globalRect.y +
25
26=== modified file 'tests/qmltests/Panel/tst_IndicatorsMenu.qml'
27--- tests/qmltests/Panel/tst_IndicatorsMenu.qml 2015-03-06 04:44:11 +0000
28+++ tests/qmltests/Panel/tst_IndicatorsMenu.qml 2015-07-03 10:45:30 +0000
29@@ -145,11 +145,17 @@
30 }
31
32 function test_progress_changes_state_to_reveal() {
33+ var firstItem = get_indicator_item(0);
34+ var firstItemMappedPosition = indicatorsMenu.mapFromItem(firstItem, firstItem.width/2, firstItem.height/2);
35+ touchPress(indicatorsMenu, firstItemMappedPosition.x, indicatorsMenu.minimizedPanelHeight / 2);
36+
37 indicatorsMenu.height = indicatorsMenu.openedHeight / 2;
38 compare(indicatorsMenu.state, "reveal", "Indicators should be revealing when partially opened.");
39
40 indicatorsMenu.height = indicatorsMenu.openedHeight;
41 compare(indicatorsMenu.state, "reveal", "Indicators should still be revealing when fully opened.");
42+
43+ touchRelease(indicatorsMenu, firstItemMappedPosition.x, indicatorsMenu.minimizedPanelHeight / 2);
44 }
45
46 function test_open_state() {

Subscribers

People subscribed via source and target branches