Changeset View
Standalone View
neon/tests/plasma/plasma_favorite.pm
| Show All 28 Lines | 24 | sub run { | |||
|---|---|---|---|---|---|
| 29 | wait_still_screen; | 29 | wait_still_screen; | ||
| 30 | # Switches to the Application Tab | 30 | # Switches to the Application Tab | ||
| 31 | assert_and_click 'kickoff-application', 30; | 31 | assert_and_click 'kickoff-application', 30; | ||
| 32 | assert_and_click 'kickoff-office'; | 32 | assert_and_click 'kickoff-office'; | ||
| 33 | # Adds Okular in the favorites tab | 33 | # Adds Okular in the favorites tab | ||
| 34 | assert_and_click 'kickoff-okular', "right", 20; | 34 | assert_and_click 'kickoff-okular', "right", 20; | ||
| 35 | assert_and_click 'kickoff-add-to-favorite'; | 35 | assert_and_click 'kickoff-add-to-favorite'; | ||
| 36 | assert_screen 'kickoff-favorite', 30; | 36 | assert_screen 'kickoff-favorite', 30; | ||
| 37 | # Removes Okular from the favorites tab | ||||
| 38 | assert_and_click 'kickoff-remove-from-favorite', "right", 20; | ||||
bshah: Ideally, you can just remove `kickoff-remove-from-favorite` needle and just use the `kickoff… | |||||
Edit, for 3rd point I meant Rename kickoff-remove needle with kickoff-remove-from-favorite, sorry for confusion. bshah: Edit, for 3rd point I meant Rename `kickoff-remove` needle with `kickoff-remove-from-favorite`… | |||||
So, I only just noticed this, but, in the entire test you can basically replace "right" with 'right' (single quotes when you don't need to interpolate a string). Additionally you can drop all timeout arguments that are 20 or 30. The default is 30, so unless you need substantial divergence from that (e.g. when something is very IO bound) you don't need to define a timeout out all. sitter: So, I only just noticed this, but, in the entire test you can basically replace `"right"` with… | |||||
| 39 | assert_and_click 'kickoff-remove'; | ||||
| 40 | assert_screen ['kickoff-favorite', 'kickoff-favorite-removed'], 30*2; | ||||
30*2 ➡ 60? ;) The rule I've been following in the code is: if it is less than a minute use the actual seconds, if it is above a minute multiply 60. sitter: `30*2` ➡ `60`? ;)
The rule I've been following in the code is: if it is less than a minute use… | |||||
Should kickoff-favorite-removed maybe be called kickoff-favorite and represent the default appearance and the current kickoff-favorite be renamed to kickoff-favorite-okular to represent the kickoff state during the test? You could then also assert kickoff-favorite at the very beginning to assert that the defaults actually look like that even before we change them by adding okular. sitter: Should `kickoff-favorite-removed` maybe be called `kickoff-favorite` and represent the default… | |||||
| 41 | if (match_has_tag('kickoff-favorite')) { | ||||
| 42 | die 'Okular should not be visible on the favorite tab' | ||||
| 43 | } | ||||
| 37 | } | 44 | } | ||
| 38 | 45 | | |||
| 39 | sub test_flags { | 46 | sub test_flags { | ||
| 40 | # without anything - rollback to 'lastgood' snapshot if failed | 47 | # without anything - rollback to 'lastgood' snapshot if failed | ||
| 41 | # 'fatal' - whole test suite is in danger if this fails | 48 | # 'fatal' - whole test suite is in danger if this fails | ||
| 42 | # 'milestone' - after this test succeeds, update 'lastgood' | 49 | # 'milestone' - after this test succeeds, update 'lastgood' | ||
| 43 | # 'important' - if this fails, set the overall state to 'fail' | 50 | # 'important' - if this fails, set the overall state to 'fail' | ||
| 44 | return { important => 1 }; | 51 | return { important => 1 }; | ||
| 45 | } | 52 | } | ||
| 46 | 53 | | |||
| 47 | 1; | 54 | 1; | ||
Ideally, you can just remove kickoff-remove-from-favorite needle and just use the kickoff-favorite needle here, also I'd say the name kickoff-remove for next needle is confusing...
I suggest to,
Otherwise code looks good to me.