Closed Bug 585788 Opened 14 years ago Closed 14 years ago

Make Mozmill-test testOpenSearchAutodiscovery local

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Product:
Mozilla QA Graveyard
Component:
Mozmill Tests
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: u279076, Assigned: aaronmt)

References

Details

(Whiteboard: [litmus-data])

Attachments

(6 files, 1 obsolete file)

Patch v1 (default)
6.02 KB, patch
aaronmt
: review-
Details | Diff | Splinter Review
Patch v2 - (default)
6.14 KB, patch
u279076
: review+
whimboo
: review-
Details | Diff | Splinter Review
Patch v2.1 - (default)
6.14 KB, patch
whimboo
: review-
Details | Diff | Splinter Review
Patch v3.1 (default)
7.23 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
Patch v3.1 (1.9.2)
7.19 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
Patch v3.1 (1.9.1)
7.19 KB, patch
whimboo
: review+
Details | Diff | Splinter Review 
Module: testSearch/testOpenSearchAutodiscovery.js
Test-page: test-files/search/opensearch.html
Blocks: 579965
Assignee: nobody → anthony.s.hughes
Status: NEW → ASSIGNED

        Attachment #464977 -
        Flags: review?(aaron.train)

    
    

    
  
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-

    
    

    
  


  
general cleanup + fixes from my comment above

        Attachment #478118 -
        Flags: review?(anthony.s.hughes)

    
  

        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-

    
    

    
  


  
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

    
    

    
  


  

        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?

    
    

    
  
(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.

    
    

    
  
(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.

    
    

    
  


  
Final revisions made.

        Attachment #478359 -
        Attachment is obsolete: true

        Attachment #478359 -
        Flags: review?(hskupin)

    
    

    
  
(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]

    
    

    
  


  
Backport patch.

        Attachment #478461 -
        Flags: review?(hskupin)

        Attachment #478461 -
        Flags: review?(hskupin) → review+

    
    

    
  
(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]

    
    

    
  


  
Backport patch for 1.9.1

        Attachment #478672 -
        Flags: review?(hskupin)

        Attachment #478672 -
        Flags: review?(hskupin) → review+

    
  
Keywords: checkin-needed

    
    

    
  
(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
Closed: 14 years ago
Resolution: --- → FIXED

    
  
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.
Product: Testing → Mozilla QA
Version: 1.9.1 Branch → unspecified

    
  
Product: Mozilla QA → Mozilla QA Graveyard

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: