Hello zakcohen,
I'd like you to do a code review. Please execute
g4 diff -c 8555351
or point your web browser to
http://mondrian/8555351
to review the following code:
Change 8555351 by [EMAIL PROTECTED] on 2008/10/10 10:32:29 *pending*
Fixes style for Geolocation JavaScript tests.
R=zakcohen
[EMAIL PROTECTED]
DELTA=62 (3 added, 0 deleted, 59 changed)
OCL=8555351
Affected files ...
... //depot/googleclient/gears/opensource/gears/test/testcases/config.js#35 edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#7
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_tests.js#41
edit
62 delta lines: 3 added, 0 deleted, 59 changed
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 8555351 by [EMAIL PROTECTED] on 2008/10/10 10:32:29 *pending*
Fixes style for Geolocation JavaScript tests.
Affected files ...
... //depot/googleclient/gears/opensource/gears/test/testcases/config.js#35 edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#7
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_tests.js#41
edit
==== //depot/googleclient/gears/opensource/gears/test/testcases/config.js#35 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/testcases/config.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/testcases/config.js
2008-11-18 18:15:04.000000000 +0000
+++ googleclient/gears/opensource/gears/test/testcases/config.js
2008-11-18 18:11:25.000000000 +0000
@@ -1,9 +1,9 @@
// Copyright 2007, Google Inc.
//
-// Redistribution and use in source and binary forms, with or without
+// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are met:
//
-// 1. Redistributions of source code must retain the above copyright notice,
+// 1. Redistributions of source code must retain the above copyright notice,
// this list of conditions and the following disclaimer.
// 2. Redistributions in binary form must reproduce the above copyright
notice,
// this list of conditions and the following disclaimer in the
documentation
@@ -13,14 +13,14 @@
// specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
-// WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+// WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
-// EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
// OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
-// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
-// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// This file contains information about all the Gears unit tests. It is used by
@@ -29,6 +29,8 @@
/**
* Represents a suite of tests that are logically related.
* @constructor
+ * @param {string} name string The name of the test suite.
+
*/
function TestSuite(name) {
this.name = name;
@@ -38,8 +40,9 @@
/**
* Adds an individual file to a test suite. Files can group related tests, or
* can be used to separate tests with different configuration requirements.
- * @param relativePath The path of the file to add to the suite.
- * @param config An object containing flags to control how the test runs.
+ * @param {string} relativePath The path of the file to add to the suite.
+ * @param {object} config An object containing flags to control how the test
+ * runs.
* Currently, support useWorker and useIFrame.
*/
TestSuite.prototype.addFile = function(relativePath, config) {
@@ -56,7 +59,7 @@
if (!isOfficial && !isSafari && !isWince) {
var audioSuite = new TestSuite('Audio');
- audioSuite.addFile('../testcases/audio_tests.js',
+ audioSuite.addFile('../testcases/audio_tests.js',
{useWorker: true, useIFrame: true});
audioSuite.addFile('../testcases/audio_recorder_tests.js',
{useWorker: true, useIFrame: true});
@@ -122,7 +125,7 @@
suites.push(factorySuite);
var geolocationSuite = new TestSuite('Geolocation');
-geolocationSuite.addFile('../testcases/geolocation_tests.js',
+geolocationSuite.addFile('../testcases/geolocation_tests.js',
{useWorker: true, useIFrame: true});
suites.push(geolocationSuite);
@@ -134,7 +137,7 @@
var internalTestSuite = new TestSuite('Internal_Tests');
internalTestSuite.addFile('../testcases/internal_tests.js',
{useWorker: true, useIFrame: true});
-internalTestSuite.addFile('../testcases/internal_coercion_tests.js',
+internalTestSuite.addFile('../testcases/internal_coercion_tests.js',
{useWorker: true, useIFrame: true});
if (!isOfficial && isUsingCCTests && !isWince) {
internalTestSuite.addFile('../testcases/internal_audio_recorder_tests.js',
@@ -149,7 +152,7 @@
{useWorker: true, useIFrame: true});
localServerSuite.addFile('../testcases/localserver_noworker_tests.js',
{useWorker: false, useIFrame: true});
-// ResourceStore's captureFile functionality is currently only supported
+// ResourceStore's captureFile functionality is currently only supported
// in IE and FireFox and may be deprected in the near future.
if (isIE || isFirefox) {
localServerSuite.addFile('../testcases/localserver_capturefile_tests.js',
@@ -165,7 +168,7 @@
}
var timerSuite = new TestSuite('Timer');
-timerSuite.addFile('../testcases/timer_tests.js',
+timerSuite.addFile('../testcases/timer_tests.js',
{useWorker: true, useIFrame: true});
suites.push(timerSuite);
====
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#7
-
c:\MyDocs\Gears3/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-18 18:07:14.000000000 +0000
+++
googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
2008-11-18 18:12:15.000000000 +0000
@@ -43,7 +43,7 @@
assertEqual(false, parsedOptions.repeats);
assertEqual(false, parsedOptions.enableHighAccuracy);
assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual("", parsedOptions.gearsAddressLanguage);
+ assertEqual('', parsedOptions.gearsAddressLanguage);
assertArrayEqual(defaultUrlArray, parsedOptions.gearsLocationProviderUrls);
// Empty options.
@@ -52,7 +52,7 @@
assertEqual(false, parsedOptions.repeats);
assertEqual(false, parsedOptions.enableHighAccuracy);
assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual("", parsedOptions.gearsAddressLanguage);
+ assertEqual('', parsedOptions.gearsAddressLanguage);
assertArrayEqual(defaultUrlArray, parsedOptions.gearsLocationProviderUrls);
// Empty provider URLs.
@@ -61,7 +61,7 @@
assertEqual(false, parsedOptions.repeats);
assertEqual(false, parsedOptions.enableHighAccuracy);
assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual("", parsedOptions.gearsAddressLanguage);
+ assertEqual('', parsedOptions.gearsAddressLanguage);
assertArrayEqual([], parsedOptions.gearsLocationProviderUrls);
// Null provider URLs.
@@ -70,7 +70,7 @@
assertEqual(false, parsedOptions.repeats);
assertEqual(false, parsedOptions.enableHighAccuracy);
assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual("", parsedOptions.gearsAddressLanguage);
+ assertEqual('', parsedOptions.gearsAddressLanguage);
assertArrayEqual([], parsedOptions.gearsLocationProviderUrls);
// All properties false, provider URLs set.
@@ -80,13 +80,13 @@
{
enableHighAccuracy: false,
gearsRequestAddress: false,
- gearsAddressLanguage: "",
+ gearsAddressLanguage: '',
gearsLocationProviderUrls: urls
});
assertEqual(false, parsedOptions.repeats);
assertEqual(false, parsedOptions.enableHighAccuracy);
assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual("", parsedOptions.gearsAddressLanguage);
+ assertEqual('', parsedOptions.gearsAddressLanguage);
assertArrayEqual(urls, parsedOptions.gearsLocationProviderUrls);
// All properties true, provider URLs set.
@@ -96,13 +96,13 @@
{
enableHighAccuracy: true,
gearsRequestAddress: true,
- gearsAddressLanguage: "test",
+ gearsAddressLanguage: 'test',
gearsLocationProviderUrls: urls
});
assertEqual(false, parsedOptions.repeats);
assertEqual(true, parsedOptions.enableHighAccuracy);
assertEqual(true, parsedOptions.gearsRequestAddress);
- assertEqual("test", parsedOptions.gearsAddressLanguage);
+ assertEqual('test', parsedOptions.gearsAddressLanguage);
assertArrayEqual(urls, parsedOptions.gearsLocationProviderUrls);
}
}
@@ -330,7 +330,7 @@
function makeSuccessfulRequest() {
internalTests.configureGeolocationWifiDataProviderForTest(
- {mac_address: "good_mac_address"});
+ {mac_address: 'good_mac_address'});
geolocation.getCurrentPosition(
successCallback,
function() {
@@ -338,31 +338,31 @@
},
{
gearsRequestAddress: true,
- gearsLocationProviderUrls: [ mockNetworkLocationProvider ]
+ gearsLocationProviderUrls: [mockNetworkLocationProvider]
});
}
function makeUnsuccessfulRequest() {
internalTests.configureGeolocationWifiDataProviderForTest(
- {mac_address: "no_location_mac_address"});
+ {mac_address: 'no_location_mac_address'});
geolocation.getCurrentPosition(
function() {
assert(false, 'makeUnsuccessfulRequest succeeded');
},
noPositionErrorCallback,
- {gearsLocationProviderUrls: [ mockNetworkLocationProvider ]});
+ {gearsLocationProviderUrls: [mockNetworkLocationProvider]});
}
function makeMalformedRequest() {
internalTests.configureGeolocationWifiDataProviderForTest(
- {mac_address: "00-00-00-00-00-00"});
+ {mac_address: '00-00-00-00-00-00'});
internalTests.configureGeolocationRadioDataProviderForTest({cell_id:
88});
geolocation.getCurrentPosition(
function() {
assert(false, 'makeMalformedRequest succeeded');
},
malformedRequestErrorCallback,
- {gearsLocationProviderUrls: [ mockNetworkLocationProvider ]});
+ {gearsLocationProviderUrls: [mockNetworkLocationProvider]});
}
function successCallback(position) {
@@ -373,14 +373,14 @@
accuracy: 1200,
altitudeAccuracy: 10,
gearsAddress: {
- streetNumber: "76",
- street: "Buckingham Palace Road",
- postalCode: "SW1W 9TQ",
- city: "London",
- county: "London",
- region: "London",
- country: "United Kingdom",
- countryCode: "uk"
+ streetNumber: '76',
+ street: 'Buckingham Palace Road',
+ postalCode: 'SW1W 9TQ',
+ city: 'London',
+ county: 'London',
+ region: 'London',
+ country: 'United Kingdom',
+ countryCode: 'uk'
}
};
position.timestamp = undefined;
@@ -531,14 +531,14 @@
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"
+ 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);
====
//depot/googleclient/gears/opensource/gears/test/testcases/internal_tests.js#41
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/testcases/internal_tests.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/testcases/internal_tests.js
2008-11-18 18:15:04.000000000 +0000
+++ googleclient/gears/opensource/gears/test/testcases/internal_tests.js
2008-11-18 17:51:08.000000000 +0000
@@ -42,35 +42,35 @@
internalTests.testPassArguments(args[0], args[1], args[2], args[3],
args[4]);
}
-
+
var good_bool = true;
var good_int = 42;
var good_int64 = Math.pow(2, 42);
var good_double = 88.8;
- var good_string = "hotdog";
-
+ var good_string = 'hotdog';
+
var bad_bool = false;
var bad_int = 43;
var bad_int64 = Math.pow(2, 43);
var bad_double = 88.9;
- var bad_string = "hotdof";
-
+ var bad_string = 'hotdof';
+
// Test a good one
var good_vals = [good_bool, good_int, good_int64, good_double,
good_string];
callTest(good_vals);
-
+
// Test passing wrong values
var bad_vals = [bad_bool, bad_int, bad_int64, bad_double, bad_string];
for (var i = 0; i < good_vals.length; i++) {
var test_vals = [].concat(good_vals);
test_vals[i] = bad_vals[i];
assertError(function() { callTest(test_vals) },
- "Incorrect value for parameter " + (i + 1));
+ 'Incorrect value for parameter ' + (i + 1));
}
// Test passing null and undefined for required args
var required_argument_error =
- "Null or undefined passed for required argument 5.";
+ 'Null or undefined passed for required argument 5.';
assertError(function() {
internalTests.testPassArguments(good_bool, good_int, good_int64,
good_double, null); },
@@ -84,7 +84,7 @@
assertError(function() {
internalTests.testPassArguments(good_bool, good_int, good_int64,
good_double); },
- "Required argument 5 is missing");
+ 'Required argument 5 is missing');
// Test passing wrong type
for (var i = 0; i < good_vals.length; i++) {
@@ -98,22 +98,22 @@
if (good_vals[i] == good_int64 && good_vals[j] == good_int) {
expect_conversion_allowed = false;
}
-
+
// Same for converting int and int64 to double.
if (good_vals[i] == good_double) {
if (good_vals[j] == good_int || good_vals[j] == good_int64) {
expect_conversion_allowed = false;
}
}
-
+
var test_vals = [].concat(good_vals);
test_vals[i] = test_vals[j];
var expected_error = expect_conversion_allowed ?
- "Argument " + (i + 1) + " has invalid type or is outside allowed "
+
- "range" :
- "Incorrect value for parameter " + (i + 1);
-
+ 'Argument ' + (i + 1) + ' has invalid type or is outside allowed '
+
+ 'range' :
+ 'Incorrect value for parameter ' + (i + 1);
+
assertError(function() { callTest(test_vals); }, expected_error);
}
}
@@ -122,11 +122,11 @@
function testPassArgumentsCallback() {
function testPassArgumentCallbackFunction(arg1, arg2, arg3, arg4, arg5) {
- assert(arg1 === true, "Bad value for argument 1");
- assert(arg2 === 42, "Bad value for argument 2");
- assert(arg3 === Math.pow(2, 42), "Bad value for argument 3");
- assert(arg4 === 88.8, "Bad value for argument 4");
- assert(arg5 === "hotdog", "Bad value for argument 5");
+ assert(arg1 === true, 'Bad value for argument 1');
+ assert(arg2 === 42, 'Bad value for argument 2');
+ assert(arg3 === Math.pow(2, 42), 'Bad value for argument 3');
+ assert(arg4 === 88.8, 'Bad value for argument 4');
+ assert(arg5 === 'hotdog', 'Bad value for argument 5');
}
if (isUsingCCTests) {
internalTests.testPassArgumentsCallback(testPassArgumentCallbackFunction);
@@ -152,7 +152,7 @@
internalTests.testPassArgumentsOptional(42, null, 42);
// Missing required argument.
- var expectedError = "Required argument 3 is missing.";
+ var expectedError = 'Required argument 3 is missing.';
assertError(
function() { internalTests.testPassArgumentsOptional(42, 42); },
expectedError);
@@ -162,16 +162,16 @@
// Incorrect type for optional argument.
assertError(
- function() { internalTests.testPassArgumentsOptional(42, 42, 42, "hi");
},
- "Argument 4 has invalid type or is outside allowed range.");
- assertError(
- function() { internalTests.testPassArgumentsOptional(42, "hi"); },
- "Argument 2 has invalid type or is outside allowed range.");
+ function() { internalTests.testPassArgumentsOptional(42, 42, 42, 'hi');
},
+ 'Argument 4 has invalid type or is outside allowed range.');
+ assertError(
+ function() { internalTests.testPassArgumentsOptional(42, 'hi'); },
+ 'Argument 2 has invalid type or is outside allowed range.');
// Incorrect value for optional argument.
assertError(
function() { internalTests.testPassArgumentsOptional(42, 42, 42, 43); },
- "Incorrect value for parameter 4.");
+ 'Incorrect value for parameter 4.');
}
}
@@ -197,7 +197,7 @@
if (isUsingCCTests) {
// TODO(cdevries): Enable this test on FF in worker pools when
// SetReturnValue() has been implemented.
- var isFirefox = google.gears.factory.getBuildInfo().indexOf("firefox") >
-1;
+ var isFirefox = google.gears.factory.getBuildInfo().indexOf('firefox') >
-1;
var isFirefoxWorker = isFirefox && google.gears.workerPool;
if (!isFirefoxWorker) {
@@ -220,7 +220,7 @@
if (isUsingCCTests) {
// TODO(cdevries): Enable this test on FF in worker pools when
// SetReturnValue() has been implemented.
- var isFirefox = google.gears.factory.getBuildInfo().indexOf("firefox") >
-1;
+ var isFirefox = google.gears.factory.getBuildInfo().indexOf('firefox') >
-1;
var isFirefoxWorker = isFirefox && google.gears.workerPool;
if (!isFirefoxWorker) {
@@ -278,9 +278,9 @@
function testAsyncTaskPostCookies() {
// TODO(steveblock): Enable this test for Chrome when bug 1301226 is fixed.
if (isUsingCCTests && !(isWin32 && isNPAPI)) {
- var setCookieUrl = "/testcases/set_cookie.txt";
- var serveIfCookiesPresentUrl = "/testcases/serve_if_cookies_present.txt";
- var serveIfCookiesAbsentUrl = "/testcases/serve_if_cookies_absent.txt";
+ var setCookieUrl = '/testcases/set_cookie.txt';
+ var serveIfCookiesPresentUrl = '/testcases/serve_if_cookies_present.txt';
+ var serveIfCookiesAbsentUrl = '/testcases/serve_if_cookies_absent.txt';
var testIndex = 0;
function runNextTest(returnCode) {
@@ -308,7 +308,7 @@
assertEqual(
404, returnCode, 'Expected 404 for request with cookies.');
internalTests.testAsyncTaskPostCookies(
- serveIfCookiesAbsentUrl,false, runNextTest);
+ serveIfCookiesAbsentUrl, false, runNextTest);
break;
case 4:
assertEqual(
@@ -316,7 +316,7 @@
completeAsync();
break;
default:
- throw new Error("Unexpected test index.");
+ throw new Error('Unexpected test index.');
}
}
