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

[Albert Astals Cid]

Update current session after changing the user

This matches unity7 greeter behaviour, i.e. if you change the session and then change the user, the session that shows up is the last session that new user logged in and not the new session you selected in the previous user

 * 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

 * 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

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2746
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2740/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3596
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2054
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2054
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3624
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3470
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3470/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3470
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3470/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3470
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3470/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3470
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3470/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3470
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3470/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3470
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3470/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2740/rebuild

review: Needs Fixing (continuous-integration)

[Josh Arenson (josharenson)]

Since my test branch relies on this change as well as mterry's branch, would you mind if I just included this change in my test branch lp:~josharenson/unity8/ported-session-test instead since this change is so small?

[Albert Astals Cid (aacid)]

I don't mind if it gets in as part of another branch, what i don't want it is to get not merged in because those branches are heavy and change lots of stuff and hard to review/approve.

[Josh Arenson (josharenson)]

While not caused by your branch, the following issue is exacerbated by it.

QWARN : qmltestrunner::WideView::test_loginListMovement(up) file:///home/josh/Documents/unity8/greeter_update_current_session_on_user_change/qml/Greeter/WideView.qml:145: Error: Cannot assign [undefined] to QString

Since the issue is, most likely, my fault, I can include a fix in one of my upcoming greeter branches if you would like. It seems to only affect the mock, so pretty low priority.

review: Needs Information

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2747
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2834/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3711
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2134
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2134
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3739
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3583
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3583
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3583
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3583
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3583
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3583
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3583/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2834/rebuild

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

> While not caused by your branch, the following issue is exacerbated by it.
>
> QWARN : qmltestrunner::WideView::test_loginListMovement(up) file:///home/josh
> /Documents/unity8/greeter_update_current_session_on_user_change/qml/Greeter/Wi
> deView.qml:145: Error: Cannot assign [undefined] to QString
>
>
> Since the issue is, most likely, my fault, I can include a fix in one of my
> upcoming greeter branches if you would like. It seems to only affect the mock,
> so pretty low priority.

Last commit should fix that, can you have a look?

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2748
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2862/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3740
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2158
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2158
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3768
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3612
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3612/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3612
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3612/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3612
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3612/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3612
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3612/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3612
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3612/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3612
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3612/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2862/rebuild

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:2748
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2886/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3769
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2181
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2181
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3797
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3641/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2886/rebuild

review: Approve (continuous-integration)

[Josh Arenson (josharenson)]

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

 * Did CI run pass?
Yes

review: Approve

[Michał Sawicz (saviq)]

This seems to be the closest one, so reporting here:

While testing silo 2272, I noticed the first user's icon wasn't correct on boot - blank instead of the previously used session.

See https://drive.google.com/file/d/0B32jwBcbaPloVHhmcFhLd2RyQ1U/view for a video of the issue.

Also related, the following test failed in britney:

FAIL! : qmltestrunner::WideView::test_selected() Compared values are not the same
   Actual (): 3
   Expected (): 2
   Loc: [/usr/share/unity8/tests/qmltests/Greeter/tst_WideView.qml(503)]

review: Needs Fixing

[Michael Terry (mterry)]

I fixed the blank icon in my greeter-prompt-model branch. Looking at the test failure... (I don't see it in my greeter-prompt-model branch alone, and it sits atop this branch -- so must be an integration issue with other branches)

[Michał Sawicz (saviq)]

Thanks, re-approving, then.

review: Abstain

=== modified file 'qml/Greeter/LoginList.qml'
--- qml/Greeter/LoginList.qml	2016-11-29 00:13:45 +0000
+++ qml/Greeter/LoginList.qml	2017-01-10 09:31:50 +0000
@@ -36,7 +36,6 @@
     readonly property int cellHeight: units.gu(5)
     readonly property int highlightedHeight: units.gu(15)
     readonly property int moveDuration: UbuntuAnimation.FastDuration
-    property string selectedSession
     property string currentSession
     readonly property string currentUser: userList.currentItem.username
     property bool wasPrompted: false
@@ -109,11 +108,15 @@
     }
 
     Keys.onUpPressed: {
-        selected(currentIndex - 1);
+        if (currentIndex > 0) {
+            selected(currentIndex - 1);
+        }
         event.accepted = true;
     }
     Keys.onDownPressed: {
-        selected(currentIndex + 1);
+        if (currentIndex + 1 < model.count) {
+            selected(currentIndex + 1);
+        }
         event.accepted = true;
     }
     Keys.onEscapePressed: {

=== modified file 'qml/Greeter/WideView.qml'
--- qml/Greeter/WideView.qml	2016-11-29 00:13:45 +0000
+++ qml/Greeter/WideView.qml	2017-01-10 09:31:50 +0000
@@ -125,8 +125,6 @@
             id: loginList
             objectName: "loginList"
 
-            property int selectedUserIndex: 0
-
             width: units.gu(40)
             anchors {
                 left: parent.left
@@ -141,11 +139,10 @@
             Behavior on boxVerticalOffset { UbuntuNumberAnimation {} }
 
             model: root.userModel
-            currentSession: LightDMService.users.data(selectedUserIndex, LightDMService.userRoles.SessionRole);
             onResponded: root.responded(response)
             onSelected: {
                 root.selected(index)
-                loginList.selectedUserIndex = index;
+                currentSession = LightDMService.users.data(index, LightDMService.userRoles.SessionRole);
             }
             onSessionChooserButtonClicked: parent.state = "SessionsList"
 

=== modified file 'tests/qmltests/Greeter/tst_WideView.qml'
--- tests/qmltests/Greeter/tst_WideView.qml	2016-08-30 14:06:47 +0000
+++ tests/qmltests/Greeter/tst_WideView.qml	2017-01-10 09:31:50 +0000
@@ -704,12 +704,15 @@
 
         function test_loginListMovement_data() {
             return [
-                {tag: "up", key: Qt.Key_Up, result: -1},
-                {tag: "down", key: Qt.Key_Down, result: 1},
+                {tag: "up", key: Qt.Key_Up, result: 1},
+                {tag: "down", key: Qt.Key_Down, result: 3},
             ]
         }
 
         function test_loginListMovement(data) {
+            selectIndex(2);
+            selectedSpy.clear();
+
             keyClick(data.key);
             compare(selectedSpy.count, 1);
             compare(selectedSpy.signalArguments[0][0], data.result);