************************* G4 reminder *************************
These new files:

        
c:\MyDocs\Gears2\googleclient\gears\opensource\gears\test\testcases\cgi\reverse_geocoder.py

are missing unit tests.
***************************************************************

Hello andreip,

I'd like you to do a code review.  Please execute
        g4 diff -c 9034220

or point your web browser to
        http://mondrian/9034220

to review the following code:

Change 9034220 by [EMAIL PROTECTED] on 2008/11/17 16:20:16 *pending*

        Adds unit tests for reverse geocoding.
        
        R=andreip
        [EMAIL PROTECTED]
        DELTA=150  (145 added, 3 deleted, 2 changed)
        OCL=9034220

Affected files ...

... 
//depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider.cc#5
 edit
... 
//depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider.h#2
 edit
... 
//depot/googleclient/gears/opensource/gears/test/testcases/cgi/reverse_geocoder.py#1
 add
... 
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#6
 edit

150 delta lines: 145 added, 3 deleted, 2 changed

Also consider running:
        g4 lint -c 9034220

which verifies that the changelist doesn't introduce new style violations.

If you can't do the review, please let me know as soon as possible.  During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately.  Visit
http://www/eng/code_review.html for more information.

This is a semiautomated message from "g4 mail".  Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 9034220 by [EMAIL PROTECTED] on 2008/11/17 16:20:16 *pending*

        Adds unit tests for reverse geocoding.

Affected files ...

... 
//depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider.cc#5
 edit
... 
//depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider.h#2
 edit
... 
//depot/googleclient/gears/opensource/gears/test/testcases/cgi/reverse_geocoder.py#1
 add
... 
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#6
 edit

==== 
//depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider.cc#5
 - 
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/geolocation/gps_location_provider.cc
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/gps_location_provider.cc    
2008-11-17 18:37:15.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/gps_location_provider.cc    
2008-11-17 18:15:02.000000000 +0000
@@ -319,7 +319,7 @@
   // Copy only the address, so as to preserve the position if an update has 
been
   // made since the reverse geocode started.
   position_mutex_.Lock();
-  position_.address = new_address_;
+  position_.address = position.address;
   position_mutex_.Unlock();
 
   is_new_reverse_geocode_available_ = true;
==== 
//depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider.h#2
 - 
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/geolocation/gps_location_provider.h
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/gps_location_provider.h     
2008-11-17 18:37:15.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/gps_location_provider.h     
2008-11-17 18:15:33.000000000 +0000
@@ -114,10 +114,10 @@
   std::string16 host_name_;
   std::string16 address_language_;
 
-  // The current best position estimate and its mutex.
+  // The current best position estimate, the latest position to be reverse
+  // geocoded and their mutex.
   Position position_;
   Position new_position_;
-  Address new_address_;
   Mutex position_mutex_;
 
   // Event used to signal to the worker thread.
==== 
//depot/googleclient/gears/opensource/gears/test/testcases/cgi/reverse_geocoder.py#1
 - 
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/test/testcases/cgi/reverse_geocoder.py
 ====
# action=add type=xtext
--- /dev/null   1970-01-01 00:00:00.000000000 +0000
+++ googleclient/gears/opensource/gears/test/testcases/cgi/reverse_geocoder.py  
2008-11-17 18:36:19.000000000 +0000
@@ -0,0 +1,90 @@
+#!/usr/bin/python2.4
+
+""" A dummy network location provider that is used for testing reverse
+geocoding. For fix requests it does not return a position. For reverse geocode
+requests, it returns a fixed address.
+"""
+
+class FixRequest(object):
+  """ Represents a fix request.
+  """
+
+  def __init__(self, request_body):
+    """ Initialize FixRequest with a JSON string.
+
+    Args:
+      request_body: Request body as JSON string.
+    """
+    # simplejson module is imported by the server, see
+    # /test/runner/testwebserver.py
+    json_decoder = simplejson.JSONDecoder()
+    self.request = json_decoder.decode(request_body)
+
+  def IsValidRequest(self):
+    return self.request.has_key("version") and self.request.has_key("host")
+
+  def IsReverseGeocodeRequest(self):
+    if self.request.has_key("location"):
+      location = self.request["location"]      
+      if location.has_key("latitude") and location.has_key("longitude"):
+        return True
+    return False
+
+# A successful reverse-geocode response. We must include a valid latitude and
+# longitude, but Gears will return to JavaScript the values from the reverse
+# geocode request, not these values.
+REVERSE_GEOCODE_REQUEST_RESPONSE = """{
+  "location": {
+    "latitude": 9.9,
+    "longitude": 9.9,
+    "address": {
+      "street_number": "76",
+      "street": "Buckingham Palace Road",
+      "postal_code": "SW1W 9TQ",
+      "city": "London",
+      "county": "London",
+      "region": "London",
+      "country": "United Kingdom",
+      "country_code": "uk"
+    }
+  }
+}"""
+
+# An empty position response, indicates no location fixes were found.
+FIX_REQUEST_RESPONSE = """{}"""
+
+def send_response(request_handler, code, response):
+  """ Helper function to construct a HTTP response.
+
+  Args:
+    request_handler: The request handler instance to issue the response
+    code: The integer HTTP status code for the response
+    response: The string body of the response.
+  """
+
+  request_handler.send_response(code)
+  if code == 200 :
+    request_handler.send_header('Content-type', 'application/json')
+  request_handler.end_headers()
+  request_handler.outgoing.append(response)
+
+
+# We accept only POST requests with Content-Type application/json.
+if self.command == 'POST':
+  if self.headers.get('Content-Type') == 'application/json':
+    if self.body:
+      fix_request = FixRequest(self.body)
+
+      # Hard-coded rules to return desired responses
+      if not fix_request.IsValidRequest():
+        send_response(self, 400, "Invalid request")
+      elif fix_request.IsReverseGeocodeRequest():
+        send_response(self, 200, REVERSE_GEOCODE_REQUEST_RESPONSE)
+      else:
+        send_response(self, 200, FIX_REQUEST_RESPONSE)
+    else:
+      send_response(self, 400, "Empty request")
+  else:
+    send_response(self, 400, "Content-Type should be application/json")
+else:
+  send_response(self, 405, "Request must be HTTP POST.")
==== 
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#6
 - 
c:\MyDocs\Gears2/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
 ====
# action=edit type=text
--- 
googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
    2008-11-17 18:37:15.000000000 +0000
+++ 
googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
    2008-11-17 17:48:59.000000000 +0000
@@ -451,8 +451,6 @@
 
 function testMockGpsDevice() {
   if (isUsingCCTests) {
-    // TODO(steveblock): Add tests that cover reverse geocoding.
-
     var geolocation = google.gears.factory.create('beta.geolocation');
     var options = {
       enableHighAccuracy: true,
@@ -503,3 +501,57 @@
     }
   }
 }
+
+function testReverseGeocoding() {
+  if (isUsingCCTests) {
+    // GPS is available only on WinCE and Android.
+    if (isWince || isAndroid) {
+      var geolocation = google.gears.factory.create('beta.geolocation');
+
+      // This network location provider will always fail to provide a position
+      // for all location requests. For all reverse-geocoding requests, it
+      // returns a fixed address.
+      var reverseGeocoder = '/testcases/cgi/reverse_geocoder.py';
+
+      var options = {
+        enableHighAccuracy: true,
+        gearsRequestAddress: true,
+        gearsLocationProviderUrls: [reverseGeocoder]
+      };
+
+      var mockGpsPosition = {
+        latitude: 0.1,
+        longitude: 2.3,
+        altitude: 4,
+        accuracy: 5,
+        altitudeAccuracy: 6
+      };
+
+      function successCallback(position) {
+        var correctPosition = mockGpsPosition;
+        // This is hard-coded in reverse_geocoder.py.
+        correctPosition.gearsAddress = {
+          streetNumber: "76",
+          street: "Buckingham Palace Road",
+          postalCode: "SW1W 9TQ",
+          city: "London",
+          county: "London",
+          region: "London",
+          country: "United Kingdom",
+          countryCode: "uk"
+        };
+        position.timestamp = undefined;
+        assertObjectEqual(correctPosition, position);
+        completeAsync();
+      }
+
+      function errorCallback(error) {
+        assert(false, 'testReverseGeocoding failed: ' + error);
+      }
+
+      internalTests.configureGeolocationMockGpsDeviceForTest(mockGpsPosition);
+      startAsync();
+      geolocation.getCurrentPosition(successCallback, errorCallback, options);
+    }
+  }
+}

Reply via email to