Clean up browser tests in Files.app.

chrome/test/data/extensions/api_test/file_manager_browsertest/background.js

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * Extension ID of Files.app.
  * @type {string}
  * @const
  */
 var FILE_MANAGER_EXTENSIONS_ID = 'hhaomjibdihmijegdhdafkllkbggdgoj';
@@ skipping 10 matching lines @@
 function callRemoteTestUtil(func, appId, args, callback) {
   chrome.runtime.sendMessage(
       FILE_MANAGER_EXTENSIONS_ID, {
         func: func,
         appId: appId,
         args: args
       },
       callback);
 }
 
+/**
+ * Executes a sequence of test steps.
+ * @constructor
+ */
+function StepsRunner() {
+  /**
+   * List of steps.
+   * @type {Array.function>}
+   * @private
+   */
+  this.steps_ = [];
+}
+
+StepsRunner.prototype = {
+  /**
+   * @return {function} The next closure.
+   */
+  get next() {
+    return this.steps_.shift();
+  }
+};
+
+/**
+ * Runs a sequence of the added test steps.
+ * @type {Array.<function>} List of the sequential steps.
+ */
+StepsRunner.prototype.run = function(steps) {
+  this.steps_ = steps.slice(0);
+  this.steps_.push(function() {});

[hirono, 2013/05/30 04:46:10]

How about putting 'checkIfNoErrorsOccured' here?

[mtomasz, 2013/05/30 06:40:53]

I was thinking about, however I thought that putting asserts here would mess up
the code. This is just a very simple steps runner, not even in test_cases.js
file. WDYT?

[hirono, 2013/05/30 07:08:59]

I'm not sure, but StepsRunner already have the role of assertion since it uses
callbackPass.
If StepsRunner doesn't call callbackPass, this class may lose the value as a
helper class.
So it is not good idea to remove callbackPass.
And If StepsRunner should call callbackPass, it is natural to do other
assertion.
But it is my thought.
If StepsRunner does not call checkIfNoErrorsOccured, still lgtm.
Thank you for replying.

On 2013/05/30 06:40:53, mtomasz wrote:

[mtomasz, 2013/05/30 07:30:11]

Hm, callbackPass is just a utility to make tests work asynchronously. I think it
is different than a real assert. Also I think that StepsRunner sounds like a
tool to run steps only. Users may not expect that it does some extra calls via
ipc and validates errors.

However, this is also only my thought. Let's wait for hashimoto's point of view.

[hashimoto, 2013/05/30 08:21:45]

I feel neutral about this point.
Making StepsRunner focusing on its own job sounds good.
OTH, chrome.test.callback which is used to implement callbackPass seems doing
some error check.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/re...

Maybe we can implement more comprehensive test fixture implemented with
StepsRunner in near future to reduce code duplication like "appId = inAppId"
pattern, then we can include checkIfNoErrorsOccured in it.

[mtomasz, 2013/05/30 08:51:26]

That's a good idea. I will write a utility which adds the first step
(initialization) and the last step (error checking) in the nearest future.

+  this.steps_ = this.steps_.map(function(f) {
+    return chrome.test.callbackPass(f);
+  });
+  this.next();
+};
+
 chrome.test.runTests([
   // Waits for the C++ code to send a string identifying a test, then runs that
   // test.
   function testRunner() {
     var command = chrome.extension.inIncognitoContext ? 'which test guest' :
         'which test non-guest';
     chrome.test.sendMessage(command, function(testCaseName) {
       // Run one of the test cases defined in the testcase namespace, in
       // test_cases.js. The test case name is passed via StartTest call in
       // file_manager_browsertest.cc.
       if (testcase[testCaseName])
         testcase[testCaseName]();
       else
         chrome.test.fail('Bogus test name passed to testRunner()');
     });
   }
 ]);