Commit 5f770283 authored by Luciano Pacheco's avatar Luciano Pacheco Committed by Commit Bot

Files app: Fix test stepper with async load of test functions

After crrev.com/c/1322340 which changed test functions to be lazy
loaded (asynchronously), running tests in step-by-step mode wasn't
working if tried to step after Files app background page was put to
sleep for inactivity.

There were 2 issues:

1. The logic was relying on a global state window.IN_TEST which gets
lost when extension goes to sleep.

2 Any queued response was replied with sendResponse(true), which was
skipping the execution of the test function that it requested to run.

This CL fixes both of these issues by doing:

1. Instead of relying on window.IN_TEST, it checks if test functions
are loaded with: "test.util.executeTestMessage !== null", if so runs
the requested function, otherwise en-queues the request and async-loads
the test functions.

2. Change to en-queue both the request and the sendResponse to be able
to run the requested functions once the test functions are fully
loaded.

This new design, made |enableTesting| and |ensureLoaded_| routines not
necessary anymore and they were removed.

Test: Manually run tests with the flags: --ui-test-action-timeout=100000
--enable-file-manager-step-by-step-tests  --remote-debugging-port=9222
and waited all extensions to disappear before issuing a step. Also
triggered MSAN and ASAN bots in addition to normal bots.

Bug: 903669
Change-Id: Ia66f7ce7cd920a7a815340119591ff5ff957603c
Reviewed-on: https://chromium-review.googlesource.com/c/1333177
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607842}
parent 7774f1d9
...@@ -40,14 +40,10 @@ test.util.executeTestMessage = null; ...@@ -40,14 +40,10 @@ test.util.executeTestMessage = null;
* Registers message listener, which runs test utility functions. * Registers message listener, which runs test utility functions.
*/ */
test.util.registerRemoteTestUtils = function() { test.util.registerRemoteTestUtils = function() {
/** let responsesWaitingForLoad = [];
* Responses that couldn't be sent while waiting for test scripts to load.
* Null if there is no load in progress.
* @type{Array<function(*)>}
*/
let responsesWaitingForLoad = null;
// Return true for asynchronous functions and false for synchronous. // Return true for asynchronous functions, which keeps the connection to the
// caller alive; Return false for synchronous functions.
chrome.runtime.onMessageExternal.addListener(function( chrome.runtime.onMessageExternal.addListener(function(
request, sender, sendResponse) { request, sender, sendResponse) {
/** /**
...@@ -69,49 +65,39 @@ test.util.registerRemoteTestUtils = function() { ...@@ -69,49 +65,39 @@ test.util.registerRemoteTestUtils = function() {
// handle external messages. // handle external messages.
return; return;
} }
if (window.IN_TEST) {
// If there are multiple foreground windows, the remote call API may have // Set the global IN_TEST flag, so other components are aware of it.
// already been initialised by one of them, so just return true to tell window.IN_TEST = true;
// the other window we are ready.
if ('enableTesting' in request) { // If testing functions are loaded just run the requested function.
sendResponse(true); if (test.util.executeTestMessage !== null)
return false; // No need to keep the connection alive.
}
return test.util.executeTestMessage(request, sendResponse); return test.util.executeTestMessage(request, sendResponse);
}
// When a valid test extension connects, the first message sent must be an // Queue the request/response pair.
// enable tests request. const obj = {request, sendResponse};
if (!('enableTesting' in request)) responsesWaitingForLoad.push(obj);
throw new Error('Expected enableTesting');
if (responsesWaitingForLoad != null) { // Only load the script with testing functions in the first request.
// Loading started, but not complete. Queue the response. if (responsesWaitingForLoad.length > 1)
responsesWaitingForLoad.push(sendResponse);
return true; return true;
}
responsesWaitingForLoad = [];
// Asynchronously load the testing functions.
let script = document.createElement('script'); let script = document.createElement('script');
document.body.appendChild(script); document.body.appendChild(script);
script.onload = function() { script.onload = function() {
// The runtime load should have populated test.util with // Run queued request/response pairs.
// executeTestMessage, allowing it to be invoked on the next responsesWaitingForLoad.forEach((queueObj) => {
// onMessageExternal call. test.util.executeTestMessage(queueObj.request, queueObj.sendResponse);
sendResponse(true);
responsesWaitingForLoad.forEach((queuedResponse) => {
queuedResponse(true);
}); });
responsesWaitingForLoad = null; responsesWaitingForLoad = [];
// Set a global flag that we are in tests, so other components are aware
// of it.
window.IN_TEST = true;
}; };
script.onerror = function(/** Event */ event) { script.onerror = function(/** Event */ event) {
console.error('Script load failed ' + event); console.error('Failed to load the run-time test script: ' + event);
throw new Error('Script load failed.'); throw new Error('Failed to load the run-time test script: ' + event);
}; };
const kFileManagerExtension = const kFileManagerExtension =
'chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj'; 'chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj';
const kTestScriptUrl = const kTestScriptUrl =
......
...@@ -25,7 +25,6 @@ function autoStep() { ...@@ -25,7 +25,6 @@ function autoStep() {
*/ */
function RemoteCall(extensionId) { function RemoteCall(extensionId) {
this.extensionId_ = extensionId; this.extensionId_ = extensionId;
this.testRuntimeLoaded_ = false;
/** /**
* Tristate holding the cached result of isStepByStepEnabled_(). * Tristate holding the cached result of isStepByStepEnabled_().
...@@ -52,25 +51,6 @@ RemoteCall.prototype.isStepByStepEnabled_ = function() { ...@@ -52,25 +51,6 @@ RemoteCall.prototype.isStepByStepEnabled_ = function() {
}); });
}; };
/**
* Asks the extension under test to load its testing functions.
* @private
* @return {Promise<bool>}
*/
RemoteCall.prototype.ensureLoaded_ = function() {
if (this.testRuntimeLoaded_)
return Promise.resolve(true);
return new Promise((fulfill) => {
chrome.runtime.sendMessage(
this.extensionId_, {enableTesting: true}, {}, (/** bool */ success) => {
chrome.test.assertTrue(success);
this.testRuntimeLoaded_ = success;
fulfill(success);
});
});
};
/** /**
* Calls a remote test util in the Files app's extension. See: * Calls a remote test util in the Files app's extension. See:
* registerRemoteTestUtils in test_util_base.js. * registerRemoteTestUtils in test_util_base.js.
...@@ -85,8 +65,7 @@ RemoteCall.prototype.ensureLoaded_ = function() { ...@@ -85,8 +65,7 @@ RemoteCall.prototype.ensureLoaded_ = function() {
*/ */
RemoteCall.prototype.callRemoteTestUtil = function( RemoteCall.prototype.callRemoteTestUtil = function(
func, appId, args, opt_callback) { func, appId, args, opt_callback) {
return this.ensureLoaded_() return this.isStepByStepEnabled_()
.then(this.isStepByStepEnabled_.bind(this))
.then((stepByStep) => { .then((stepByStep) => {
if (!stepByStep) if (!stepByStep)
return false; return false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment