← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad

 

Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad.

Commit message:
Sitesearch code cleanup.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup/+merge/342472

Sitesearch code cleanup:

- Add unittests based on sitesearch doctests.
  - These doctests are still tested; unsure of removing as they're still useful documentation.
- Add tests for PageMatch and PageMatches.
- Use test scenarios to deduplicate sitesearch testservices testing code.
- Fix permissions on googletestservice.py script.
- Move config.{bing,google}.url_rewrite_exceptions into config.sitesearch.
- Update python sources following the standard template.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad.
=== modified file 'lib/lp/app/browser/doc/launchpad-search-pages-bing.txt'
--- lib/lp/app/browser/doc/launchpad-search-pages-bing.txt	2018-03-28 19:31:02 +0000
+++ lib/lp/app/browser/doc/launchpad-search-pages-bing.txt	2018-03-30 23:12:56 +0000
@@ -508,16 +508,14 @@
 are used for making links to the pages. The summary contains markup
 showing the matching terms in context of the page text.
 
-    >>> print range(20)
-    [0, 1, ..., 18, 19]
     >>> page = pages[0]
     >>> page
     <...PageMatch ...>
     >>> page.title
     u'Bugs - Launchpad Help'
     >>> page.url
-    'https://help.launchpad.net/Bugs'
-    >>> page.summary  # doctest: +ELLIPSIS
+    'http://bugs.launchpad.dev/ubuntu/hoary/+bug/2'
+    >>> page.summary
     u"Launchpad Help > Bugs . Use Launchpad's bug tracker for your project..."
 
 See `google-searchservice.txt` for more information about the

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2018-03-26 18:59:34 +0000
+++ lib/lp/services/config/schema-lazr.conf	2018-03-30 23:12:56 +0000
@@ -788,6 +788,7 @@
 # datatype: string, a url to a host
 site: http://www.google.com/cse
 
+[sitesearch]
 # url_rewrite_exceptions is a list of launchpad.net domains that must
 # not be rewritten.
 # datatype: string of space separated domains
@@ -824,12 +825,6 @@
 # datatype: string
 custom_config_id:
 
-# url_rewrite_exceptions is a list of launchpad.net domains that must
-# not be rewritten.
-# datatype: string of space separated domains
-# Example: help.launchpad.net login.launchpad.net
-url_rewrite_exceptions: help.launchpad.net
-
 [gpghandler]
 # Should we allow uploading keys to the keyserver?
 # datatype: boolean

=== modified file 'lib/lp/services/sitesearch/__init__.py'
--- lib/lp/services/sitesearch/__init__.py	2018-03-27 17:43:27 +0000
+++ lib/lp/services/sitesearch/__init__.py	2018-03-30 23:12:56 +0000
@@ -52,9 +52,9 @@
     def url_rewrite_exceptions(self):
         """A list of launchpad.net URLs that must not be rewritten.
 
-        Configured in config.google.url_rewrite_exceptions.
+        Configured in config.sitesearch.url_rewrite_exceptions.
         """
-        return config.google.url_rewrite_exceptions.split()
+        return config.sitesearch.url_rewrite_exceptions.split()
 
     @property
     def url_rewrite_scheme(self):
@@ -220,7 +220,7 @@
                 "The response errored: %s" % str(error))
         finally:
             action.finish()
-        page_matches = self._parse_google_search_protocol(response.content)
+        page_matches = self._parse_search_response(response.content)
         return page_matches
 
     def _checkParameter(self, name, value, is_int=False):
@@ -271,7 +271,7 @@
         """
         return self._getElementsByAttributeValue(doc, path, name, value)[0]
 
-    def _parse_google_search_protocol(self, gsp_xml):
+    def _parse_search_response(self, gsp_xml):
         """Return a `PageMatches` object.
 
         :param gsp_xml: A string that should be Google Search Protocol
@@ -396,7 +396,7 @@
                 "The response errored: %s" % str(error))
         finally:
             action.finish()
-        page_matches = self._parse_bing_response(response.content, start)
+        page_matches = self._parse_search_response(response.content, start)
         return page_matches
 
     def _checkParameter(self, name, value, is_int=False):
@@ -429,7 +429,7 @@
             'Ocp-Apim-Subscription-Key': self.subscription_key,
             }
 
-    def _parse_bing_response(self, bing_json, start=0):
+    def _parse_search_response(self, bing_json, start=0):
         """Return a `PageMatches` object.
 
         :param bing_json: A string containing Bing Custom Search API v7 JSON.

=== modified file 'lib/lp/services/sitesearch/bingtestservice.py'
--- lib/lp/services/sitesearch/bingtestservice.py	2018-03-28 21:28:12 +0000
+++ lib/lp/services/sitesearch/bingtestservice.py	2018-03-30 23:12:56 +0000
@@ -8,6 +8,19 @@
 when given certain user-configurable URLs.
 """
 
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'BingRequestHandler',
+    'get_service_endpoint',
+    'kill_running_process',
+    'service_is_available',
+    'start_as_process',
+    'wait_for_service',
+    ]
+
+
 import logging
 import os
 
@@ -27,7 +40,7 @@
 
 
 class BingRequestHandler(testservice.RequestHandler):
-    default_content_type = 'text/xml; charset=UTF-8'
+    default_content_type = 'application/json; charset=UTF-8'
     log = log
     mapfile = config.bing_test_service.mapfile
     content_dir = config.bing_test_service.canned_response_directory

=== modified file 'lib/lp/services/sitesearch/doc/bing-searchservice.txt'
--- lib/lp/services/sitesearch/doc/bing-searchservice.txt	2018-03-28 21:28:12 +0000
+++ lib/lp/services/sitesearch/doc/bing-searchservice.txt	2018-03-30 23:12:56 +0000
@@ -72,7 +72,7 @@
 of returned results.
 
 The first search for 'bugs' returned a subset of items in the
-ISearchResult. There are 25 total items, but the results contains the
+ISearchResult. There are 25 total items, but the result contains the
 first 20 matches (because they start at index 0).
 
     >>> first_page_matches.start
@@ -99,17 +99,17 @@
 index. All the items in the collection can be iterated.
 
     >>> second_page_matches[1].url
-    'http://blog.launchpad.dev/general/of-bugs-and-statuses'
+    'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2'
 
     >>> for page_match in second_page_matches:
     ...     page_match.url
-    'https://help.launchpad.net/Bugs'
-    'http://blog.launchpad.dev/general/of-bugs-and-statuses'
-    'http://launchpad.dev/mahara/+milestone/1.8.0'
-    'http://launchpad.dev/mb'
-    'http://launchpad.dev/bugs'
+    'http://bugs.launchpad.dev/ubuntu/hoary/+bug/2'
+    'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2'
+    'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3'
+    'http://bugs.launchpad.dev/bugs/bugtrackers'
+    'http://bugs.launchpad.dev/bugs/bugtrackers/debbugs'
 
-An empty PageMatches is returns if there are no results.
+An empty PageMatches is returned if there are no results.
 
     >>> no_page_matches = bing_search.search(terms='fnord')
     >>> no_page_matches.start
@@ -239,7 +239,7 @@
 Bing Search response parsing
 ============================
 
-The BingSearchService's _parse_bing_response() expects a JSON response to
+The BingSearchService's _parse_search_response() expects a JSON response to
 create the PageMatch and PageMatches objects. An error is raised when
 the JSON document cannot be parsed into objects.
 
@@ -261,7 +261,7 @@
     {...
         "totalEstimatedMatches": "~25"...
 
-    >>> bing_search._parse_bing_response(data)
+    >>> bing_search._parse_search_response(data)
     Traceback (most recent call last):
      ...
     SiteSearchResponseError: Could not get the total from the
@@ -278,7 +278,7 @@
     {...
         "totalEstimatedMatches": -25...
 
-    >>> bing_search._parse_bing_response(data).total
+    >>> bing_search._parse_search_response(data).total
     0
 
 A PageMatch requires a title, url, and a summary. If those elements cannot
@@ -290,7 +290,7 @@
     ...     base_path, 'bingsearchservice-missing-title.json')
     >>> with open(json_file_name, 'r') as json_file:
     ...     data = json_file.read()
-    >>> page_matches = bing_search._parse_bing_response(data)
+    >>> page_matches = bing_search._parse_search_response(data)
     >>> len(page_matches)
     1
     >>> page_matches[0].title
@@ -308,7 +308,7 @@
     ...     base_path, 'bingsearchservice-missing-summary.json')
     >>> with open(json_file_name, 'r') as json_file:
     ...     data = json_file.read()
-    >>> page_matches = bing_search._parse_bing_response(data)
+    >>> page_matches = bing_search._parse_search_response(data)
     >>> len(page_matches)
     1
     >>> page_matches[0].title
@@ -324,7 +324,7 @@
     ...     base_path, 'bingsearchservice-missing-url.json')
     >>> with open(json_file_name, 'r') as json_file:
     ...     data = json_file.read()
-    >>> page_matches = bing_search._parse_bing_response(data)
+    >>> page_matches = bing_search._parse_search_response(data)
     >>> len(page_matches)
     1
     >>> page_matches[0].title
@@ -344,7 +344,7 @@
     ...     base_path, 'bingsearchservice-no-meaningful-results.json')
     >>> with open(json_file_name, 'r') as json_file:
     ...     data = json_file.read()
-    >>> page_matches = bing_search._parse_bing_response(data)
+    >>> page_matches = bing_search._parse_search_response(data)
     >>> len(page_matches)
     0
 
@@ -388,10 +388,10 @@
     'http://launchpad.dev/ubuntu'
 
 There is a list of URLs that are not rewritten configured in
-config.bing.url_rewrite_exceptions. For example, help.launchpad.net
+config.sitesearch.url_rewrite_exceptions. For example, help.launchpad.net
 is only run in one environment, so links to that site will be preserved.
 
-    >>> config.bing.url_rewrite_exceptions
+    >>> config.sitesearch.url_rewrite_exceptions
     'help.launchpad.net'
     >>> page_match.url_rewrite_exceptions
     ['help.launchpad.net']

=== modified file 'lib/lp/services/sitesearch/doc/google-searchservice.txt'
--- lib/lp/services/sitesearch/doc/google-searchservice.txt	2018-03-27 20:41:35 +0000
+++ lib/lp/services/sitesearch/doc/google-searchservice.txt	2018-03-30 23:12:56 +0000
@@ -248,7 +248,7 @@
 Google Search Protocol parsing
 ==============================
 
-The GoogleSearchService's _parse_google_search_protocol() requires a
+The GoogleSearchService's _parse_search_response() requires a
 subset of the GSP 3.2 markup to create the PageMatch and PageMatches
 objects. An error is raised when the XML document cannot be parsed into
 objects. The probable cause of an error is that Google returned a new
@@ -272,7 +272,7 @@
     <...
     <PARAM name="start" value="" original_value="0"/>...
 
-    >>> google_search._parse_google_search_protocol(gsp_xml)
+    >>> google_search._parse_search_response(gsp_xml)
     Traceback (most recent call last):
      ...
     GoogleWrongGSPVersion: Could not get the 'start' from the
@@ -294,7 +294,7 @@
     <RES SN="1" EN="1">
     <M>~1</M>...
 
-    >>> google_search._parse_google_search_protocol(gsp_xml)
+    >>> google_search._parse_search_response(gsp_xml)
     Traceback (most recent call last):
      ...
     GoogleWrongGSPVersion: Could not get the 'total' from the
@@ -313,7 +313,7 @@
     <RES SN="1" EN="1">
     <M>-1</M>...
 
-    >>> google_search._parse_google_search_protocol(gsp_xml).total
+    >>> google_search._parse_search_response(gsp_xml).total
     0
 
 A PageMatch requires a title, url, and a summary. If those elements
@@ -350,7 +350,7 @@
     <UE>https://bugs.launchpad.net/bugs/205991</UE>
     <T>Bug #205991 in Ubuntu: ... pair Bluetooth Logitech ...</T>...
 
-    >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+    >>> page_matches = google_search._parse_search_response(gsp_xml)
     >>> len(page_matches)
     1
     >>> page_matches[0].title
@@ -394,7 +394,7 @@
     <RK>0</RK>
     <S>Discuss what needs to be done for &lt;b&gt;Gobuntu&lt;/b&gt;...
 
-    >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+    >>> page_matches = google_search._parse_search_response(gsp_xml)
     >>> len(page_matches)
     1
     >>> page_matches[0].title
@@ -433,7 +433,7 @@
     <UE>https://blueprints.launchpad.net/ubuntu/%2Bspec/gobuntu-hardy</UE>
     <T>Blueprint: &lt;b&gt;Gobuntu&lt;/b&gt; 8.04</T>...
 
-    >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+    >>> page_matches = google_search._parse_search_response(gsp_xml)
     >>> len(page_matches)
     1
     >>> page_matches[0].title
@@ -499,7 +499,7 @@
       <HAS><L/><RT/></HAS>
     </RESULT>...
 
-    >>> google_search._parse_google_search_protocol(gsp_xml)
+    >>> google_search._parse_search_response(gsp_xml)
     Traceback (most recent call last):
      ...
     GoogleWrongGSPVersion: Could not get any PageMatches from the
@@ -528,7 +528,7 @@
          &lt;br&gt;
          tracker allows collaboration</S>...
 
-    >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+    >>> page_matches = google_search._parse_search_response(gsp_xml)
     >>> page_match = page_matches[0]
     >>> page_match.summary
     u'<b>Bug</b> tracking <b>...</b> Search
@@ -575,11 +575,11 @@
     >>> page_match.url
     'http://launchpad.dev/ubuntu'
 
-There is a list of URLs that are not rewritten configured in
-config.google.url_rewrite_exceptions. For example, help.launchpad.net
+There is a list of URLs that are not rewritten configured via
+config.vhosts.use_https. For example, help.launchpad.net
 is only run in one environment, so links to that site will be preserved.
 
-    >>> config.google.url_rewrite_exceptions
+    >>> config.sitesearch.url_rewrite_exceptions
     'help.launchpad.net'
     >>> page_match.url_rewrite_exceptions
     ['help.launchpad.net']

=== modified file 'lib/lp/services/sitesearch/googletestservice.py' (properties changed: -x to +x)
--- lib/lp/services/sitesearch/googletestservice.py	2018-03-28 15:58:21 +0000
+++ lib/lp/services/sitesearch/googletestservice.py	2018-03-30 23:12:56 +0000
@@ -8,6 +8,19 @@
 when given certain user-configurable URLs.
 """
 
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'GoogleRequestHandler',
+    'get_service_endpoint',
+    'kill_running_process',
+    'service_is_available',
+    'start_as_process',
+    'wait_for_service',
+    ]
+
+
 import logging
 import os
 

=== modified file 'lib/lp/services/sitesearch/tests/bingserviceharness.py'
--- lib/lp/services/sitesearch/tests/bingserviceharness.py	2018-03-16 21:04:45 +0000
+++ lib/lp/services/sitesearch/tests/bingserviceharness.py	2018-03-30 23:12:56 +0000
@@ -1,12 +1,11 @@
 # Copyright 2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""
-Fixtures for running the Bing test webservice.
-"""
+"""Fixtures for running the Bing test webservice."""
+
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
-
 __all__ = ['BingServiceTestSetup']
 
 
@@ -18,55 +17,7 @@
 
 
 class BingServiceTestSetup:
-    """Set up the Bing web service stub for use in functional tests.
-    """
-
-    # XXX gary 2008-12-06 bug=305858: Spurious test failures discovered on
-    # buildbot, builds 40 and 43. The locations of the failures are marked
-    # below with " # SPURIOUS FAILURE". To reinstate, add the text below back
-    # to the docstring above.  Note that the test that uses this setup,
-    # bing-service-stub.txt, is also disabled.  See test_doc.py.
-    """
-    >>> from lp.services.sitesearch.bingtestservice import (
-    ...     service_is_available)
-    >>> from lp.services.config import config
-
-    >>> assert not service_is_available()  # Sanity check. # SPURIOUS FAILURE
-
-    >>> BingServiceTestSetup().setUp()
-
-    After setUp is called, a Bing test service instance is running.
-
-    >>> assert service_is_available()
-    >>> assert BingServiceTestSetup.service is not None
-
-    After tearDown is called, the service is shut down.
-
-    >>> BingServiceTestSetup().tearDown()
-
-    >>> assert not service_is_available()
-    >>> assert BingServiceTestSetup.service is None
-
-    The fixture can be started and stopped multiple time in succession:
-
-    >>> BingServiceTestSetup().setUp()
-    >>> assert service_is_available()
-
-    Having a service instance already running doesn't prevent a new
-    service from starting.  The old instance is killed off and replaced
-    by the new one.
-
-    >>> old_pid = BingServiceTestSetup.service.pid
-    >>> BingServiceTestSetup().setUp() # SPURIOUS FAILURE
-    >>> BingServiceTestSetup.service.pid != old_pid
-    True
-
-    Tidy up.
-
-    >>> BingServiceTestSetup().tearDown()
-    >>> assert not service_is_available()
-
-    """
+    """Set up the Bing web service stub for use in functional tests."""
 
     service = None  # A reference to our running service.
 

=== modified file 'lib/lp/services/sitesearch/tests/data/bingsearchservice-bugs-2.json'
--- lib/lp/services/sitesearch/tests/data/bingsearchservice-bugs-2.json	2018-03-27 20:45:52 +0000
+++ lib/lp/services/sitesearch/tests/data/bingsearchservice-bugs-2.json	2018-03-30 23:12:56 +0000
@@ -15,7 +15,7 @@
       {
         "id": "https://api.cognitive.microsoft.com/api/v7/#WebPages.0";,
         "name": "Bugs - Launchpad Help",
-        "url": "https://help.launchpad.net/Bugs";,
+        "url": "http://bugs.launchpad.dev/ubuntu/hoary/+bug/2";,
         "urlPingSuffix": "DevEx,5103.1",
         "isFamilyFriendly": true,
         "displayUrl": "https://help.launchpad.net/Bugs";,
@@ -58,7 +58,7 @@
       {
         "id": "https://api.cognitive.microsoft.com/api/v7/#WebPages.1";,
         "name": "Of Bugs and Statuses - Launchpad Blog",
-        "url": "https://blog.launchpad.net/general/of-bugs-and-statuses";,
+        "url": "http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2";,
         "urlPingSuffix": "DevEx,5149.1",
         "isFamilyFriendly": true,
         "displayUrl": "https://blog.launchpad.net/general/of-bugs-and-statuses";,
@@ -69,7 +69,7 @@
       {
         "id": "https://api.cognitive.microsoft.com/api/v7/#WebPages.2";,
         "name": "Mahara 1.8.0",
-        "url": "https://launchpad.net/mahara/+milestone/1.8.0";,
+        "url": "http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3";,
         "urlPingSuffix": "DevEx,5178.1",
         "isFamilyFriendly": true,
         "displayUrl": "https://launchpad.net/mahara/+milestone/1.8.0";,
@@ -80,7 +80,7 @@
       {
         "id": "https://api.cognitive.microsoft.com/api/v7/#WebPages.3";,
         "name": "Mighty Box in Launchpad",
-        "url": "https://launchpad.net/mb";,
+        "url": "http://bugs.launchpad.dev/bugs/bugtrackers";,
         "urlPingSuffix": "DevEx,5193.1",
         "isFamilyFriendly": true,
         "displayUrl": "https://launchpad.net/mb";,
@@ -91,7 +91,7 @@
       {
         "id": "https://api.cognitive.microsoft.com/api/v7/#WebPages.4";,
         "name": "Bug tracking - Launchpad Bugs",
-        "url": "https://launchpad.net/bugs";,
+        "url": "http://bugs.launchpad.dev/bugs/bugtrackers/debbugs";,
         "urlPingSuffix": "DevEx,5208.1",
         "about": [
           {

=== modified file 'lib/lp/services/sitesearch/tests/data/bingsearchservice-no-meaningful-results.json'
--- lib/lp/services/sitesearch/tests/data/bingsearchservice-no-meaningful-results.json	2018-03-27 20:45:52 +0000
+++ lib/lp/services/sitesearch/tests/data/bingsearchservice-no-meaningful-results.json	2018-03-30 23:12:56 +0000
@@ -1,7 +1,7 @@
 {
   "_type": "SearchResponse",
   "webPages": {
-    "totalEstimatedMatches": 25,
+    "totalEstimatedMatches": 1,
     "value": [
       {
         "id": "https://api.cognitive.microsoft.com/api/v7/#WebPages.2";,

=== added file 'lib/lp/services/sitesearch/tests/data/googlesearchservice-error.xml'
--- lib/lp/services/sitesearch/tests/data/googlesearchservice-error.xml	1970-01-01 00:00:00 +0000
+++ lib/lp/services/sitesearch/tests/data/googlesearchservice-error.xml	2018-03-30 23:12:56 +0000
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<GSP VER="3.3">
+</GSP>
+

=== modified file 'lib/lp/services/sitesearch/tests/data/googlesearchservice-mapping.txt'
--- lib/lp/services/sitesearch/tests/data/googlesearchservice-mapping.txt	2018-03-16 14:02:16 +0000
+++ lib/lp/services/sitesearch/tests/data/googlesearchservice-mapping.txt	2018-03-30 23:12:56 +0000
@@ -20,5 +20,7 @@
 
 /cse?client=google-csbe&cx=ABCDEF2323&ie=utf8&num=20&oe=utf8&output=xml_no_dtd&q=no-meaningful&start=0 googlesearchservice-no-meaningful-results.xml
 
+/cse?client=google-csbe&cx=ABCDEF2323&ie=utf8&num=20&oe=utf8&output=xml_no_dtd&q=errors-please&start=0 googlesearchservice-error.xml
+
 # This stub service is also used to impersonate the Blog feed
 /blog-feed blog.launchpad.net-feed.xml

=== modified file 'lib/lp/services/sitesearch/tests/googleserviceharness.py'
--- lib/lp/services/sitesearch/tests/googleserviceharness.py	2018-03-28 17:00:10 +0000
+++ lib/lp/services/sitesearch/tests/googleserviceharness.py	2018-03-30 23:12:56 +0000
@@ -1,12 +1,11 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""
-Fixtures for running the Google test webservice.
-"""
+"""Fixtures for running the Google test webservice."""
+
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
-
 __all__ = ['GoogleServiceTestSetup']
 
 
@@ -18,55 +17,7 @@
 
 
 class GoogleServiceTestSetup:
-    """Set up the Google web service stub for use in functional tests.
-    """
-
-    # XXX gary 2008-12-06 bug=305858: Spurious test failures discovered on
-    # buildbot, builds 40 and 43. The locations of the failures are marked
-    # below with " # SPURIOUS FAILURE". To reinstate, add the text below back
-    # to the docstring above.  Note that the test that uses this setup,
-    # google-service-stub.txt, is also disabled.  See test_doc.py.
-    """
-    >>> from lp.services.sitesearch.googletestservice import (
-    ...     service_is_available)
-    >>> from lp.services.config import config
-
-    >>> assert not service_is_available()  # Sanity check. # SPURIOUS FAILURE
-
-    >>> GoogleServiceTestSetup().setUp()
-
-    After setUp is called, a Google test service instance is running.
-
-    >>> assert service_is_available()
-    >>> assert GoogleServiceTestSetup.service is not None
-
-    After tearDown is called, the service is shut down.
-
-    >>> GoogleServiceTestSetup().tearDown()
-
-    >>> assert not service_is_available()
-    >>> assert GoogleServiceTestSetup.service is None
-
-    The fixture can be started and stopped multiple time in succession:
-
-    >>> GoogleServiceTestSetup().setUp()
-    >>> assert service_is_available()
-
-    Having a service instance already running doesn't prevent a new
-    service from starting.  The old instance is killed off and replaced
-    by the new one.
-
-    >>> old_pid = GoogleServiceTestSetup.service.pid
-    >>> GoogleServiceTestSetup().setUp() # SPURIOUS FAILURE
-    >>> GoogleServiceTestSetup.service.pid != old_pid
-    True
-
-    Tidy up.
-
-    >>> GoogleServiceTestSetup().tearDown()
-    >>> assert not service_is_available()
-
-    """
+    """Set up the Google web service stub for use in functional tests."""
 
     service = None  # A reference to our running service.
 

=== modified file 'lib/lp/services/sitesearch/tests/test_bing.py'
--- lib/lp/services/sitesearch/tests/test_bing.py	2018-03-28 14:43:29 +0000
+++ lib/lp/services/sitesearch/tests/test_bing.py	2018-03-30 23:12:56 +0000
@@ -1,35 +1,199 @@
-# Copyright 2011-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the bing search service."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
 
+import json
+from os import path
+
 from fixtures import MockPatch
 from requests.exceptions import (
     ConnectionError,
     HTTPError,
     )
+from testtools.matchers import HasLength
 
+from lp.services.config import config
 from lp.services.sitesearch import BingSearchService
 from lp.services.sitesearch.interfaces import SiteSearchResponseError
 from lp.services.timeout import TimeoutError
 from lp.testing import TestCase
-from lp.testing.layers import (
-    BingLaunchpadFunctionalLayer,
-    LaunchpadFunctionalLayer,
-    )
+from lp.testing.layers import BingLaunchpadFunctionalLayer
 
 
 class TestBingSearchService(TestCase):
     """Test BingSearchService."""
 
-    layer = LaunchpadFunctionalLayer
-
     def setUp(self):
         super(TestBingSearchService, self).setUp()
         self.search_service = BingSearchService()
+        self.base_path = path.normpath(
+            path.join(path.dirname(__file__), 'data'))
+
+    def test_configuration(self):
+        self.assertEqual(config.bing.site, self.search_service.site)
+        self.assertEqual(
+            config.bing.subscription_key, self.search_service.subscription_key)
+        self.assertEqual(
+            config.bing.custom_config_id, self.search_service.custom_config_id)
+
+    def test_create_search_url(self):
+        self.assertEndsWith(
+            self.search_service.create_search_url(terms='svg +bugs'),
+            '&offset=0&q=svg+%2Bbugs')
+
+    def test_create_search_url_escapes_unicode_chars(self):
+        self.assertEndsWith(
+            self.search_service.create_search_url('Carlo Perell\xf3 Mar\xedn'),
+            '&offset=0&q=Carlo+Perell%C3%B3+Mar%C3%ADn')
+
+    def test_create_search_url_with_offset(self):
+        self.assertEndsWith(
+            self.search_service.create_search_url(terms='svg +bugs', start=20),
+            '&offset=20&q=svg+%2Bbugs')
+
+    def test_create_search_url_empty_terms(self):
+        e = self.assertRaises(
+            ValueError, self.search_service.create_search_url, '')
+        self.assertEqual("Missing value for parameter 'q'.", str(e))
+
+    def test_create_search_url_null_terms(self):
+        e = self.assertRaises(
+            ValueError, self.search_service.create_search_url, None)
+        self.assertEqual("Missing value for parameter 'q'.", str(e))
+
+    def test_create_search_url_requires_start(self):
+        e = self.assertRaises(
+            ValueError, self.search_service.create_search_url, 'bugs', 'true')
+        self.assertEqual("Value for parameter 'offset' is not an int.", str(e))
+
+    def test_parse_search_response_invalid_total(self):
+        """The PageMatches's total attribute comes from the
+        `webPages.totalEstimatedMatches` JSON element.
+        When it cannot be found and the value cast to an int,
+        an error is raised. If Bing were to redefine the meaning of the
+        element to use a '~' to indicate an approximate total, an error would
+        be raised.
+        """
+        file_name = path.join(
+            self.base_path, 'bingsearchservice-incompatible-matches.json')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert (
+            json.loads(response)['webPages']['totalEstimatedMatches'] == '~25')
+
+        e = self.assertRaises(
+            SiteSearchResponseError,
+            self.search_service._parse_search_response, response)
+        self.assertEqual(
+            "Could not get the total from the Bing JSON response.", str(e))
+
+    def test_parse_search_response_negative_total(self):
+        """If the total is ever less than zero (see bug 683115),
+        this is expected: we simply return a total of 0.
+        """
+        file_name = path.join(
+            self.base_path, 'bingsearchservice-negative-total.json')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert json.loads(response)['webPages']['totalEstimatedMatches'] == -25
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertEqual(0, matches.total)
+
+    def test_parse_search_response_missing_title(self):
+        """A PageMatch requires a title, url, and a summary. If those elements
+        cannot be found, a PageMatch cannot be made. A missing title ('name')
+        indicates a bad page on Launchpad, so it is ignored. In this example,
+        the first match is missing a title, so only the second page is present
+        in the PageMatches.
+        """
+        file_name = path.join(
+            self.base_path, 'bingsearchservice-missing-title.json')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert len(json.loads(response)['webPages']['value']) == 2
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(1))
+        self.assertEqual('GCleaner in Launchpad', matches[0].title)
+        self.assertEqual('http://launchpad.dev/gcleaner', matches[0].url)
+
+    def test_parse_search_response_missing_summary(self):
+        """When a match is missing a summary ('snippet'), the match is skipped
+        because there is no information about why it matched. This appears to
+        relate to pages that are in the index, but should be removed. In this
+        example taken from real data, the links are to the same page on
+        different vhosts. The edge vhost has no summary, so it is skipped.
+        """
+        file_name = path.join(
+            self.base_path, 'bingsearchservice-missing-summary.json')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert len(json.loads(response)['webPages']['value']) == 2
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(1))
+        self.assertEqual('BugExpiry - Launchpad Help', matches[0].title)
+        self.assertEqual(
+            'https://help.launchpad.net/BugExpiry', matches[0].url)
+
+    def test_parse_search_response_missing_url(self):
+        """When the URL ('url') cannot be found the match is skipped. There are
+        no examples of this. We do not want this hypothetical situation to give
+        users a bad experience.
+        """
+        file_name = path.join(
+            self.base_path, 'bingsearchservice-missing-url.json')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert len(json.loads(response)['webPages']['value']) == 2
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(1))
+        self.assertEqual('LongoMatch in Launchpad', matches[0].title)
+        self.assertEqual('http://launchpad.dev/longomatch', matches[0].url)
+
+    def test_parse_search_response_with_no_meaningful_results(self):
+        """If no matches are found in the response, and there are 20 or fewer
+        results, an Empty PageMatches is returned. This happens when the
+        results are missing titles and summaries. This is not considered to be
+        a problem because the small number implies that Bing did a poor job
+        of indexing pages or indexed the wrong Launchpad server. In this
+        example, there is only one match, but the results is missing a title so
+        there is not enough information to make a PageMatch.
+        """
+        file_name = path.join(
+            self.base_path, 'bingsearchservice-no-meaningful-results.json')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert len(json.loads(response)['webPages']['value']) == 1
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(0))
+
+    def test_parse_search_response_TypeError(self):
+        # The method converts TypeError to SiteSearchResponseError.
+        self.assertRaises(
+            SiteSearchResponseError,
+            self.search_service._parse_search_response, None)
+
+    def test_parse_search_response_ValueError(self):
+        # The method converts ValueError to SiteSearchResponseError.
+        self.assertRaises(
+            SiteSearchResponseError,
+            self.search_service._parse_search_response, '')
+
+    def test_parse_search_response_KeyError(self):
+        # The method converts KeyError to SiteSearchResponseError.
+        self.assertRaises(
+            SiteSearchResponseError,
+            self.search_service._parse_search_response, '{}')
 
     def test_search_converts_HTTPError(self):
         # The method converts HTTPError to SiteSearchResponseError.
@@ -55,32 +219,14 @@
         self.assertRaises(
             SiteSearchResponseError, self.search_service.search, 'fnord')
 
-    def test_parse_bing_response_TypeError(self):
-        # The method converts TypeError to SiteSearchResponseError.
-        self.assertRaises(
-            SiteSearchResponseError,
-            self.search_service._parse_bing_response, None)
-
-    def test_parse_bing_response_ValueError(self):
-        # The method converts ValueError to SiteSearchResponseError.
-        self.assertRaises(
-            SiteSearchResponseError,
-            self.search_service._parse_bing_response, '')
-
-    def test_parse_bing_response_KeyError(self):
-        # The method converts KeyError to SiteSearchResponseError.
-        self.assertRaises(
-            SiteSearchResponseError,
-            self.search_service._parse_bing_response, '{}')
-
-
-class FunctionalTestBingSearchService(TestCase):
+
+class FunctionalBingSearchServiceTests(TestCase):
     """Test BingSearchService."""
 
     layer = BingLaunchpadFunctionalLayer
 
     def setUp(self):
-        super(FunctionalTestBingSearchService, self).setUp()
+        super(FunctionalBingSearchServiceTests, self).setUp()
         self.search_service = BingSearchService()
 
     def test_search_with_results(self):
@@ -94,6 +240,13 @@
         self.assertEqual(20, matches.start)
         self.assertEqual(25, matches.total)
         self.assertEqual(5, len(matches))
+        self.assertEqual([
+            'http://bugs.launchpad.dev/ubuntu/hoary/+bug/2',
+            'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2',
+            'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3',
+            'http://bugs.launchpad.dev/bugs/bugtrackers',
+            'http://bugs.launchpad.dev/bugs/bugtrackers/debbugs'],
+            [match.url for match in matches])
 
     def test_search_no_results(self):
         matches = self.search_service.search('fnord')
@@ -104,7 +257,7 @@
     def test_search_no_meaningful_results(self):
         matches = self.search_service.search('no-meaningful')
         self.assertEqual(0, matches.start)
-        self.assertEqual(25, matches.total)
+        self.assertEqual(1, matches.total)
         self.assertEqual(0, len(matches))
 
     def test_search_incomplete_response(self):

=== removed file 'lib/lp/services/sitesearch/tests/test_bingharness.py'
--- lib/lp/services/sitesearch/tests/test_bingharness.py	2018-03-16 21:04:45 +0000
+++ lib/lp/services/sitesearch/tests/test_bingharness.py	1970-01-01 00:00:00 +0000
@@ -1,10 +0,0 @@
-# Copyright 2018 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-import doctest
-
-
-def test_suite():
-    return doctest.DocTestSuite(
-            'lp.services.sitesearch.tests.bingserviceharness',
-            optionflags=doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS)

=== removed file 'lib/lp/services/sitesearch/tests/test_bingservice.py'
--- lib/lp/services/sitesearch/tests/test_bingservice.py	2018-03-26 20:05:20 +0000
+++ lib/lp/services/sitesearch/tests/test_bingservice.py	1970-01-01 00:00:00 +0000
@@ -1,38 +0,0 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""
-Unit tests for the Bing test service stub.
-"""
-
-__metaclass__ = type
-
-
-import os
-import unittest
-
-from lp.services.osutils import process_exists
-from lp.services.pidfile import pidfile_path
-from lp.services.sitesearch import bingtestservice
-
-
-class TestServiceUtilities(unittest.TestCase):
-    """Test the service's supporting functions."""
-
-    def test_stale_pid_file_cleanup(self):
-        """The service should be able to clean up invalid PID files."""
-        bogus_pid = 9999999
-        self.assertFalse(
-            process_exists(bogus_pid),
-            "There is already a process with PID '%d'." % bogus_pid)
-
-        # Create a stale/bogus PID file.
-        filepath = pidfile_path(bingtestservice.service_name)
-        with file(filepath, 'w') as pidfile:
-            pidfile.write(str(bogus_pid))
-
-        # The PID clean-up code should silently remove the file and return.
-        bingtestservice.kill_running_process()
-        self.assertFalse(
-            os.path.exists(filepath),
-            "The PID file '%s' should have been deleted." % filepath)

=== modified file 'lib/lp/services/sitesearch/tests/test_google.py'
--- lib/lp/services/sitesearch/tests/test_google.py	2018-03-28 19:31:02 +0000
+++ lib/lp/services/sitesearch/tests/test_google.py	2018-03-30 23:12:56 +0000
@@ -3,30 +3,215 @@
 
 """Test the google search service."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
 
+from os import path
+
 from fixtures import MockPatch
 from requests.exceptions import (
     ConnectionError,
     HTTPError,
     )
+from testtools.matchers import HasLength
 
+from lp.services.config import config
 from lp.services.sitesearch import GoogleSearchService
-from lp.services.sitesearch.interfaces import SiteSearchResponseError
+from lp.services.sitesearch.interfaces import (
+    GoogleWrongGSPVersion,
+    SiteSearchResponseError,
+    )
 from lp.services.timeout import TimeoutError
 from lp.testing import TestCase
-from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.layers import GoogleLaunchpadFunctionalLayer
 
 
 class TestGoogleSearchService(TestCase):
     """Test GoogleSearchService."""
 
-    layer = LaunchpadFunctionalLayer
-
     def setUp(self):
         super(TestGoogleSearchService, self).setUp()
         self.search_service = GoogleSearchService()
+        self.base_path = path.normpath(
+            path.join(path.dirname(__file__), 'data'))
+
+    def test_configuration(self):
+        self.assertEqual(config.google.site, self.search_service.site)
+        self.assertEqual(
+            config.google.client_id, self.search_service.client_id)
+
+    def test_create_search_url(self):
+        self.assertEndsWith(
+            self.search_service.create_search_url(terms='svg +bugs'),
+            '&q=svg+%2Bbugs&start=0')
+
+    def test_create_search_url_escapes_unicode_chars(self):
+        self.assertEndsWith(
+            self.search_service.create_search_url('Carlo Perell\xf3 Mar\xedn'),
+            '&q=Carlo+Perell%C3%B3+Mar%C3%ADn&start=0')
+
+    def test_create_search_url_with_offset(self):
+        self.assertEndsWith(
+            self.search_service.create_search_url(terms='svg +bugs', start=20),
+            '&q=svg+%2Bbugs&start=20')
+
+    def test_create_search_url_empty_terms(self):
+        e = self.assertRaises(
+            ValueError, self.search_service.create_search_url, '')
+        self.assertEqual("Missing value for parameter 'q'.", str(e))
+
+    def test_create_search_url_null_terms(self):
+        e = self.assertRaises(
+            ValueError, self.search_service.create_search_url, None)
+        self.assertEqual("Missing value for parameter 'q'.", str(e))
+
+    def test_create_search_url_requires_start(self):
+        e = self.assertRaises(
+            ValueError, self.search_service.create_search_url, 'bugs', 'true')
+        self.assertEqual("Value for parameter 'start' is not an int.", str(e))
+
+    def test_parse_search_response_incompatible_param(self):
+        """The PageMatches's start attribute comes from the GSP XML element
+        '<PARAM name="start" value="0" original_value="0"/>'. When it cannot
+        be found and the value cast to an int, an error is raised. There is
+        nothing in the value attribute in the next test, so an error is raised.
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-incompatible-param.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert '<M>' not in response
+
+        e = self.assertRaises(
+            GoogleWrongGSPVersion,
+            self.search_service._parse_search_response, response)
+        self.assertEqual(
+            "Could not get the 'start' from the GSP XML response.", str(e))
+
+    def test_parse_search_response_invalid_total(self):
+        """The PageMatches's total attribute comes from the GSP XML element
+        '<M>5</M>'. When it cannot be found and the value cast to an int,
+        an error is raised. If Google were to redefine the meaning of the M
+        element to use a '~' to indicate an approximate total, an error would
+        be raised.
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-incompatible-matches.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert '<M>~1</M>' in response
+
+        e = self.assertRaises(
+            GoogleWrongGSPVersion,
+            self.search_service._parse_search_response, response)
+        self.assertEqual(
+            "Could not get the 'total' from the GSP XML response.", str(e))
+
+    def test_parse_search_response_negative_total(self):
+        """If the total is ever less than zero (see bug 683115),
+        this is expected: we simply return a total of 0.
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-negative-total.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert '<M>-1</M>' in response
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertEqual(0, matches.total)
+
+    def test_parse_search_response_missing_title(self):
+        """A PageMatch requires a title, url, and a summary. If those elements
+        ('<T>', '<U>', '<S>') cannot be found nested in an '<R>' a PageMatch
+        cannot be made. A missing title (<T>) indicates a bad page on Launchpad
+        so it is ignored. In this example, The first match is missing a title,
+        so only the second page is present in the PageMatches.
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-missing-title.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(1))
+        self.assertStartsWith(matches[0].title, 'Bug #205991 in Ubuntu:')
+        self.assertEqual(
+            'http://bugs.launchpad.dev/bugs/205991', matches[0].url)
+
+    def test_parse_search_response_missing_summary(self):
+        """When a match is missing a summary (<S>), it is skipped because
+        there is no information about why it matched. This appears to relate to
+        pages that are in the index, but should be removed. In this example
+        taken from real data, the links are to the same page on different
+        vhosts. The edge vhost has no summary, so it is skipped.
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-missing-summary.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(1))
+        self.assertEqual('Blueprint: <b>Gobuntu</b> 8.04', matches[0].title)
+        self.assertEqual(
+            'http://blueprints.launchpad.dev/ubuntu/+spec/gobuntu-hardy',
+            matches[0].url)
+
+    def test_parse_search_response_missing_url(self):
+        """When the URL (<U>) cannot be found the match is skipped. There are
+        no examples of this. We do not want this hypothetical situation to give
+        users a bad experience.
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-missing-url.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(1))
+        self.assertEqual('Blueprint: <b>Gobuntu</b> 8.04', matches[0].title)
+        self.assertEqual(
+            'http://blueprints.launchpad.dev/ubuntu/+spec/gobuntu-hardy',
+            matches[0].url)
+
+    def test_parse_search_response_with_no_meaningful_results(self):
+        """If no matches are found in the response, and there are 20 or fewer
+        results, an Empty PageMatches is returned. This happens when the
+        results are missing titles and summaries. This is not considered to be
+        a problem because the small number implies that Google did a poor job
+        of indexing pages or indexed the wrong Launchpad server. In this
+        example, there is only one match, but the results is missing a title so
+        there is not enough information to make a PageMatch.
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-no-meaningful-results.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert '<M>1</M>' in response
+
+        matches = self.search_service._parse_search_response(response)
+        self.assertThat(matches, HasLength(0))
+
+    def test_parse_search_response_with_incompatible_result(self):
+        """If no matches are found in the response, and there are more than 20
+        possible matches, an error is raised. Unlike the previous example there
+        are lots of results; there is a possibility that the GSP version is
+        incompatible. This example says it has 1000 matches, but none of the R
+        tags can be parsed (because the markup was changed to use RESULT).
+        """
+        file_name = path.join(
+            self.base_path, 'googlesearchservice-incompatible-result.xml')
+        with open(file_name, 'r') as response_file:
+            response = response_file.read()
+        assert '<M>1000</M>' in response
+
+        e = self.assertRaises(
+            GoogleWrongGSPVersion,
+            self.search_service._parse_search_response, response)
+        self.assertEqual(
+            "Could not get any PageMatches from the GSP XML response.", str(e))
 
     def test_search_converts_HTTPError(self):
         # The method converts HTTPError to SiteSearchResponseError.
@@ -52,22 +237,73 @@
         self.assertRaises(
             SiteSearchResponseError, self.search_service.search, 'fnord')
 
-    def test___parse_google_search_protocol_SyntaxError(self):
+    def test_parse_search_response_SyntaxError(self):
         # The method converts SyntaxError to SiteSearchResponseError.
         self.useFixture(MockPatch(
             'lp.services.sitesearch.urlfetch',
             side_effect=SyntaxError('oops')))
         self.assertRaises(
             SiteSearchResponseError,
-            self.search_service._parse_google_search_protocol, '')
+            self.search_service._parse_search_response, '')
 
-    def test___parse_google_search_protocol_IndexError(self):
+    def test_parse_search_response_IndexError(self):
         # The method converts IndexError to SiteSearchResponseError.
         self.useFixture(MockPatch(
             'lp.services.sitesearch.urlfetch', side_effect=IndexError('oops')))
-        data = (
+        response = (
             '<?xml version="1.0" encoding="UTF-8" standalone="no"?>'
             '<GSP VER="3.2"></GSP>')
         self.assertRaises(
             SiteSearchResponseError,
-            self.search_service._parse_google_search_protocol, data)
+            self.search_service._parse_search_response, response)
+
+
+class FunctionalGoogleSearchServiceTests(TestCase):
+    """Test GoogleSearchService."""
+
+    layer = GoogleLaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(FunctionalGoogleSearchServiceTests, self).setUp()
+        self.search_service = GoogleSearchService()
+
+    def test_search_with_results(self):
+        matches = self.search_service.search('bug')
+        self.assertEqual(0, matches.start)
+        self.assertEqual(25, matches.total)
+        self.assertEqual(20, len(matches))
+
+    def test_search_with_results_and_offset(self):
+        matches = self.search_service.search('bug', start=20)
+        self.assertEqual(20, matches.start)
+        self.assertEqual(25, matches.total)
+        self.assertEqual(5, len(matches))
+        self.assertEqual([
+            'http://bugs.launchpad.dev/ubuntu/hoary/+bug/2',
+            'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2',
+            'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3',
+            'http://bugs.launchpad.dev/bugs/bugtrackers',
+            'http://bugs.launchpad.dev/bugs/bugtrackers/debbugs'],
+            [match.url for match in matches])
+
+    def test_search_no_results(self):
+        matches = self.search_service.search('fnord')
+        self.assertEqual(0, matches.start)
+        self.assertEqual(0, matches.total)
+        self.assertEqual(0, len(matches))
+
+    def test_search_no_meaningful_results(self):
+        matches = self.search_service.search('no-meaningful')
+        self.assertEqual(0, matches.start)
+        self.assertEqual(1, matches.total)
+        self.assertEqual(0, len(matches))
+
+    def test_search_incomplete_response(self):
+        self.assertRaises(
+            SiteSearchResponseError,
+            self.search_service.search, 'gnomebaker')
+
+    def test_search_error_response(self):
+        self.assertRaises(
+            SiteSearchResponseError,
+            self.search_service.search, 'errors-please')

=== modified file 'lib/lp/services/sitesearch/tests/test_pagematch.py'
--- lib/lp/services/sitesearch/tests/test_pagematch.py	2018-03-16 14:02:16 +0000
+++ lib/lp/services/sitesearch/tests/test_pagematch.py	2018-03-30 23:12:56 +0000
@@ -5,14 +5,59 @@
 
 __metaclass__ = type
 
-from lp.services.sitesearch import PageMatch
-from lp.testing import TestCaseWithFactory
-from lp.testing.layers import DatabaseFunctionalLayer
-
-
-class TestPageMatchURLHandling(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
+from lp.services.sitesearch import (
+    PageMatch,
+    PageMatches,
+    )
+from lp.testing import TestCase
+
+
+class TestPageMatchURLHandling(TestCase):
+
+    def test_attributes(self):
+        p = PageMatch(
+            u'Unicode Titles in Launchpad',
+            'http://example.com/unicode-titles',
+            u'Unicode Titles is a modest project dedicated to using Unicode.')
+        self.assertEqual(u'Unicode Titles in Launchpad', p.title)
+        self.assertEqual(
+            u'Unicode Titles is a modest project dedicated to using Unicode.',
+            p.summary)
+        self.assertEqual('http://example.com/unicode-titles', p.url)
+
+    def test_rewrite_url(self):
+        """The URL scheme used in the rewritten URL is configured via
+        config.vhosts.use_https. The hostname is set in the shared
+        key config.vhost.mainsite.hostname.
+        """
+        p = PageMatch(
+            u'Bug #456 in Unicode title: "testrunner hates Unicode"',
+            'https://bugs.launchpad.net/unicode-titles/+bug/456',
+            u'The Zope testrunner likes ASCII more than Unicode.')
+        self.assertEqual(
+            'http://bugs.launchpad.dev/unicode-titles/+bug/456', p.url)
+
+    def test_rewrite_url_with_trailing_slash(self):
+        """A URL's trailing slash is removed; Launchpad does not use trailing
+        slashes.
+        """
+        p = PageMatch(
+            u'Ubuntu in Launchpad',
+            'https://launchpad.net/ubuntu/',
+            u'Ubuntu also includes more software than any other operating')
+        self.assertEqual('http://launchpad.dev/ubuntu', p.url)
+
+    def test_rewrite_url_exceptions(self):
+        """There is a list of URLs that are not rewritten configured in
+        config.sitesearch.url_rewrite_exceptions. For example,
+        help.launchpad.net is only run in one environment, so links to
+        that site will be preserved.
+        """
+        p = PageMatch(
+            u'OpenID',
+            'https://help.launchpad.net/OpenID',
+            u'Launchpad uses OpenID.')
+        self.assertEqual('https://help.launchpad.net/OpenID', p.url)
 
     def test_rewrite_url_handles_invalid_data(self):
         # Given a bad url, pagematch can get a valid one.
@@ -37,3 +82,24 @@
             "field.text=WUSB54GC+%2Bkarmic&"
             "field.actions.search=Search")
         self.assertEqual(expected, p.url)
+
+
+class TestPageMatches(TestCase):
+
+    def test_initialisation(self):
+        matches = PageMatches([], start=12, total=15)
+        self.assertEqual(12, matches.start)
+        self.assertEqual(15, matches.total)
+
+    def test_len(self):
+        matches = PageMatches(['match1', 'match2', 'match3'], 12, 15)
+        self.assertEqual(3, len(matches))
+
+    def test_getitem(self):
+        matches = PageMatches(['match1', 'match2', 'match3'], 12, 15)
+        self.assertEqual('match2', matches[1])
+
+    def test_iter(self):
+        matches = PageMatches(['match1', 'match2', 'match3'], 12, 15)
+        self.assertEqual(
+            ['match1', 'match2', 'match3'], [match for match in matches])

=== renamed file 'lib/lp/services/sitesearch/tests/test_googleharness.py' => 'lib/lp/services/sitesearch/tests/test_serviceharness.py'
--- lib/lp/services/sitesearch/tests/test_googleharness.py	2018-03-16 14:02:16 +0000
+++ lib/lp/services/sitesearch/tests/test_serviceharness.py	2018-03-30 23:12:56 +0000
@@ -1,10 +1,73 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-import doctest
-
-
-def test_suite():
-    return doctest.DocTestSuite(
-            'lp.services.sitesearch.tests.googleserviceharness',
-            optionflags=doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS)
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+
+import time
+
+from testscenarios import WithScenarios
+
+from lp.services.sitesearch.bingtestservice import (
+    service_is_available as bing_service_is_available,
+    )
+from lp.services.sitesearch.googletestservice import (
+    service_is_available as google_service_is_available,
+    )
+from lp.services.sitesearch.tests.bingserviceharness import (
+    BingServiceTestSetup,
+    )
+from lp.services.sitesearch.tests.googleserviceharness import (
+    GoogleServiceTestSetup,
+    )
+from lp.testing import TestCase
+
+
+class TestServiceTestSetup(WithScenarios, TestCase):
+
+    scenarios = [
+        ("Bing", {
+            "ServiceTestSetup": BingServiceTestSetup,
+            "service_is_available": bing_service_is_available,
+            }),
+        ("Google", {
+            "ServiceTestSetup": GoogleServiceTestSetup,
+            "service_is_available": google_service_is_available,
+            }),
+        ]
+
+    def test_restart(self):
+        """The fixture can be started/stopped multiple time in succession."""
+        assert not self.service_is_available()  # Sanity check.
+
+        for i in range(2):
+            self.ServiceTestSetup().setUp()
+            self.assertTrue(self.service_is_available())
+            self.assertIsNotNone(self.ServiceTestSetup.service)
+
+            time.sleep(1)  # Give it time to settle down.
+            self.ServiceTestSetup().tearDown()
+            self.assertFalse(self.service_is_available())
+            self.assertIsNone(self.ServiceTestSetup.service)
+
+    def test_multiple_instances_not_allowed(self):
+        """Having a service instance already running doesn't prevent a new
+        service from starting.  The old instance is killed off and replaced
+        by the new one.
+        """
+        assert not self.service_is_available()  # Sanity check.
+
+        self.ServiceTestSetup().setUp()
+        old_pid = self.ServiceTestSetup.service.pid
+
+        time.sleep(1)  # Give it time to settle down.
+        self.ServiceTestSetup().setUp()
+        self.assertNotEqual(self.ServiceTestSetup.service.pid, old_pid)
+
+        # Tidy up.
+        time.sleep(1)  # Give it time to settle down.
+        self.ServiceTestSetup().tearDown()
+        self.assertFalse(self.service_is_available())
+        self.assertIsNone(self.ServiceTestSetup.service)

=== renamed file 'lib/lp/services/sitesearch/tests/test_googleservice.py' => 'lib/lp/services/sitesearch/tests/test_testservices.py'
--- lib/lp/services/sitesearch/tests/test_googleservice.py	2018-03-27 17:43:27 +0000
+++ lib/lp/services/sitesearch/tests/test_testservices.py	2018-03-30 23:12:56 +0000
@@ -1,9 +1,9 @@
 # Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""
-Unit tests for the Google test service stub.
-"""
+"""Unit tests for sitesearch test service stubs."""
+
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 
@@ -11,14 +11,28 @@
 import os
 import unittest
 
+from testscenarios import WithScenarios
+
 from lp.services.osutils import process_exists
 from lp.services.pidfile import pidfile_path
-from lp.services.sitesearch import googletestservice
-
-
-class TestServiceUtilities(unittest.TestCase):
+from lp.services.sitesearch import (
+    bingtestservice,
+    googletestservice,
+    )
+
+
+class TestServiceUtilities(WithScenarios, unittest.TestCase):
     """Test the service's supporting functions."""
 
+    scenarios = [
+        ("Bing", {
+            "testservice": bingtestservice,
+            }),
+        ("Google", {
+            "testservice": googletestservice,
+            }),
+        ]
+
     def test_stale_pid_file_cleanup(self):
         """The service should be able to clean up invalid PID files."""
         bogus_pid = 9999999
@@ -27,13 +41,12 @@
             "There is already a process with PID '%d'." % bogus_pid)
 
         # Create a stale/bogus PID file.
-        filepath = pidfile_path(googletestservice.service_name)
-        pidfile = file(filepath, 'w')
-        pidfile.write(str(bogus_pid))
-        pidfile.close()
+        filepath = pidfile_path(self.testservice.service_name)
+        with file(filepath, 'w') as pidfile:
+            pidfile.write(str(bogus_pid))
 
         # The PID clean-up code should silently remove the file and return.
-        googletestservice.kill_running_process()
+        self.testservice.kill_running_process()
         self.assertFalse(
             os.path.exists(filepath),
             "The PID file '%s' should have been deleted." % filepath)

=== modified file 'lib/lp/services/sitesearch/testservice.py' (properties changed: +x to -x)
--- lib/lp/services/sitesearch/testservice.py	2018-03-28 17:24:11 +0000
+++ lib/lp/services/sitesearch/testservice.py	2018-03-30 23:12:56 +0000
@@ -8,6 +8,19 @@
 when given certain user-configurable URLs.
 """
 
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'RequestHandler',
+    'hostpair',
+    'kill_running_process',
+    'service_is_available',
+    'start_as_process',
+    'url_to_file_map',
+    'wait_for_service',
+    'wait_for_service_shutdown',
+    ]
 
 import errno
 import os


Follow ups