Make Mozmill-test testOpenSearchAutodiscovery local

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: ashughes, Assigned: aaronmt)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [litmus-data])

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
Module: testSearch/testOpenSearchAutodiscovery.js
Test-page: test-files/search/opensearch.html
(Reporter)

Updated

7 years ago
Blocks: 579965
(Reporter)

Updated

7 years ago
Assignee: nobody → anthony.s.hughes
Status: NEW → ASSIGNED
(Reporter)

Comment 1

7 years ago
Created attachment 464977 [details] [diff] [review]
Patch v1 (default)
Attachment #464977 - Flags: review?(aaron.train)
(Assignee)

Comment 2

7 years ago
Comment on attachment 464977 [details] [diff] [review]
Patch v1 (default)

>+<html><head>
>+  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
>+  <title>Add OpenSearch Plugin</title>
>+  <link rel="search" type="application/opensearchdescription+xml" title="OpenSearch Test" href="http://hg.mozilla.org/qa/litmus-data/raw-file/95d5a0ba395e/firefox/search/opensearch.xml">
Check your indentation on <head> and that <link> should be divided up into a new line

>+<body>
>+  Click <a href="#" name="add" onclick="add();">here</a> to add the Mozilla
>+  Test OpenSearch plugin.
>+
>+
>+</body></html>
Here too
>\ No newline at end of file
New line
>diff --git a/firefox/test-files/search/opensearch.xml b/firefox/test-files/search/opensearch.xml
>\ No newline at end of file
New line
>+const searchEngine = {name: "OpenSearch Test",
>+                      url : LOCAL_TEST_FOLDER + 'search/opensearch.html'};
SEARCHENGINE or SEARCH_ENGINE
Attachment #464977 - Flags: review?(aaron.train) → review-
(Assignee)

Comment 3

7 years ago
Created attachment 478118 [details] [diff] [review]
Patch v2 - (default)

general cleanup + fixes from my comment above
Attachment #478118 - Flags: review?(anthony.s.hughes)
(Reporter)

Updated

7 years ago
Attachment #478118 - Flags: review?(hskupin)
Attachment #478118 - Flags: review?(anthony.s.hughes)
Attachment #478118 - Flags: review+
Comment on attachment 478118 [details] [diff] [review]
Patch v2 - (default)

Doesn't apply:

applying search
unable to strip away 1 dirs from 814880ff6a9a
unable to strip away 1 dirs from 814880ff6a9a
patching file firefox/testSearch/testOpenSearchAutodiscovery.js
patch failed to apply
firefox/testSearch/testOpenSearchAutodiscovery.js
patch failed, rejects left in working dir
errors during apply, please fix and refresh search
Attachment #478118 - Flags: review?(hskupin) → review-
(Assignee)

Comment 5

7 years ago
Created attachment 478242 [details] [diff] [review]
Patch v2.1 - (default)

Previous patch works for me on a clean clone and apply. I created a new patch which also has no issues for me applying on a clone and looks no different than the previous patch, try it out as well. Anthony, can you try it too?
Attachment #478242 - Flags: review?(hskupin)
(In reply to comment #5)
> Previous patch works for me on a clean clone and apply. I created a new patch

Probably my fault. Looks like that I have accidentally used the unified raw patch.
Comment on attachment 478242 [details] [diff] [review]
Patch v2.1 - (default)

>+  <link rel="search" type="application/opensearchdescription+xml" title="OpenSearch Test" 
>+        href="http://hg.mozilla.org/qa/litmus-data/raw-file/95d5a0ba395e/firefox/search/opensearch.xml">

This doesn't make the test local. In our case we will have to use the local version of the opensearch.xml file.

>+++ b/firefox/test-files/search/opensearch.xml	Fri Sep 24 08:41:20 2010 -0400
[..]
>+  <Url type="text/html" method="get" template="http://hg.mozilla.org/qa/litmus-data/raw-file/tip/firefox/search/searchresults.html?q={searchTerms}"/>

Same here.

>+const SEARCH_ENGINE = {

>-  var engine = search.getElement({type: "engine", subtype: "title", value: addEngines[0].name});
>+  var engine = search.getElement({type: "engine", 
>+                                  subtype: "title", 
>+                                  value: addEngines[0].name});
[..]
>+  controller.waitForEval("subject.search.selectedEngine == subject.newEngine", TIMEOUT, 100,
>+                         {search: search, 
>+                          newEngine: SEARCH_ENGINE.name});
[..]
>   controller.assertJS("subject.placeholder == subject.engineName",
>                       {placeholder: inputField.getNode().placeholder,
>-                       engineName: searchEngine.name});
>+                       engineName: SEARCH_ENGINE.name});

JSON style indentation.
Attachment #478242 - Flags: review?(hskupin) → review-
Assignee: anthony.s.hughes → aaron.train
Version: unspecified → 1.9.1 Branch
(Assignee)

Comment 8

7 years ago
Created attachment 478359 [details] [diff] [review]
Patch v3 - (default) + [local fix + indent]
Attachment #478359 - Flags: review?(hskupin)
Comment on attachment 478359 [details] [diff] [review]
Patch v3 - (default) + [local fix + indent]

>+        href="http://localhost:43336/search/opensearch.xml">

This testcase is probably a bit harder to make it really local for now. If it should work at 100% of the times we can't hardcode the port in the file. Instead we would have to create a temporary copy of the file. :/ So I wonder if we should go with the last version of the patch and do the remaining work next quarter. Anthony, what do you think?
(Reporter)

Comment 10

7 years ago
(In reply to comment #9)
> Comment on attachment 478359 [details] [diff] [review]
> Patch v3 - (default) + [local fix + indent]
> 
> >+        href="http://localhost:43336/search/opensearch.xml">
> 
> This testcase is probably a bit harder to make it really local for now. If it
> should work at 100% of the times we can't hardcode the port in the file.
> Instead we would have to create a temporary copy of the file. :/ So I wonder if
> we should go with the last version of the patch and do the remaining work next
> quarter. Anthony, what do you think?

The html and xml files are in the same folder.  We COULD use relative path names.  I've tested this and it works:

<link rel="search" type="application/opensearchdescription+xml" 
      title="OpenSearch Test" href="opensearch.xml">

If you are ok with this, I'm fine checking it in like this until we find a better solution.
(In reply to comment #10)
> <link rel="search" type="application/opensearchdescription+xml" 
>       title="OpenSearch Test" href="opensearch.xml">
> 
> If you are ok with this, I'm fine checking it in like this until we find a
> better solution.

That would fix it half-wise. But the opensearch.xml still points to a port on localhost. Eventually we should simply let it point to the litmus data for now. We can fix it together with the other search tests next month.
(Reporter)

Comment 12

7 years ago
(In reply to comment #11)
> (In reply to comment #10)
> > <link rel="search" type="application/opensearchdescription+xml" 
> >       title="OpenSearch Test" href="opensearch.xml">
> > 
> > If you are ok with this, I'm fine checking it in like this until we find a
> > better solution.
> 
> That would fix it half-wise. But the opensearch.xml still points to a port on
> localhost. Eventually we should simply let it point to the litmus data for now.
> We can fix it together with the other search tests next month.

So, for now, we can just use:
http://hg.mozilla.org/qa/litmus-data/raw-file/tip/firefox/search/<file>

I can update both opensearch.xml and opensearch.html to use the same URL.  Is that ok?
(In reply to comment #12)
> I can update both opensearch.xml and opensearch.html to use the same URL.  Is
> that ok?

Please only update the html file so we use the local xml file but perform the search on hg.mozilla.org.
(Reporter)

Comment 14

7 years ago
Created attachment 478443 [details] [diff] [review]
Patch v3.1 (default)

Final revisions made.
Attachment #478359 - Attachment is obsolete: true
Attachment #478359 - Flags: review?(hskupin)
(Reporter)

Comment 15

7 years ago
(In reply to comment #14)
> Created attachment 478443 [details] [diff] [review]
> Patch v3.1 (default)
> 
> Final revisions made.

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/3b6a535b9d4f [default]
(Reporter)

Comment 16

7 years ago
Created attachment 478461 [details] [diff] [review]
Patch v3.1 (1.9.2)

Backport patch.
Attachment #478461 - Flags: review?(hskupin)
Attachment #478443 - Flags: review+
Attachment #478461 - Flags: review?(hskupin) → review+
(Reporter)

Comment 17

7 years ago
(In reply to comment #16)
> Created attachment 478461 [details] [diff] [review]
> Patch v3.1 (1.9.2)
> 
> Backport patch.

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/033b8306838a [mozilla1.9.2]
(Reporter)

Comment 18

7 years ago
Created attachment 478672 [details] [diff] [review]
Patch v3.1 (1.9.1)

Backport patch for 1.9.1
Attachment #478672 - Flags: review?(hskupin)
Attachment #478672 - Flags: review?(hskupin) → review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(Reporter)

Comment 19

7 years ago
(In reply to comment #18)
> Created attachment 478672 [details] [diff] [review]
> Patch v3.1 (1.9.1)
> 
> Backport patch for 1.9.1

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/65f59b1af947 [mozilla1.9.1]
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Updated

7 years ago
Keywords: checkin-needed
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.
Component: Mozmill Tests → Mozmill Tests
Product: Testing → Mozilla QA
Version: 1.9.1 Branch → unspecified
You need to log in before you can comment on or make changes to this bug.