1037443 - Remove __force_skip__ statements from mozmill-tests

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P4)

Description

[Andrei Eftimie]

Since all disabling of tests is now done at manifest level, we should remove all __force_skip__ statements from the code.

Comment 1

[ug201210031]

I am a newbie and willing to fix this bug.
I found these two lines as per the following link:

http://mxr.mozilla.org/mozilla-central/search?string=__force_skip__&find=&findi=&filter=&hitlimit=&tree=mozilla-central

Please guide me what to do.

Comment 2

[Anuj Pahuja]

Attachment: 1037443.diff

Removed all instances of __force_skip__ statements from mozmill. Is there anything else that needs to be done?

Comment 3

[Anuj Pahuja]

^Please ignore my patch. I didn't make any changes in mozmill-tests. Only in the mozilla-central repo.

Comment 4

[Andrei Eftimie]

Comment on attachment 8477915 [details] [diff] [review]
1037443.diff

Hi Anuj,

This is the mozmill-tests repository: http://hg.mozilla.org/qa/mozmill-tests

There are a couple of __force_skip__ statements that will likely need to remain, for example the ones in http://hg.mozilla.org/qa/mozmill-tests/file/03dd84d656d3/firefox/tests/functional/restartTests/testRestartChangeArchitecture because they can _only_ be run under OSX

We want the ones temporarily disabled because of some bug like:
> setupModule.__force_skip__ = "Bug 930509 - Window has been found.";
> teardownModule.__force_skip__ = "Bug 930509 - Window has been found.";
from http://hg.mozilla.org/qa/mozmill-tests/file/03dd84d656d3/firefox/tests/functional/testDownloading/testCloseDownloadManager.js#l76

Comment 5

[Andrei Eftimie]

Hi guys,

both of you showed interest in this bug and I'm torn to make a decision here, so whoever comes first please also assign this bug to yourself, the other please pick a new bug from the list of good first bugs: https://bugzilla.mozilla.org/buglist.cgi?list_id=11059480&classification=Components&classification=Other&status_whiteboard_type=allwordssubstr&query_format=advanced&status_whiteboard=%5Bgood%20first%20bug%5D&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=REOPENED&component=Infrastructure&component=Mozmill&component=Mozmill%20Tests&product=Mozilla%20QA&product=Testing

Comment 6

[Manu Jain]

Attachment: bug1037443.patch

Is this correct?

Comment 7

[Andrei Eftimie]

Comment on attachment 8487435 [details] [diff] [review]
bug1037443.patch

Review of attachment 8487435 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Manu,

This is a good start, but there are a few more instances where this needs to be removed.
I will assign this bug to you.

I've done a quick search and I've found the following:
> firefox/tests/functional/testDownloading/testOpenDownloadManager.js
> firefox/tests/functional/testPreferences/testRestoreHomepageToDefault.js
> firefox/tests/functional/testPreferences/testSetToCurrentPage.js
> firefox/tests/functional/testPrivateBrowsing/testPrivateDownloadPanel.js
> firefox/tests/functional/testToolbar/testHomeButton.js
> firefox/tests/remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js
> firefox/tests/remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test2.js
> firefox/tests/remote/restartTests/testDiscoveryPane_FeaturedModule/test1.js
> firefox/tests/remote/restartTests/testDiscoveryPane_FirstTimeModule/test1.js
> firefox/tests/remote/restartTests/testDiscoveryPane_installAddonWithEULA/test1.js
> firefox/tests/remote/restartTests/testDiscoveryPane_installCollectionAddon/test1.js
> firefox/tests/remote/restartTests/testDiscoveryPane_installPickOfMonthAddon/test1.js
> firefox/tests/remote/restartTests/testDiscoveryPane_UpAndComingModule/test1.js
> firefox/tests/remote/testDownloading/testDownloadStates.js

Comment 8

[Manu Jain]

Attachment: bug_1037443.patch

I've removed all _force_skip_ statements.

Comment 9

[Andrei Eftimie]

Attachment: bug_1037443_v3.patch

Thanks Manu for the patch, this indeed works fine, and you got all instances that are being used atm.

I have updated the patch because there was 1 easy to solve conflict (and updated the commit message).

Landed:
https://hg.mozilla.org/qa/mozmill-tests/rev/317bc75d3f99 (default)

Comment 10

[Andrei Eftimie]

I initially thought of not backporting this, but upon further examination it might lead to more work in the future for other patches if they won't apply cleanly because we are touching multiple files here.

So we should probably backport this.

Manu, please try applying the landed patch on the rest of the branches (and check if there are other __force_skip__ statements that would need to be removed - we might have tests on other branches which are not available on default anymore).

Report here if it applies correctly, and for the branches it wont, fix the conflicts, generate a new patch and upload it here.

Thanks!

Comment 11

[Manu Jain]

Andrei, can you tell how to apply the patch on other branches and how to fix the conflicts? Thanks!!

Comment 12

[Andrei Eftimie]

(In reply to manu.jain13 from comment #11)
> Andrei, can you tell how to apply the patch on other branches and how to fix
> the conflicts? Thanks!!

Surely.

First you should update the repository to have the landed patch locally:
> hg pull -u

Lets check what branches we have:
> hg branches

Lets try 'mozill-aurora' (which is now firefox 34):
> hg checkout mozilla-aurora

Now lets apply the patch on the currently checked-out branch:
(in this case download the final patch I applied since it got a minor change, usually it should be the one you already have)
> hg qimport %path/to/patch%

Or if you have installed qimportbz (from https://hg.mozilla.org/hgcustom/version-control-tools/)
> hg qimport bz:1037443 // this will import the patch directly from this bug

Once we have the patch in our queue try to push it:
> hg qpush

If this works fine, the patch applies cleanly. For this bug I would do another search for __force_skip__ and make sure no other instances that we would like removed exist, run a testrun to make sur everything is fine, and post in this bug the results.

If this won't apply cleanly the conflicts will be saved in .rej files.
You should be able to see them easily with `hg status`. Apply the changes manually to the files, remove the .rej file, run a `hg qrefresh`, export a new patch and upload it here mentioning the branch for which it is.

Comment 13

[Manu Jain]

Attachment: bug_1037443_branch=aurora.patch

Branch- Mozilla aurora patch.

Comment 14

[Manu Jain]

Attachment: bug_1037443_mozilla-esr31.patch

Branch- Mozilla esr31 patch.

Comment 15

[Andrei Eftimie]

Comment on attachment 8491508 [details] [diff] [review]
bug_1037443_branch=aurora.patch

Review of attachment 8491508 [details] [diff] [review]:
-----------------------------------------------------------------

Landed:
https://hg.mozilla.org/qa/mozmill-tests/rev/5813895813a4 (mozilla-aurora)
https://hg.mozilla.org/qa/mozmill-tests/rev/ef0055fa1d6e (mozilla-beta)

Comment 16

[Manu Jain]

Andrei, I've also attached esr-31 patch. Can you tell which branch is for firefox32, as there is no branch named firefox32. Thanks!!

Comment 17

[Andrei Eftimie]

(In reply to Manu Jain from comment #16)
> Andrei, I've also attached esr-31 patch. Can you tell which branch is for
> firefox32, as there is no branch named firefox32. Thanks!!

I saw that Manu, I look into landing that today.

This is how the branches currently correspond to Firefox versions:
> default         // 35
> mozilla-aurora  // 34
> mozilla-beta    // 33
> mozilla-release // 32
> mozilla-esr31   // 31 (esr)

Each 6 weeks a new Firefox version comes out so they all get pushed down (expect the Extended Support Release, which is supported for ~1 year) 


There's nothing for you to do here right now, I will look into and land the patch for Release and ESR31, and hopefully we'll all be done :)

Comment 18

[Andrei Eftimie]

Comment on attachment 8491720 [details] [diff] [review]
bug_1037443_mozilla-esr31.patch

Review of attachment 8491720 [details] [diff] [review]:
-----------------------------------------------------------------

Everything works fine on release and esr31.

Landed:
https://hg.mozilla.org/qa/mozmill-tests/rev/b6e8b90eef1a (mozilla-release)
https://hg.mozilla.org/qa/mozmill-tests/rev/beb218205683 (mozilla-esr31)

Comment 19

[Andrei Eftimie]

Thanks Manu for the work.
If you wish to help out on anything else, let me know.

Here is a list of simple tasks that are open in relation to Automation and Javascript:
http://www.joshmatthews.net/bugsahoy/?automation=1&js=1&simple=1

This issue is fixed.