Merge lp://qastaging/~josharenson/unity8/fix-default-session into lp://qastaging/unity8
- fix-default-session
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Michael Terry |
Approved revision: | 2763 |
Merged at revision: | 2917 |
Proposed branch: | lp://qastaging/~josharenson/unity8/fix-default-session |
Merge into: | lp://qastaging/unity8 |
Prerequisite: | lp://qastaging/~josharenson/unity8/polish-greeter |
Diff against target: |
75 lines (+33/-0) 4 files modified
plugins/LightDM/UsersModel.cpp (+5/-0) tests/mocks/liblightdm/MockUsersModel.cpp (+1/-0) tests/plugins/LightDM/IntegratedLightDM/usersmodel.cpp (+13/-0) tests/qmltests/Greeter/tst_WideView.qml (+14/-0) |
To merge this branch: | bzr merge lp://qastaging/~josharenson/unity8/fix-default-session |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot | continuous-integration | Approve | |
Michael Terry | Approve | ||
Review via email:
|
Commit message
Handle a user in the greeter having never selected a session better
Choose the default session instead of an empty string
Description of the change
* Are there any related MPs required for this MP to build/function as expected? Please list.
lp:~josharenson/unity8/polish-greeter
* 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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
Seems good to me, but might this not be better done at the Model level? Rather than have the View massage data? Much like we modify user name data if it's empty.
setCurrentSession() feels like it belongs in Greeter or a piece of code shared between Wide and Narrow views... But I think my greeter-arrangement branch does that (I should dust that off, fix its tests, and re-propose).
Tests didn't pass, for unrelated reason.
Nice test addition.
- 2762. By Josh Arenson
-
Move the fix to the model level
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Josh Arenson (josharenson) wrote : | # |
Moved the fix into our (unity8) model and updated the test. How did we not notice this earlier, and is it worth filing a bug against LightDM?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
Couple small nits:
> + if (Q_UNLIKELY(role == QLightDM:
Like the Q_UNLIKELY, forgot about that little macro. :) But isEmpty() is probably what you want there, for ease of reading (instead of checking length).
> + function test_sessionles
This is fine and useful. But now that this is in the model, a new test in tests/plugins/
> How did we not notice this earlier, and is it worth filing a bug against LightDM?
I guess because all our mocks have a default session already, and we don't test on fresh systems much (and when we did on the phone, we didn't expose the session).
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2762
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 2763. By Josh Arenson
-
Add backend test
Also, clean up string length logic
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Josh Arenson (josharenson) wrote : | # |
Kept the original test, because its a valid test, and tests are good, but also added a backend test Since LightDM should guarantee that it returns a valid session (or unknown), the new test should be valid.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2763
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
Love it. The failing test seems unrelated. Thanks for putting up with my nits!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2763
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2763
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2763
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2763
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2763
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2763
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2763
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2761 /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3466/ /unity8- jenkins. ubuntu. com/job/ build/4576 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= xenial+ overlay, testname= qmluitests. sh/2759 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= zesty,testname= qmluitests. sh/2759 /unity8- jenkins. ubuntu. com/job/ build-0- fetch/4604 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4431 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4431/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4431 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4431/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4431 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4431/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4431 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4431/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4431 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4431/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4431 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4431/ artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3466/ rebuild
https:/