Clean up browser tests in Files.app.

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

Index: chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
diff --git a/chrome/test/data/extensions/api_test/file_manager_browsertest/background.js b/chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
index bc60f6fb58f86668d2c4e8440117945320275a24..3fccf6df9b74658c586368b0cb48131b2fddb0be 100644
--- a/chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
+++ b/chrome/test/data/extensions/api_test/file_manager_browsertest/background.js
@@ -28,6 +28,41 @@ function callRemoteTestUtil(func, appId, args, callback) {
       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.