Verify if favorite and alternative changes persist across session restart.
ClosedPublic

Authored by bdhruve on Jun 15 2018, 12:54 PM.

Details

Reviewers
bshah
sitter
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
R1005:035fa0ecccf0: Verify if favorite and alternative changes persist across session restart.
Summary
  • Add code for logging out and getting back in the session
  • Check if the changes done works after getting back in session

Diff Detail

Repository
R1005 Neon OpenQA Components
Branch
session-persist
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 1008
Build 1021: arc lint + arc unit
bdhruve created this revision.Jun 15 2018, 12:54 PM
Owners added a reviewer: Restricted Owners Package.Jun 15 2018, 12:54 PM
bdhruve requested review of this revision.Jun 15 2018, 12:54 PM

Does this really need a test? Did this ever break?

It appears to me this is essentially doing a test of kconfig/plasmashell's use of kconfig, which should be much more efficiently tested via a unit test (and likely already is)

neon/tests/plasma/plasma_alternatives.pm
74

might be worth thinking about a helper logout function in basetest seeing as we do this in two tests now

bdhruve updated this revision to Diff 37185.Jul 5 2018, 12:02 PM

Add logout function in basetest

Does this really need a test? Did this ever break?

It appears to me this is essentially doing a test of kconfig/plasmashell's use of kconfig, which should be much more efficiently tested via a unit test (and likely already is)

I am not sure if there are any existing autotests present for testing that favorites and alternative switch persist through session restart, and that is why thought of adding openqa test for it.

Yeah, I am fairly certain there isn't much that can go wrong which isn't already covered in the relevant unit tests to be honest. Ultimately the switch is a simple config mutation which is more efficiently tested as a unit test I expect.

All that said, I am not objecting, but I am also not convinced this adds much for the cost of having to maintain that additional logout needle.

@bshah what's your thought on this?

bshah added a comment.Jul 5 2018, 1:12 PM

Yeah, I am fairly certain there isn't much that can go wrong which isn't already covered in the relevant unit tests to be honest. Ultimately the switch is a simple config mutation which is more efficiently tested as a unit test I expect.

All that said, I am not objecting, but I am also not convinced this adds much for the cost of having to maintain that additional logout needle.

@bshah what's your thought on this?

About logout needle, I think this code can be useful for future tests as well (not sure about exact usecase but logout is pretty generic workflow thing I am sure)

About test itself, I think I've heard multiple bugs of favorites getting resetted, quick search in inbox gives at least two results, didn't search in bugzilla yet..., (although different then what is being tested)

But in general I'd say favorite and alternatives part are most error-prone and I think they deserve a QA. So at least from my side +1.

bshah added inline comments.Jul 5 2018, 2:38 PM
neon/lib/basetest_neon.pm
78–87

New idea, the idea is from folder-desktop, you can start krunner and type "Logout" and press enter, avoiding alll of the new needle kicker-logout, making this much more compact. You can as well use testapi::x11_start_program('Logout') to trigger the Logout dialog and then assert_and_click ksmserver-logout

bdhruve updated this revision to Diff 37873.Jul 16 2018, 12:26 PM

Add Logout function using krunner in basetest

bshah requested changes to this revision.Jul 17 2018, 7:55 AM

Can you please rebase your changes on latest master? plasma_alternatives.pm has been changed in master branch.

This revision now requires changes to proceed.Jul 17 2018, 7:55 AM

This looks really good now though.

neon/lib/basetest_neon.pm
80

I am wondering if we can't drop this. wait_still_screen is essentially a sleep nowadays and only used in legacy code more or less, as such it mostly has no useful application.

for this specifically, the only needle the caller can feasibly assert after a logout is sddm, and in between logout and sddm there are no "confusing" screen states that could match sddm, so the wait only delays the test while an assert_screen 'sddm' would do the same albeit more dynamically.

So... I would kill that line entirely.

bdhruve updated this revision to Diff 38078.Jul 19 2018, 10:00 AM

Rebase to master branch and remove wait_still_screen.

sitter requested changes to this revision.Jul 19 2018, 10:07 AM

Ah sorry, I missed something: you still need to delete the kicker-logout needle, seeing as we do not need it anymore.

This revision now requires changes to proceed.Jul 19 2018, 10:07 AM
bdhruve updated this revision to Diff 38079.Jul 19 2018, 10:24 AM

Removed kicker-logout needle.

sitter accepted this revision.Jul 19 2018, 10:48 AM
sitter accepted this revision.
This revision was not accepted when it landed; it landed in state Needs Review.Jul 19 2018, 11:01 AM
This revision was automatically updated to reflect the committed changes.