751143 - Add restart tests and disable skipped tests in manifest files

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

With the introduction of manifest files, tests can now be skipped without modifying the test. This also means the test will not execute at all, and will save time when running the tests. This bug covers moving the skips from the tests to the manifest files as implemented in bug 751142.

Comment 1

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Skip tests in manifest files. v1.0

We won't be using manifest files with Mozmill 1.5.x so until we have migrated to 2.x we will need to keep the skips within the tests. This patch adds the skips to the manifest files.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 624020 [details] [diff] [review]
Skip tests in manifest files. v1.0

Quick glance over it shows me that it is fine. Please make sure to also disable all existing restart tests because they wont work with Mozmill 2 yet, right?

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The restart tests issue is bug 753190, which I have set as a blocker. I'd prefer not to disable them in the manifest if possible, and get the issue to be resolved instead.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Dave, can you try to get this done soon? Would be necessary for us to lower the amount of failures if we run our tests with Mozmill 2.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I have a local patch for this based on the current skips, but I think I was waiting for the restart tests to be migrating/working in Mozmill 2.0. I will update and attach the patch for feedback.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add restart tests and disable skipped tests in manifest files. v2.0

Added all remaining tests to manifest files, and skip as appropriate.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 638722 [details] [diff] [review]
Add restart tests and disable skipped tests in manifest files. v2.0

>+++ b/tests/functional/restartTests/testRestartChangeArchitecture/manifest.ini
>@@ -0,0 +1,5 @@
>+[test1.js]
>+[test2.js]
>+[test3.js]
>+[test4.js]
>+[test5.js]

Those tests are for mac only. Probably you can include it already in 'restartTests/manifest.ini'?

>+++ b/tests/l10n/manifest.ini
>@@ -1,2 +1,3 @@
>-[testAccessKeys]
>-[testCropped]
>+[testAccessKeys/test1.js]
>+[testCropped/test1.js]
>+disabled = Bug 614579 - Crop test sometimes shows single line for cropped elements

Please create separate manifests for both folders because those will get more tests.

>+++ b/tests/remote/manifest.ini
>@@ -0,0 +1,11 @@
>+[restartTests/testDiscoveryPane_FeaturedModule/test1.js]
>+disabled = Bug 732353 - Disable all Discovery Pane tests due to unpredictable web dependencies

Those entries should be in a manifest under restartTests. Similar to functional tests.

Otherwise looks good.

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Henrik Skupin (:whimboo) from comment #7)
> Comment on attachment 638722 [details] [diff] [review]
> Add restart tests and disable skipped tests in manifest files. v2.0
> 
> >+++ b/tests/functional/restartTests/testRestartChangeArchitecture/manifest.ini
> >@@ -0,0 +1,5 @@
> >+[test1.js]
> >+[test2.js]
> >+[test3.js]
> >+[test4.js]
> >+[test5.js]
> 
> Those tests are for mac only. Probably you can include it already in
> 'restartTests/manifest.ini'?

I can use "run-if = os == 'mac'" but there's no way afaict to limit to a specific version. I'm not sure if I can disable based on an include, if that's what you're suggesting?

> >+++ b/tests/l10n/manifest.ini
> >@@ -1,2 +1,3 @@
> >-[testAccessKeys]
> >-[testCropped]
> >+[testAccessKeys/test1.js]
> >+[testCropped/test1.js]
> >+disabled = Bug 614579 - Crop test sometimes shows single line for cropped elements
> 
> Please create separate manifests for both folders because those will get
> more tests.

Sure, I will do that.

> >+++ b/tests/remote/manifest.ini
> >@@ -0,0 +1,11 @@
> >+[restartTests/testDiscoveryPane_FeaturedModule/test1.js]
> >+disabled = Bug 732353 - Disable all Discovery Pane tests due to unpredictable web dependencies
> 
> Those entries should be in a manifest under restartTests. Similar to
> functional tests.

The only reason I didn't do that is that there are only restart tests, and there's only ever one test. I guessed this was due to the limitations of restart tests with Mozmill 1.5.x and a necessity for the restartTests folder. Please confirm if you would like me to add remote/restartTests/manifest.ini and include it in remote/manifest.ini. Also, please let me know if you would like manifests for each folder even though there is just one file (test1.js) in each. Same question for endurance tests.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Dave Hunt (:davehunt) [Away until July 3rd] from comment #8)
> (In reply to Henrik Skupin (:whimboo) from comment #7)
> > Comment on attachment 638722 [details] [diff] [review]
> > Add restart tests and disable skipped tests in manifest files. v2.0
> > 
> > >+++ b/tests/functional/restartTests/testRestartChangeArchitecture/manifest.ini
> > >@@ -0,0 +1,5 @@
> > >+[test1.js]
> > >+[test2.js]
> > >+[test3.js]
> > >+[test4.js]
> > >+[test5.js]
> > 
> > Those tests are for mac only. Probably you can include it already in
> > 'restartTests/manifest.ini'?
> 
> I can use "run-if = os == 'mac'" but there's no way afaict to limit to a
> specific version. I'm not sure if I can disable based on an include, if
> that's what you're suggesting?

Disabling based on include works fine, but I had to add the filtering to the new automation scripts: https://github.com/whimboo/mozmill-automation/pull/12

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Dave Hunt (:davehunt) [Away until July 3rd] from comment #8)
> > Those tests are for mac only. Probably you can include it already in
> > 'restartTests/manifest.ini'?
> 
> I can use "run-if = os == 'mac'" but there's no way afaict to limit to a
> specific version. I'm not sure if I can disable based on an include, if
> that's what you're suggesting?

There is no need for us to limit to a specific version. We do not support PPC builds anytime longer.

> The only reason I didn't do that is that there are only restart tests, and
> there's only ever one test. I guessed this was due to the limitations of
> restart tests with Mozmill 1.5.x and a necessity for the restartTests
> folder. Please confirm if you would like me to add
> remote/restartTests/manifest.ini and include it in remote/manifest.ini.

Yes, please do so. It's very likely that we will have other remote tests in the future which are not restart tests, e.g. tests which really have to access remote sites like tests for SSL certificates.

> Also, please let me know if you would like manifests for each folder even
> though there is just one file (test1.js) in each. Same question for
> endurance tests.

If that's not a big deal we should probably do that. I like consistency across the whole repository.

Comment 11

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add restart tests and disable skipped tests in manifest files. v2.1

(In reply to Henrik Skupin (:whimboo) from comment #10)
> (In reply to Dave Hunt (:davehunt) [Away until July 3rd] from comment #8)
> > The only reason I didn't do that is that there are only restart tests, and
> > there's only ever one test. I guessed this was due to the limitations of
> > restart tests with Mozmill 1.5.x and a necessity for the restartTests
> > folder. Please confirm if you would like me to add
> > remote/restartTests/manifest.ini and include it in remote/manifest.ini.
> 
> Yes, please do so. It's very likely that we will have other remote tests in
> the future which are not restart tests, e.g. tests which really have to
> access remote sites like tests for SSL certificates.

I have added this for consistency. In the future I don't expect restart tests to be in their own folder.

> > Also, please let me know if you would like manifests for each folder even
> > though there is just one file (test1.js) in each. Same question for
> > endurance tests.
> 
> If that's not a big deal we should probably do that. I like consistency
> across the whole repository.

I agree with keeping things consistent. I had one eye on the near future when we can move/rename files and not be restricted by the current limitations.

Only question remaining is where you would prefer tests to be disabled. This can either be when including the manifest that specifies the test files, or can be in the manifest that includes that manifest. See the latest patch, and let me know if you think I should do it differently.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Dave Hunt (:davehunt) from comment #11)
> > Yes, please do so. It's very likely that we will have other remote tests in
> > the future which are not restart tests, e.g. tests which really have to
> > access remote sites like tests for SSL certificates.
> 
> I have added this for consistency. In the future I don't expect restart
> tests to be in their own folder.

We will not have that but it's necessary for the 1.5 -> 2.0 transition. Once Mozmill 2.0 works without failures on production for a while we can start making those changes.

> Only question remaining is where you would prefer tests to be disabled. This
> can either be when including the manifest that specifies the test files, or
> can be in the manifest that includes that manifest. See the latest patch,
> and let me know if you think I should do it differently.

Personally I would prefer to skip at the highest possible level. Means if all the tests in a subfolder should be disabled, then mark the include as disabled.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 639035 [details] [diff] [review]
Add restart tests and disable skipped tests in manifest files. v2.1

Looks good. I will get this landed right away.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Pushed to default:
http://hg.mozilla.org/qa/mozmill-tests/rev/5e548773fed1

We should also get this landed on older branches once the patch on bug 751142 has been landed everywhere.

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add restart tests and disable skipped tests in manifest files. v2.1 (mozilla-aurora, mozilla-beta, mozilla-release)

Patch for mozilla-aurora, mozilla-beta, and mozilla-release.

Comment 16

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add restart tests and disable skipped tests in manifest files. v2.1 (mozilla-esr10)

Patch for mozilla-esr10

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 639494 [details] [diff] [review]
Add restart tests and disable skipped tests in manifest files. v2.1 (mozilla-aurora, mozilla-beta, mozilla-release)

>+++ b/tests/endurance/testTabView_SwitchTabs/manifest.ini
>@@ -0,0 +1,1 @@
>+[test1.js]

This is disabled for me on Aurora.

>+++ b/tests/endurance/testTabbedBrowsing_PinAndUnpinAppTab/manifest.ini
>@@ -0,0 +1,1 @@
>+[test1.js]

This too.

>+++ b/tests/functional/testPreferences/manifest.ini
>@@ -1,5 +1,6 @@
> [testPaneRetention.js]
> [testPreferredLanguage.js]
>+disabled = Bug 731158 - Mozmill test failure /testPreferences/testPreferredLanguage.js | could not find element Link: Gruppi

This is not skipped anymore.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 639495 [details] [diff] [review]
Add restart tests and disable skipped tests in manifest files. v2.1 (mozilla-esr10)

Mostly the same as for the other patch. We have to explicitly check which tests are disabled on which branch. It's a bit of work right now but once we have it right we can massively benefit from. Thanks for working on it Dave.

Comment 19

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Henrik Skupin (:whimboo) from comment #17)
> Comment on attachment 639494 [details] [diff] [review]
> Add restart tests and disable skipped tests in manifest files. v2.1
> (mozilla-aurora, mozilla-beta, mozilla-release)
> 
> >+++ b/tests/endurance/testTabView_SwitchTabs/manifest.ini
> >@@ -0,0 +1,1 @@
> >+[test1.js]
> 
> This is disabled for me on Aurora.

It's skipped in the patch too...

+[include:testTabView_SwitchTabs/manifest.ini]
+disabled = Bug 684801 - Timeout failure in /testTabView_SwitchTabs/test1.js | TabView is still open

> >+++ b/tests/endurance/testTabbedBrowsing_PinAndUnpinAppTab/manifest.ini
> >@@ -0,0 +1,1 @@
> >+[test1.js]
> 
> This too.

This too :)

+[include:testTabbedBrowsing_PinAndUnpinAppTab/manifest.ini]
+disabled = Bug 707663 - Timeout failure | Tab has scrolled into view

> >+++ b/tests/functional/testPreferences/manifest.ini
> >@@ -1,5 +1,6 @@
> > [testPaneRetention.js]
> > [testPreferredLanguage.js]
> >+disabled = Bug 731158 - Mozmill test failure /testPreferences/testPreferredLanguage.js | could not find element Link: Gruppi
> 
> This is not skipped anymore.

Only just. ;) I will update the patch.

Comment 20

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add restart tests and disable skipped tests in manifest files. v2.2 (mozilla-aurora, mozilla-beta, mozilla-release)

Comment 21

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add restart tests and disable skipped tests in manifest files. v2.2 (mozilla-esr10)

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Dave Hunt (:davehunt) from comment #19)
> > >+++ b/tests/endurance/testTabView_SwitchTabs/manifest.ini
> > >@@ -0,0 +1,1 @@
> > >+[test1.js]
> > 
> > This is disabled for me on Aurora.
> 
> It's skipped in the patch too...
> 
> +[include:testTabView_SwitchTabs/manifest.ini]
> +disabled = Bug 684801 - Timeout failure in /testTabView_SwitchTabs/test1.js
> | TabView is still open

Gotcha. Missed that sorry. Will take care of in the next review.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 639610 [details] [diff] [review]
Add restart tests and disable skipped tests in manifest files. v2.2 (mozilla-aurora, mozilla-beta, mozilla-release)

Looks good then.

Comment 24

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/f60086085fc1 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/fc09b2852ee3 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/2fa9bf2fb6f8 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/74485a9866ef (esr10)