751142 - Add manifest files for running entire test suite

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

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

Manifests are supported for 1.5.x and 2.x, and we are going to want to use them going forward. This bug covers adding manifest files that work for both current and future Mozmill releases so that we can switch the automation scripts to run tests via the manifests (-m command line option) instead of tests (-t).

Comment 1

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

Attachment: Manifest files for Mozmill 1.5.x v1.0

The attached patch works with Mozmill 1.5.x but will not work for Mozmill 2.x. The reason is the restart tests, which for Mozmill 1.x must be specified as directories, but for Mozmill 2.x are specified as files. When we upgrade our tests for Mozmill 2.x, we can also update the manifest files if needed, however I suspect we'll take advantage of the ability to invoke a restart from within a test, and the current concept of 'restartTests' will be made redundant.

Comment 2

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

Comment on attachment 620363 [details] [diff] [review]
Manifest files for Mozmill 1.5.x v1.0

Comments inline. Just some additional notes in-front:

* Would be great if you can also add a manifest for /lib/tests

>+++ b/tests/endurance/manifest.ini
>+[testAddons_OpenAndCloseExtensionList]
>+[testAddons_OpenAndCloseGetAddonsPane]
>+[testBackForwardNavigation]
>+[testBookmarks_AddAndRemoveBookmarkViaAwesomeBar]
>+[testBookmarks_OpenAllInTabs]
>+[testFlash_SWFVideoEmbed]
>+[testFlash_SWFVideoObject]
>+[testPrivateBrowsing_EnterAndLeave]
>+[testTabView_OpenNewTab]
>+[testTabView_OpenTabViewWithTabs]
>+[testTabView_SwitchTabs]
>+[testTabbedBrowsing_OpenNewTab]
>+[testTabbedBrowsing_PinAndUnpinAppTab]

How would we handle the reserved Endurance testrun (MemBuster)?

>+++ b/tests/l10n/manifest.ini
>@@ -0,0 +1,2 @@
>+[include:testAccessKeys/manifest.ini]
>+[include:testCropped/manifest.ini]

Both are restart tests and do not need additional manifest files.

>+++ b/tests/remote/restartTests/manifest.ini
>@@ -0,0 +1,6 @@
>+[testDiscoveryPane_FeaturedModule]
>+[testDiscoveryPane_FirstTimeModule]
>+[testDiscoveryPane_UpAndComingModule]
>+[testDiscoveryPane_installAddonWithEULA]
>+[testDiscoveryPane_installCollectionAddon]
>+[testDiscoveryPane_installPickOfMonthAddon]

What about the non-restart dummy test? This also has to be added or do we not fail with the -m option here if no test has been specified?

>+++ b/tests/update/testDirectUpdate/manifest.ini
>@@ -0,0 +1,3 @@
>+[test1.js]
>+[test2.js]
>+[test3.js]
>diff --git a/tests/update/testFallbackUpdate/manifest.ini b/tests/update/testFallbackUpdate/manifest.ini
>new file mode 100644
>--- /dev/null
>+++ b/tests/update/testFallbackUpdate/manifest.ini
>@@ -0,0 +1,4 @@
>+[test1.js]
>+[test2.js]
>+[test3.js]
>+[test4.js]

Why are you adding the individual tests here but not for all the other restart tests e.g. functional and remote?

Comment 3

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

(In reply to Henrik Skupin (:whimboo) from comment #2)
> Comment on attachment 620363 [details] [diff] [review]
> Manifest files for Mozmill 1.5.x v1.0
> 
> Comments inline. Just some additional notes in-front:
> 
> * Would be great if you can also add a manifest for /lib/tests

Of course, I'll add this.

> How would we handle the reserved Endurance testrun (MemBuster)?

I was thinking it would be the same way we currently do - specify it via the test command line rather than a manifest. Or did you have a better suggestion?

> >+++ b/tests/l10n/manifest.ini
> >@@ -0,0 +1,2 @@
> >+[include:testAccessKeys/manifest.ini]
> >+[include:testCropped/manifest.ini]
> 
> Both are restart tests and do not need additional manifest files.

My mistake, sorry.

> What about the non-restart dummy test? This also has to be added or do we
> not fail with the -m option here if no test has been specified?

I thought one of the advantages of using manifests was that we no longer needed the dummy tests.. I did try using an empty manifest.ini and it basically launched Firefox as if no tests had been specified, so that's not ideal.

> Why are you adding the individual tests here but not for all the other
> restart tests e.g. functional and remote?

Once again, my mistake. It's not as obvious that these are restart tests (the others are in a directory named restartTests, which makes them unmistakable). I'll fix this up in a new version of the patch.

Comment 4

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

Attachment: Manifest files for Mozmill 1.5.x v1.1

Comment 5

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

(In reply to Dave Hunt (:davehunt) from comment #3)
> > How would we handle the reserved Endurance testrun (MemBuster)?
> 
> I was thinking it would be the same way we currently do - specify it via the
> test command line rather than a manifest. Or did you have a better
> suggestion?

We should always use the manifest files and totally get rid of the -t option.

> > What about the non-restart dummy test? This also has to be added or do we
> > not fail with the -m option here if no test has been specified?
> 
> I thought one of the advantages of using manifests was that we no longer
> needed the dummy tests.. I did try using an empty manifest.ini and it
> basically launched Firefox as if no tests had been specified, so that's not
> ideal.

What about the exit code? That's the important information here. If it not exits with 0 we can't get rid of those. And I think that's what we still have to depend on.

> > Why are you adding the individual tests here but not for all the other
> > restart tests e.g. functional and remote?
> 
> Once again, my mistake. It's not as obvious that these are restart tests
> (the others are in a directory named restartTests, which makes them
> unmistakable). I'll fix this up in a new version of the patch.

So I have checked the Mutt tests and as it looks like the restart tests need separate lines for each test. Can you please clarify that?

https://github.com/mozautomation/mozmill/tree/master/mutt/mutt/tests/js/restart/test_user_restart

With the latest version of the patch you have removed all of those entries.

Comment 6

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

Comment on attachment 620424 [details] [diff] [review]
Manifest files for Mozmill 1.5.x v1.1

Clearing review request until we have solved the remaining question.

Comment 7

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

(In reply to Henrik Skupin (:whimboo) from comment #5)
> (In reply to Dave Hunt (:davehunt) from comment #3)
> > > How would we handle the reserved Endurance testrun (MemBuster)?
> > 
> > I was thinking it would be the same way we currently do - specify it via the
> > test command line rather than a manifest. Or did you have a better
> > suggestion?
> 
> We should always use the manifest files and totally get rid of the -t option.

Okay, so a manifest just for a single test? How would you propose we do this? Something like membuster.ini?

> > > What about the non-restart dummy test? This also has to be added or do we
> > > not fail with the -m option here if no test has been specified?
> > 
> > I thought one of the advantages of using manifests was that we no longer
> > needed the dummy tests.. I did try using an empty manifest.ini and it
> > basically launched Firefox as if no tests had been specified, so that's not
> > ideal.
> 
> What about the exit code? That's the important information here. If it not
> exits with 0 we can't get rid of those. And I think that's what we still
> have to depend on.

If you attempt to run with a non-existent manifest file it exits with an exception: IOError: Missing files: manifest.ini. When we change the automation scripts I suspect we will be looking for testrun/manifest.ini, and we can even check if this exists first.

> > > Why are you adding the individual tests here but not for all the other
> > > restart tests e.g. functional and remote?
> > 
> > Once again, my mistake. It's not as obvious that these are restart tests
> > (the others are in a directory named restartTests, which makes them
> > unmistakable). I'll fix this up in a new version of the patch.
> 
> So I have checked the Mutt tests and as it looks like the restart tests need
> separate lines for each test. Can you please clarify that?
> 
> https://github.com/mozautomation/mozmill/tree/master/mutt/mutt/tests/js/
> restart/test_user_restart
> 
> With the latest version of the patch you have removed all of those entries.

With master I think that is correct, however with 1.5.x and mozmill-restart, the testrun fails if the manifest doesn't point to a directory. This is why this patch is not going to work with Mozmill 2.x as it is.

Comment 8

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

(In reply to Dave Hunt (:davehunt) from comment #7)
> Okay, so a manifest just for a single test? How would you propose we do
> this? Something like membuster.ini?

I would still use manifest.ini and store it under reserved.

> If you attempt to run with a non-existent manifest file it exits with an
> exception: IOError: Missing files: manifest.ini. When we change the
> automation scripts I suspect we will be looking for testrun/manifest.ini,
> and we can even check if this exists first.

So please create an empty manifest.ini for the dummy test in the remote folder. If no tests are getting run how does mozmill exit?

> With master I think that is correct, however with 1.5.x and mozmill-restart,
> the testrun fails if the manifest doesn't point to a directory. This is why
> this patch is not going to work with Mozmill 2.x as it is.

Ok, so I'm fine with it then. Thanks.

Comment 9

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

(In reply to Henrik Skupin (:whimboo) from comment #8)
> (In reply to Dave Hunt (:davehunt) from comment #7)
> > Okay, so a manifest just for a single test? How would you propose we do
> > this? Something like membuster.ini?
> 
> I would still use manifest.ini and store it under reserved.

The issue is that there may be other reserved tests in here in the future, and it would be very unlikely that you would want to ever run all tests in this directory. They are for a very single purpose.

> > If you attempt to run with a non-existent manifest file it exits with an
> > exception: IOError: Missing files: manifest.ini. When we change the
> > automation scripts I suspect we will be looking for testrun/manifest.ini,
> > and we can even check if this exists first.
> 
> So please create an empty manifest.ini for the dummy test in the remote
> folder. If no tests are getting run how does mozmill exit?

As mentioned in comment 3, an empty manifest file actually causes Firefox to launch and nothing more. It's like you ran without the -m option at all. I'm happy to raise this as a bug if this should actually exit with no tests executed.

Comment 10

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

(In reply to Dave Hunt (:davehunt) from comment #9)
> The issue is that there may be other reserved tests in here in the future,
> and it would be very unlikely that you would want to ever run all tests in
> this directory. They are for a very single purpose.

Makes sense. So use the value we specify via the testrun CLI as name of the manifest file.

> As mentioned in comment 3, an empty manifest file actually causes Firefox to
> launch and nothing more. It's like you ran without the -m option at all. I'm
> happy to raise this as a bug if this should actually exit with no tests
> executed.

This doesn't answer my question. What is the exit code of Mozmill? Is it 0 or 1?

Comment 11

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

(In reply to Henrik Skupin (:whimboo) from comment #10)
> (In reply to Dave Hunt (:davehunt) from comment #9)
> > The issue is that there may be other reserved tests in here in the future,
> > and it would be very unlikely that you would want to ever run all tests in
> > this directory. They are for a very single purpose.
> 
> Makes sense. So use the value we specify via the testrun CLI as name of the
> manifest file.

Okay, cool. This will be in the next version of the patch.

> 
> > As mentioned in comment 3, an empty manifest file actually causes Firefox to
> > launch and nothing more. It's like you ran without the -m option at all. I'm
> > happy to raise this as a bug if this should actually exit with no tests
> > executed.
> 
> This doesn't answer my question. What is the exit code of Mozmill? Is it 0
> or 1?

I think you're misunderstanding... there is no exit code because Firefox just sits idle. You have to close it manually.

Comment 12

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

(In reply to Dave Hunt (:davehunt) from comment #11)
> I think you're misunderstanding... there is no exit code because Firefox
> just sits idle. You have to close it manually.

Oh, can you please clarify this with Jeff? He has written all that code.

Comment 13

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

Certainly.

Jeff: If an empty manifest file is provided, what should be the expected result?

Right now, 1.5.11 is running as if no manifest file was provided (Firefox is launched and must be closed manually).

On master, Mozmill is exiting with "mozmill: error: No tests found. Please specify tests with -t or -m"

Is this a bug with 1.5.11?

Comment 14

[Jeff Hammel]

IIRC, in mozmill 1.5 if there are no tests mozmill launches Firefox and just sits there.  I have been told that this is not a bug, but I've always considered this bad behaviour.

https://github.com/mozautomation/mozmill/blob/hotfix-1.5/mozmill/mozmill/__init__.py#L795

Comment 15

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

Given that we do not really want to release another version of Mozmill 1.5.x I would say that we could go with the proposal from Dave and simply check if a manifest file is present and execute those tests only in such a case. That would give us the chance to remove the useless dummy tests.

Comment 16

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

Attachment: Manifest files for Mozmill 1.5.x v1.2

Comment 17

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/19ec9a02110d

Comment 18

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

Dave, can you please check that patch so we can get it landed on older branches? Please be careful because tests differ in existence and enabled state.

Comment 19

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

Attachment: Manifest files for Mozmill 1.5.x (mozilla-beta, mozilla-release)

Patch for mozilla-beta and mozilla-release.

Comment 20

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

Attachment: Manifest files for Mozmill 1.5.x (mozilla-esr10)

Patch for mozilla-esr10.

Comment 21

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

Comment on attachment 639114 [details] [diff] [review]
Manifest files for Mozmill 1.5.x (mozilla-beta, mozilla-release)

Looks good. I will land.

Comment 22

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

Pushed to other branches:
http://hg.mozilla.org/qa/mozmill-tests/rev/073a7fb8c1d4 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/dfa39da77e72 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/b54f33ee585d (esr10)