Skip to content

Commit

Permalink
[Extensions] Add chrome.test.assertNe()
Browse files Browse the repository at this point in the history
Add a new chrome.test.assertNe() function to our test API. In addition
to being more readable, this avoids easy-to-make mistakes when
comparing objects (since they need to have a deep-equals comparison
done on them, rather than just `==`).

Bug: 1417270
Change-Id: I266bbbc5440d9e9cbefe60f538d53e378646debd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4265470
Reviewed-by: Tim <tjudkins@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1107102}
  • Loading branch information
rdcronin authored and Chromium LUCI CQ committed Feb 18, 2023
1 parent c599a43 commit bf596c9
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 4 deletions.
64 changes: 64 additions & 0 deletions chrome/browser/extensions/api/test/apitest_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,70 @@ IN_PROC_BROWSER_TEST_P(TestAPITestWithContextType, AsyncExceptions) {
EXPECT_EQ(kExpectedFailureMessage, result_catcher.message());
}

// Exercises chrome.test.assertNe() in cases where the check should succeed
// (that is, when the passed values are different).
IN_PROC_BROWSER_TEST_P(TestAPITestWithContextType, AssertNe_Success) {
ResultCatcher result_catcher;
static constexpr char kBackgroundJs[] =
R"(chrome.test.runTests([
function assertNeTestsWithPrimitiveTypes() {
chrome.test.assertNe(1, 2);
chrome.test.assertNe(2, 1);
chrome.test.assertNe(true, false);
chrome.test.assertNe(1.8, 2.4);
chrome.test.assertNe('tolstoy', 'dostoyevsky');
chrome.test.succeed();
},
function assertNeTestsWithObjects() {
chrome.test.assertNe([], [1]);
chrome.test.assertNe({x: 1}, {x: 2});
chrome.test.assertNe({x: 1}, {y: 1});
chrome.test.assertNe({}, []);
chrome.test.assertNe({}, 'Object object');
chrome.test.assertNe({}, '{}');
chrome.test.assertNe({}, null);
chrome.test.assertNe(null, {});
chrome.test.succeed();
},
]);)";
ASSERT_TRUE(LoadExtensionScriptWithContext(kBackgroundJs, GetParam()));
EXPECT_TRUE(result_catcher.GetNextResult());
}

// Exercises chrome.test.assertNe() in failure cases (i.e., the passed values
// are equal). We can only test one case at a time since otherwise we'd be
// unable to determine which part of the test failed (since "failure" here is
// a successful assertNe() check).
IN_PROC_BROWSER_TEST_P(TestAPITestWithContextType, AssertNe_Failure_Primitive) {
ResultCatcher result_catcher;
static constexpr char kBackgroundJs[] =
R"(chrome.test.runTests([
function assertNeTestsWithPrimitiveTypes() {
chrome.test.assertNe(1, 1);
},
]);)";
ASSERT_TRUE(LoadExtensionScriptWithContext(kBackgroundJs, GetParam()));
EXPECT_FALSE(result_catcher.GetNextResult());
EXPECT_EQ(kExpectedFailureMessage, result_catcher.message());
}

// Exercises chrome.test.assertNe() in failure cases (i.e., the passed values
// are equal). We can only test one case at a time since otherwise we'd be
// unable to determine which part of the test failed (since "failure" here is
// a successful assertNe() check).
IN_PROC_BROWSER_TEST_P(TestAPITestWithContextType, AssertNe_Failure_Object) {
ResultCatcher result_catcher;
static constexpr char kBackgroundJs[] =
R"(chrome.test.runTests([
function assertNeTestsWithPrimitiveTypes() {
chrome.test.assertNe({x: 42}, {x: 42});
},
]);)";
ASSERT_TRUE(LoadExtensionScriptWithContext(kBackgroundJs, GetParam()));
EXPECT_FALSE(result_catcher.GetNextResult());
EXPECT_EQ(kExpectedFailureMessage, result_catcher.message());
}

// Verifies that chrome.test.assertPromiseRejects() succeeds using
// promises that reject with the expected message.
IN_PROC_BROWSER_TEST_F(TestAPITest, AssertPromiseRejects_Successful) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ chrome.test.runTests([
let tab = await getSingleTab(query);
// Double-check that the title is not the one from the script file to be
// injected.
chrome.test.assertFalse(tab.title == NEW_TITLE_FROM_FILE);
chrome.test.assertNe(NEW_TITLE_FROM_FILE, tab.title);
const results = await chrome.scripting.executeScript({
target: {
tabId: tab.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ chrome.test.getConfig(async config => {
((resolve, details) => {
prerenderingFrameId = details.frameId;
prerenderingDocumentId = details.documentId;
chrome.test.assertTrue(prerenderingFrameId != 0);
chrome.test.assertNe(0, prerenderingFrameId);
chrome.webRequest.onBeforeRequest.removeListener(onBeforeRequest);
resolve();
}).bind(this, resolve);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ chrome.test.runTests([
chrome.test.assertEq('b.com', url2.hostname);
// Verify the subframe has any non-main-frame ID. Note: specific frame IDs
// are exercised more heavily below.
chrome.test.assertFalse(results[1].frameId == 0);
chrome.test.assertNe(0, results[1].frameId);
chrome.test.succeed();
},

Expand Down
17 changes: 17 additions & 0 deletions extensions/common/api/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,23 @@
{"type": "string", "name": "message", "optional": true}
]
},
{
"name": "assertNe",
"type": "function",
"nocompile": true,
"allowAmbiguousOptionalArguments": true,
"parameters": [
// These need to be optional because they can be null.
{"type": "any", "name": "expected", "optional": true},
{"type": "any", "name": "actual", "optional": true},
{
"type": "string",
"name": "message",
"optional": true,
"description": "A custom error message to print out with the test failure, if any."
}
]
},
{
"name": "assertNoLastError",
"type": "function",
Expand Down
25 changes: 25 additions & 0 deletions extensions/renderer/resources/test_custom_bindings.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,31 @@ apiBridge.registerCustomHook(function(api) {
}
});

apiFunctions.setHandleRequest('assertNe',
function(expected, actual, message) {
// Easy case: different types are always inequal.
if (typeof expected != typeof actual)
return;

let errorMsg = 'API Test Error in ' + testName(currentTest);
if (message)
errorMessage += ': ' + message;

if (typeof expected == 'object') {
if (chromeTest.checkDeepEq(expected, actual)) {
errorMsg += '\nExpected inequal values, but both are ' +
$JSON.stringify(expected);
chromeTest.fail(errorMsg);
}
return;
}

if (expected == actual) {
errorMsg += '\nExpected inequal values, but both are ' + expected;
chromeTest.fail(errorMsg);
}
});

apiFunctions.setHandleRequest('assertNoLastError', function() {
if (chrome.runtime.lastError != undefined) {
chromeTest.fail("lastError.message == " +
Expand Down
11 changes: 10 additions & 1 deletion third_party/closure_compiler/externs/test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022 The Chromium Authors
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -134,6 +134,15 @@ chrome.test.checkDeepEq = function(expected, actual) {};
*/
chrome.test.assertEq = function(expected, actual, message) {};

/**
* @param {*=} expected
* @param {*=} actual
* @param {string=} message A custom error message to print out with the test
* failure, if any.
* @see https://developer.chrome.com/extensions/test#method-assertNe
*/
chrome.test.assertNe = function(expected, actual, message) {};

/**
* @see https://developer.chrome.com/extensions/test#method-assertNoLastError
*/
Expand Down

0 comments on commit bf596c9

Please sign in to comment.