1193978 - Automated Test [Clock] Use stopwatch, set a few laps, reset.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[John Dorlus [:Silne30]]

https://moztrap.mozilla.org/manage/cases/?filter-id=15803

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

Comment 2

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8647180 [details] [review]
[gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

The content of the tests looks good! I think we can improve the form a little bit more. See more details in the PR.

Comment 3

[Martijn Wargers (dead)]

Comment on attachment 8647180 [details] [review]
[gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

Looks good. I made some comments in the pull request. Not sure if they are all valid, but some of them might be useful.
You need to add the new test files into the manifest.ini file.

Comment 4

[Martijn Wargers (dead)]

I'm also wondering if we care if these tests would run correctly on b2g desktop, I haven't checked that. I guess we don't care anymore about that, since this is now hidden on Treeherder.

Comment 5

[Johan Lorenzo [:jlorenzo]]

Like in bug 1195305, the manifest hasn't been edited. These tests should belong in the dogfood suite.

Comment 6

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8647180 [details] [review]
[gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

A couple of minor nits. The major issues to me are: The manifest missing and the verify_pause() in the page class. This function doesn't perform any verification (it helps it, though), which can confuse future readers.

More details in the PR.

Comment 7

[Martijn Wargers (dead)]

Comment on attachment 8647180 [details] [review]
[gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

I'll review (and mainly test) once you updated the pull request following the suggestions from Johan.

Comment 8

[John Dorlus [:Silne30]]

The ad-hoc run that we did, passed completely. 

http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc/866/

I think we should be good to pull.

Comment 9

[Martijn Wargers (dead)]

Comment on attachment 8647180 [details] [review]
[gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

Sorry, perhaps I'm totally mistaken, but as I understand it, verify_laps_not_same is wrong, because it's used in a situation where the stopwatch times should actually be the same, because the pause button was pressed.
Also, some other comments in the pull request.

Comment 10

[Martijn Wargers (dead)]

Comment on attachment 8647180 [details] [review]
[gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

Some more comments in the pull request. test_clock_run_stopwatch_laps.py didn't pass for me on b2g desktop.
Regarding test_clock_run_stopwatch_then_reset.py, I think the check surrounding the pause button is not useful. It doesn't check that the button works properly.

Comment 11

[Johan Lorenzo [:jlorenzo]]

Side note: The adhoc job didn't start yesterday. Reason:
> There are no nodes with the label '' .

This means you need to provide a Jenkins slave node to execute the tests. Usually we use: "flame&&ui". This means you target a Flame device in the lab, that is attached to the UI jobs (and not a perf job for instance).

In order to get results when you awake. I started a new one http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/869/console

Comment 12

[John Dorlus [:Silne30]]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #11)
> Side note: The adhoc job didn't start yesterday. Reason:
> > There are no nodes with the label '' .
> 
> This means you need to provide a Jenkins slave node to execute the tests.
> Usually we use: "flame&&ui". This means you target a Flame device in the
> lab, that is attached to the UI jobs (and not a perf job for instance).
> 
> In order to get results when you awake. I started a new one
> http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/869/console

I knew that may happen. Firefox Nightly blew away all of the fields before it crashed. I will do it again.

Comment 13

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8647180 [details] [review]
[gaia] silne30:Bug_1193978_Stopwatch_Laps_Test > mozilla-b2g:master

Looks good to me, modulo:
* https://github.com/mozilla-b2g/gaia/pull/31336/files?diff=split#r37523027
* https://github.com/mozilla-b2g/gaia/pull/31336/files?diff=split#r37557477

Comment 14

[Johan Lorenzo [:jlorenzo]]

I just noticed this PR has never been merged. John, do you have the rights to merge this PR?

Comment 15

[John Dorlus [:Silne30]]

Merged this today. It had been so long that there were a ton of merge conflicts to fix. Done!

Comment 16

[Martijn Wargers (dead)]

Did you forget to squash the commits before merging?

Comment 17

[Kevin Grandon :kgrandon]

John - please be more careful in the future. Every commit landed to gaia should contain a bug number, and be atmoic in nature, typically via squashing pull requests. I've gone ahead and backed out the PR and re-landed as a single commit.

Revert: https://github.com/mozilla-b2g/gaia/commit/d8c2ac6cb386be689bca5f84beecc68030f0a32b
Re-land: https://github.com/mozilla-b2g/gaia/commit/8a1b3d7c120b17838757c92226daadccf473c327

Comment 18

[John Dorlus [:Silne30]]

Sorry about that. Noted.