568008 - [mozmill] Update Mozmill tests which are affected by the new Litmus test data location

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Last week we have moved the litmus test data from the Litmus dev repository (http://hg.mozilla.org/webtools/litmus/) to our own test data repository (http://hg.mozilla.org/qa/litmus-data/). Some of our Mozmill tests are referencing testcases at the old location.

Anthony, can you please check which scripts we have to update? Also for any test modules we will have to create a local version of the minimized test data in our mozmill repository, which we can load via httpd.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Please take this with higher priority as other open test failures.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Anthony, please move all local test data files to the central place under firefox/files. That way we can have a 1:1 relationship between the Litmus test data files and our Mozmill versions.

Comment 3

[u279076]

Here is a quick pass of what scripts are affected and how:

grep -r "testcase_files" .
./firefox/testPopups/testPopupsAllowed.js:  var url = "https://litmus.mozilla.org/testcase_files/firefox/5918/index.html";
./firefox/testPopups/testPopupsBlocked.js:  var url = "https://litmus.mozilla.org/testcase_files/firefox/5918/index.html";
./firefox/testPrivateBrowsing/testDisabledPermissions.js:                "https://litmus.mozilla.org/testcase_files/firefox/5918/index.html",
./firefox/testPrivateBrowsing/testDisabledPermissions.js:                "https://litmus.mozilla.org/testcase_files/firefox/cookies/cookie_single.html"
./firefox/testSearch/testAddMozSearchProvider.js:                      url : "https://litmus.mozilla.org/testcase_files/firefox/search/mozsearch.html"};

Comment 4

[u279076]

Attachment: Patch v1

This patch does a few different things:

1. Creates the necessary local litmus-data structure and files
2. Updates the four test files which were using the old litmus-data:
  * testPopupAllowed
  * testPopupBlocked
  * testDisabledPermissions
  * testAddMozSearchPlugin
3. Resolves failures in the above tests related to moving of Litmus Data

Comment 5

[u279076]

Attachment: Patch v1

Sorry, had some debugging code in that last patch.  Resubmitting with debugging code removed.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 449744 [details] [diff] [review]
Patch v1

>\ No newline at end of file

Somehow all test files don't have an empty line at the end. I will add those before I check them into our repository.

Please create a specific patch for mozilla1.9.1 based on the one I will upload in a second.

Anthony, please also update all of our existing tests which already make use of the httpd server. All those test files should be moved to the firefox/test-files folder. Use 'hg mv' for it. We can check this in as a follow-up.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Attachment: Updated patch (trunk, mozilla1.9.2) [checked-in]

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 449899 [details] [diff] [review]
Updated patch (trunk, mozilla1.9.2) [checked-in]

Landed on default and 1.9.2:
http://hg.mozilla.org/qa/mozmill-tests/rev/1844482c24e5
http://hg.mozilla.org/qa/mozmill-tests/rev/095705fe76d6

Comment 9

[u279076]

Attachment: Patch v2 (1.9.1) [checked-in]

I found an inconsistency across branches:

// Include necessary modules
var RELATIVE_ROOT = '../../shared-modules';

Some tests include the comment, some do not.  This causes patch failures when the comment exists on one branch but not another.  This patch includes the requested revisions plus adds this comment to all files which are missing it.

Comment 10

[u279076]

I'll be adding the comment inconsistency mentioned in the previous comment to DEFAULT via the httpd patch.

Comment 11

[u279076]

Just FYI, here is the grep I used to identify files missing the comment:

grep -r -l "var RELATIVE_ROOT" `grep -r -L "// Include necessary modules" .`

Comment 12

[u279076]

Attachment: Follow-up - moving test files internally (default)

This patch fixes the commenting inconsistency with INCLUDED MODULES on DEFAULT branch.  
This also moves any existing local test files to the new firefox/test-files location to create consistency with litmus-date.  
Finally, this updates any affected tests by the previously mentioned move.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 449912 [details] [diff] [review]
Patch v2 (1.9.1) [checked-in]

Landed on 1.9.1 as:
http://hg.mozilla.org/qa/mozmill-tests/rev/6e3f4586bb19

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 449919 [details] [diff] [review]
Follow-up - moving test files internally (default)

>rename from firefox/testPrivateBrowsing/files/geolocation.html
>rename to firefox/test-files/geolocation/geolocation.html

Please use position.html as name so that we are in sync with the litmus-data repository.

>rename from firefox/testAddons/files/plugin.html
>rename to firefox/test-files/plugins/plugin.html

Please rename it to default_plugin.html.

>-   controller.open(localTestFolder + 'openinnewtab_target.html?id=' + i);
>+   controller.open(localTestFolder + '/tabbedbrowsing/openinnewtab_target.html?id=' + i);

Please split it up into two lines.

With those changes, r=me.

Comment 15

[u279076]

Attachment: Patch v3 [checked-in]

Here's a patch with the latest nits addressed.  There is one anomaly in this patch:

>rename from firefox/testAddons/files/plugin.html
>rename to firefox/test-files/plugins/plugin.html
Please rename it to default_plugin.html.

For some reason, the hg mv registers in a patch as a deletion of plugin.html and a creation of default_plugin.html (not a "rename").

I think this is minor and should still apply ok.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 450206 [details] [diff] [review]
Patch v3 [checked-in]

> >rename from firefox/testAddons/files/plugin.html
> >rename to firefox/test-files/plugins/plugin.html
> Please rename it to default_plugin.html.
> 
> For some reason, the hg mv registers in a patch as a deletion of plugin.html
> and a creation of default_plugin.html (not a "rename").
> 
> I think this is minor and should still apply ok.

That's fine. We really don't loose any important information here. A test would be more critical.

Please use another commit message the next time when you create a follow-up for the same branch.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 450206 [details] [diff] [review]
Patch v3 [checked-in]

Landed on default and 1.9.2:
http://hg.mozilla.org/qa/mozmill-tests/rev/2e25ec611a52
http://hg.mozilla.org/qa/mozmill-tests/rev/1857ec9e8826

So only a patch for 1.9.1 is remaining before we can call this fixed.

Comment 18

[u279076]

> Please use another commit message the next time when you create a follow-up for
the same branch.

How?  I'll use this for the 1.9.1 follow up patch.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Same as what you did initially. Run 'hg qrefresh -m "message". See the message I have used for the check-in and which you should use for 1.9.1 too.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Anthony, while working with Jeff on bug 570596, I have seen that we accidentally have two slashes right after the port number. The reason is how we define the local test folder and add the test file itself:

const localTestFolder = collector.addHttpResource('../test-files');
controller.open(localTestFolder + "/tabbedbrowsing/openinnewtab.html"); 

To behave correctly we have to use the following method:

const localTestFolder = collector.addHttpResource('../test-files/');
controller.open(localTestFolder + "tabbedbrowsing/openinnewtab.html"); 

Please make sure in your remaining patch for 1.9.1 that this is included and also create another follow-up for default and 1.9.2.

Comment 21

[u279076]

Attachment: httpd path updates (default, mozilla1.9.2)

Here is a revision to DEFAULT which should fix the "localhost:port//folder/file.html" issue.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 450683 [details] [diff] [review]
httpd path updates (default, mozilla1.9.2)

r=me

Landed on default and mozilla1.9.2 as:
http://hg.mozilla.org/qa/mozmill-tests/rev/be04885ffef8
http://hg.mozilla.org/qa/mozmill-tests/rev/f2834e5103c1

Please combine the remaining work for mozilla1.9.1 in a single patch.

Comment 23

[u279076]

Attachment: Patch v3 (1.9.1)

This patch combines all the default/mozilla1.9.2 changes for mozilla1.9.1.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 450708 [details] [diff] [review]
Patch v3 (1.9.1)

>  * Contributor(s):
>+ *   Anthony Hughes <[e-mail]>
>  *   Henrik Skupin <[e-mail]>

Even that I have said I want to check-in those changes, I can't. The list of contributors isn't sorted alphabetically and if "(Original Author)" hasn't been explicitly added, the first entry shows the original author. That said please don't add yourself always implicitly as original author of the tests.

While you have to check that please also make sure to only add yourself to a test when you have made a real code contribution.  

With all those updates r=me.

Comment 25

[u279076]

You told me on skype this would not be an issue, provided I did not do this in the future.  Blocking this checkin because you don't think I've contributed enough to be called a "contributor" unnecessarily blocks progress.  I need this before working on all other failures and you wanted this patch checked in early this week.  You not checking this in means you've delayed checkin a week from the original timeline; resulting in this being a failed goal.

You should not be blocking on something so trivial this close to the end of the quarter.

I emplore you to check this is tonight to get the necessary changes in place.  If you do that, I'll follow up with a patch to remove the line I added to the license block; if it's that important to you.

As I spoke with you on skype, any further revisions to this will not come from me until Monday.

Comment 26

[u279076]

(In reply to comment #24)
> (From update of attachment 450708 [details] [diff] [review])
> >  * Contributor(s):
> >+ *   Anthony Hughes <[e-mail]>
> >  *   Henrik Skupin <[e-mail]>
> 
> Even that I have said I want to check-in those changes, I can't. The list of
> contributors isn't sorted alphabetically and if "(Original Author)" hasn't been
> explicitly added, the first entry shows the original author. That said please
> don't add yourself always implicitly as original author of the tests.
> 
> While you have to check that please also make sure to only add yourself to a
> test when you have made a real code contribution.  
> 
> With all those updates r=me.

Have re-read this, I think I misinterpreted your English.  I always thought the contributor list was alphabetic.  In fact, I've advised other contributors to that effect.  The list is in order of contribution I suppose.

Please check in this patch as is.  It's important that we get the fixes checked in ASAP.  I'll submit a follow-up to this bug which corrects the order of the contributors list to all tests I've contributed to.

I don't think this should block checking in fixes.

Comment 27

[Matt Evans [:mevans]]

Henrik, I'm going to have side with Anthony on this. If we are spending time on the correct order of the contributor comment, this is not a good use of time and resources. Lets focus on correct functionality 1st. Thanks Matt.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Sorry, but the request I have made is a normal for the review process. I don't wanna discuss anything here on the bug. But I have made those updates in 2 minutes. No idea why this would have been affected any of your goals done by next week.(In reply to comment #26)

> (In reply to comment #24)
> Please check in this patch as is.  It's important that we get the fixes checked
> in ASAP.  I'll submit a follow-up to this bug which corrects the order of the
> contributors list to all tests I've contributed to.
> 
> I don't think this should block checking in fixes.

We have a review process and it would be nice when you can respect it. Starting a discussion like above has probably been taken longer as to quickly fix the patch.

As said above, fixed in under 2 minutes and checked-in:
http://hg.mozilla.org/qa/mozmill-tests/rev/9fea9dc4adaf

Comment 29

[u279076]

Thanks for checking it in.

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

I had to land a follow-up breakage fix for testDisableEnablePlugin.js because it was only half-wise patched. Should be fixed now on all branches:

http://hg.mozilla.org/qa/mozmill-tests/rev/09523a42d459
http://hg.mozilla.org/qa/mozmill-tests/rev/90ce586727e0
http://hg.mozilla.org/qa/mozmill-tests/rev/507d3aa49e8a

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.