Bug 748684 - GCLI shouldn't require removeCommand before addCommand; r=dcamp
authorJoe Walker <jwalker@mozilla.com>
Tue, 15 May 2012 11:27:18 +0100
changeset 94088 cb193fb4d45ab03309612e69999a55ee9e62523d
parent 94087 5467d61f6e6e1fb83f6b19b3de78b184dc65e8ff
child 94089 9cb840a343947ae24822efd2ac0792211c78d0d7
push id22696
push userrcampbell@mozilla.com
push dateWed, 16 May 2012 19:04:23 +0000
treeherdermozilla-central@65fb8b9ea0b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdcamp
bugs748684
milestone15.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 748684 - GCLI shouldn't require removeCommand before addCommand; r=dcamp
browser/devtools/shared/test/browser_gcli_web.js
browser/devtools/webconsole/gcli.jsm
--- a/browser/devtools/shared/test/browser_gcli_web.js
+++ b/browser/devtools/shared/test/browser_gcli_web.js
@@ -93,26 +93,27 @@ define('gclitest/index', ['require', 'ex
 
 });
 /*
  * Copyright 2009-2011 Mozilla Foundation and contributors
  * Licensed under the New BSD license. See LICENSE.txt or:
  * http://opensource.org/licenses/BSD-3-Clause
  */
 
-define('gclitest/suite', ['require', 'exports', 'module' , 'gcli/index', 'test/examiner', 'gclitest/testCli', 'gclitest/testCompletion', 'gclitest/testExec', 'gclitest/testHelp', 'gclitest/testHistory', 'gclitest/testInputter', 'gclitest/testIntro', 'gclitest/testJs', 'gclitest/testKeyboard', 'gclitest/testPref', 'gclitest/testRequire', 'gclitest/testResource', 'gclitest/testScratchpad', 'gclitest/testSettings', 'gclitest/testSpell', 'gclitest/testSplit', 'gclitest/testTokenize', 'gclitest/testTooltip', 'gclitest/testTypes', 'gclitest/testUtil'], function(require, exports, module) {
+define('gclitest/suite', ['require', 'exports', 'module' , 'gcli/index', 'test/examiner', 'gclitest/testCanon', 'gclitest/testCli', 'gclitest/testCompletion', 'gclitest/testExec', 'gclitest/testHelp', 'gclitest/testHistory', 'gclitest/testInputter', 'gclitest/testIntro', 'gclitest/testJs', 'gclitest/testKeyboard', 'gclitest/testPref', 'gclitest/testRequire', 'gclitest/testResource', 'gclitest/testScratchpad', 'gclitest/testSettings', 'gclitest/testSpell', 'gclitest/testSplit', 'gclitest/testTokenize', 'gclitest/testTooltip', 'gclitest/testTypes', 'gclitest/testUtil'], function(require, exports, module) {
 
   // We need to make sure GCLI is initialized before we begin testing it
   require('gcli/index');
 
   var examiner = require('test/examiner');
 
   // It's tempting to want to unify these strings and make addSuite() do the
   // call to require(), however that breaks the build system which looks for
   // the strings passed to require
+  examiner.addSuite('gclitest/testCanon', require('gclitest/testCanon'));
   examiner.addSuite('gclitest/testCli', require('gclitest/testCli'));
   examiner.addSuite('gclitest/testCompletion', require('gclitest/testCompletion'));
   examiner.addSuite('gclitest/testExec', require('gclitest/testExec'));
   examiner.addSuite('gclitest/testHelp', require('gclitest/testHelp'));
   examiner.addSuite('gclitest/testHistory', require('gclitest/testHistory'));
   examiner.addSuite('gclitest/testInputter', require('gclitest/testInputter'));
   examiner.addSuite('gclitest/testIntro', require('gclitest/testIntro'));
   examiner.addSuite('gclitest/testJs', require('gclitest/testJs'));
@@ -556,16 +557,288 @@ define('test/assert', ['require', 'expor
 
 });
 /*
  * Copyright 2009-2011 Mozilla Foundation and contributors
  * Licensed under the New BSD license. See LICENSE.txt or:
  * http://opensource.org/licenses/BSD-3-Clause
  */
 
+define('gclitest/testCanon', ['require', 'exports', 'module' , 'gclitest/helpers', 'gcli/canon', 'test/assert'], function(require, exports, module) {
+
+  var helpers = require('gclitest/helpers');
+  var canon = require('gcli/canon');
+  var test = require('test/assert');
+
+  exports.testAddRemove = function(options) {
+    var startCount = canon.getCommands().length;
+    var events = 0;
+
+    var canonChange = function(ev) {
+      events++;
+    };
+    canon.onCanonChange.add(canonChange);
+
+    canon.addCommand({
+      name: 'testadd',
+      exec: function() {
+        return 1;
+      }
+    });
+
+    test.is(canon.getCommands().length, startCount + 1, 'add command success');
+    test.is(events, 1, 'add event');
+    helpers.exec(options, {
+      typed: 'testadd',
+      outputMatch: /^1$/
+    });
+
+    canon.addCommand({
+      name: 'testadd',
+      exec: function() {
+        return 2;
+      }
+    });
+
+    test.is(canon.getCommands().length, startCount + 1, 'readd command success');
+    test.is(events, 2, 'readd event');
+    helpers.exec(options, {
+      typed: 'testadd',
+      outputMatch: /^2$/
+    });
+
+    canon.removeCommand('testadd');
+
+    test.is(canon.getCommands().length, startCount, 'remove command success');
+    test.is(events, 3, 'remove event');
+    helpers.status(options, {
+      typed: 'testadd',
+      status: 'ERROR'
+    });
+
+    canon.addCommand({
+      name: 'testadd',
+      exec: function() {
+        return 3;
+      }
+    });
+
+    test.is(canon.getCommands().length, startCount + 1, 'rereadd command success');
+    test.is(events, 4, 'rereadd event');
+    helpers.exec(options, {
+      typed: 'testadd',
+      outputMatch: /^3$/
+    });
+
+    canon.removeCommand({
+      name: 'testadd'
+    });
+
+    test.is(canon.getCommands().length, startCount, 'reremove command success');
+    test.is(events, 5, 'reremove event');
+    helpers.status(options, {
+      typed: 'testadd',
+      status: 'ERROR'
+    });
+
+    canon.onCanonChange.remove(canonChange);
+  };
+
+});
+/*
+ * Copyright 2009-2011 Mozilla Foundation and contributors
+ * Licensed under the New BSD license. See LICENSE.txt or:
+ * http://opensource.org/licenses/BSD-3-Clause
+ */
+
+define('gclitest/helpers', ['require', 'exports', 'module' , 'test/assert'], function(require, exports, module) {
+
+
+var test = require('test/assert');
+
+/**
+ * Check that we can parse command input.
+ * Doesn't execute the command, just checks that we grok the input properly:
+ *
+ * helpers.status({
+ *   // Test inputs
+ *   typed: "ech",           // Required
+ *   cursor: 3,              // Optional cursor position
+ *
+ *   // Thing to check
+ *   status: "INCOMPLETE",   // One of "VALID", "ERROR", "INCOMPLETE"
+ *   emptyParameters: [ "<message>" ], // Still to type
+ *   directTabText: "o",     // Simple completion text
+ *   arrowTabText: "",       // When the completion is not an extension
+ *   markup: "VVVIIIEEE",    // What state should the error markup be in
+ * });
+ */
+exports.status = function(options, tests) {
+  var requisition = options.display.requisition;
+  var inputter = options.display.inputter;
+  var completer = options.display.completer;
+
+  if (tests.typed) {
+    inputter.setInput(tests.typed);
+  }
+  else {
+    test.ok(false, "Missing typed for " + JSON.stringify(tests));
+    return;
+  }
+
+  if (tests.cursor) {
+    inputter.setCursor(tests.cursor);
+  }
+
+  if (tests.status) {
+    test.is(requisition.getStatus().toString(), tests.status,
+            "status for " + tests.typed);
+  }
+
+  if (tests.emptyParameters != null) {
+    var realParams = completer.emptyParameters;
+    test.is(realParams.length, tests.emptyParameters.length,
+            'emptyParameters.length for \'' + tests.typed + '\'');
+
+    if (realParams.length === tests.emptyParameters.length) {
+      for (var i = 0; i < realParams.length; i++) {
+        test.is(realParams[i].replace(/\u00a0/g, ' '), tests.emptyParameters[i],
+                'emptyParameters[' + i + '] for \'' + tests.typed + '\'');
+      }
+    }
+  }
+
+  if (tests.markup) {
+    var cursor = tests.cursor ? tests.cursor.start : tests.typed.length;
+    var statusMarkup = requisition.getInputStatusMarkup(cursor);
+    var actualMarkup = statusMarkup.map(function(s) {
+      return Array(s.string.length + 1).join(s.status.toString()[0]);
+    }).join('');
+    test.is(tests.markup, actualMarkup, 'markup for ' + tests.typed);
+  }
+
+  if (tests.directTabText) {
+    test.is(completer.directTabText, tests.directTabText,
+            'directTabText for \'' + tests.typed + '\'');
+  }
+
+  if (tests.arrowTabText) {
+    test.is(completer.arrowTabText, ' \u00a0\u21E5 ' + tests.arrowTabText,
+            'arrowTabText for \'' + tests.typed + '\'');
+  }
+};
+
+/**
+ * Execute a command:
+ *
+ * helpers.exec({
+ *   // Test inputs
+ *   typed: "echo hi",        // Optional, uses existing if undefined
+ *
+ *   // Thing to check
+ *   args: { message: "hi" }, // Check that the args were understood properly
+ *   outputMatch: /^hi$/,     // Regex to test against textContent of output
+ *   blankOutput: true,       // Special checks when there is no output
+ * });
+ */
+exports.exec = function(options, tests) {
+  var requisition = options.display.requisition;
+  var inputter = options.display.inputter;
+
+  tests = tests || {};
+
+  if (tests.typed) {
+    inputter.setInput(tests.typed);
+  }
+
+  var typed = inputter.getInputState().typed;
+  var output = requisition.exec({ hidden: true });
+
+  test.is(typed, output.typed, 'output.command for: ' + typed);
+
+  if (tests.completed !== false) {
+    test.ok(output.completed, 'output.completed false for: ' + typed);
+  }
+  else {
+    // It is actually an error if we say something is async and it turns
+    // out not to be? For now we're saying 'no'
+    // test.ok(!output.completed, 'output.completed true for: ' + typed);
+  }
+
+  if (tests.args != null) {
+    test.is(Object.keys(tests.args).length, Object.keys(output.args).length,
+            'arg count for ' + typed);
+
+    Object.keys(output.args).forEach(function(arg) {
+      var expectedArg = tests.args[arg];
+      var actualArg = output.args[arg];
+
+      if (Array.isArray(expectedArg)) {
+        if (!Array.isArray(actualArg)) {
+          test.ok(false, 'actual is not an array. ' + typed + '/' + arg);
+          return;
+        }
+
+        test.is(expectedArg.length, actualArg.length,
+                'array length: ' + typed + '/' + arg);
+        for (var i = 0; i < expectedArg.length; i++) {
+          test.is(expectedArg[i], actualArg[i],
+                  'member: "' + typed + '/' + arg + '/' + i);
+        }
+      }
+      else {
+        test.is(expectedArg, actualArg, 'typed: "' + typed + '" arg: ' + arg);
+      }
+    });
+  }
+
+  if (!options.window.document.createElement) {
+    test.log('skipping output tests (missing doc.createElement) for ' + typed);
+    return;
+  }
+
+  var div = options.window.document.createElement('div');
+  output.toDom(div);
+  var displayed = div.textContent.trim();
+
+  if (tests.outputMatch) {
+    function doTest(match, against) {
+      if (!match.test(against)) {
+        test.ok(false, "html output for " + typed + " against " + match.source);
+        console.log("Actual textContent");
+        console.log(against);
+      }
+    }
+    if (Array.isArray(tests.outputMatch)) {
+      tests.outputMatch.forEach(function(match) {
+        doTest(match, displayed);
+      });
+    }
+    else {
+      doTest(tests.outputMatch, displayed);
+    }
+  }
+
+  if (tests.blankOutput != null) {
+    if (!/^$/.test(displayed)) {
+      test.ok(false, "html for " + typed + " (textContent sent to info)");
+      console.log("Actual textContent");
+      console.log(displayed);
+    }
+  }
+};
+
+
+});
+/*
+ * Copyright 2009-2011 Mozilla Foundation and contributors
+ * Licensed under the New BSD license. See LICENSE.txt or:
+ * http://opensource.org/licenses/BSD-3-Clause
+ */
+
 define('gclitest/testCli', ['require', 'exports', 'module' , 'gcli/cli', 'gcli/types', 'gclitest/mockCommands', 'test/assert'], function(require, exports, module) {
 
 
 var Requisition = require('gcli/cli').Requisition;
 var Status = require('gcli/types').Status;
 var mockCommands = require('gclitest/mockCommands');
 
 var test = require('test/assert');
@@ -1732,201 +2005,16 @@ define('gclitest/testHelp', ['require', 
 
 });
 /*
  * Copyright 2009-2011 Mozilla Foundation and contributors
  * Licensed under the New BSD license. See LICENSE.txt or:
  * http://opensource.org/licenses/BSD-3-Clause
  */
 
-define('gclitest/helpers', ['require', 'exports', 'module' , 'test/assert'], function(require, exports, module) {
-
-
-var test = require('test/assert');
-
-/**
- * Check that we can parse command input.
- * Doesn't execute the command, just checks that we grok the input properly:
- *
- * helpers.status({
- *   // Test inputs
- *   typed: "ech",           // Required
- *   cursor: 3,              // Optional cursor position
- *
- *   // Thing to check
- *   status: "INCOMPLETE",   // One of "VALID", "ERROR", "INCOMPLETE"
- *   emptyParameters: [ "<message>" ], // Still to type
- *   directTabText: "o",     // Simple completion text
- *   arrowTabText: "",       // When the completion is not an extension
- *   markup: "VVVIIIEEE",    // What state should the error markup be in
- * });
- */
-exports.status = function(options, tests) {
-  var requisition = options.display.requisition;
-  var inputter = options.display.inputter;
-  var completer = options.display.completer;
-
-  if (tests.typed) {
-    inputter.setInput(tests.typed);
-  }
-  else {
-    test.ok(false, "Missing typed for " + JSON.stringify(tests));
-    return;
-  }
-
-  if (tests.cursor) {
-    inputter.setCursor(tests.cursor);
-  }
-
-  if (tests.status) {
-    test.is(requisition.getStatus().toString(), tests.status,
-            "status for " + tests.typed);
-  }
-
-  if (tests.emptyParameters != null) {
-    var realParams = completer.emptyParameters;
-    test.is(realParams.length, tests.emptyParameters.length,
-            'emptyParameters.length for \'' + tests.typed + '\'');
-
-    if (realParams.length === tests.emptyParameters.length) {
-      for (var i = 0; i < realParams.length; i++) {
-        test.is(realParams[i].replace(/\u00a0/g, ' '), tests.emptyParameters[i],
-                'emptyParameters[' + i + '] for \'' + tests.typed + '\'');
-      }
-    }
-  }
-
-  if (tests.markup) {
-    var cursor = tests.cursor ? tests.cursor.start : tests.typed.length;
-    var statusMarkup = requisition.getInputStatusMarkup(cursor);
-    var actualMarkup = statusMarkup.map(function(s) {
-      return Array(s.string.length + 1).join(s.status.toString()[0]);
-    }).join('');
-    test.is(tests.markup, actualMarkup, 'markup for ' + tests.typed);
-  }
-
-  if (tests.directTabText) {
-    test.is(completer.directTabText, tests.directTabText,
-            'directTabText for \'' + tests.typed + '\'');
-  }
-
-  if (tests.arrowTabText) {
-    test.is(completer.arrowTabText, ' \u00a0\u21E5 ' + tests.arrowTabText,
-            'arrowTabText for \'' + tests.typed + '\'');
-  }
-};
-
-/**
- * Execute a command:
- *
- * helpers.exec({
- *   // Test inputs
- *   typed: "echo hi",        // Optional, uses existing if undefined
- *
- *   // Thing to check
- *   args: { message: "hi" }, // Check that the args were understood properly
- *   outputMatch: /^hi$/,     // Regex to test against textContent of output
- *   blankOutput: true,       // Special checks when there is no output
- * });
- */
-exports.exec = function(options, tests) {
-  var requisition = options.display.requisition;
-  var inputter = options.display.inputter;
-
-  tests = tests || {};
-
-  if (tests.typed) {
-    inputter.setInput(tests.typed);
-  }
-
-  var typed = inputter.getInputState().typed;
-  var output = requisition.exec({ hidden: true });
-
-  test.is(typed, output.typed, 'output.command for: ' + typed);
-
-  if (tests.completed !== false) {
-    test.ok(output.completed, 'output.completed false for: ' + typed);
-  }
-  else {
-    // It is actually an error if we say something is async and it turns
-    // out not to be? For now we're saying 'no'
-    // test.ok(!output.completed, 'output.completed true for: ' + typed);
-  }
-
-  if (tests.args != null) {
-    test.is(Object.keys(tests.args).length, Object.keys(output.args).length,
-            'arg count for ' + typed);
-
-    Object.keys(output.args).forEach(function(arg) {
-      var expectedArg = tests.args[arg];
-      var actualArg = output.args[arg];
-
-      if (Array.isArray(expectedArg)) {
-        if (!Array.isArray(actualArg)) {
-          test.ok(false, 'actual is not an array. ' + typed + '/' + arg);
-          return;
-        }
-
-        test.is(expectedArg.length, actualArg.length,
-                'array length: ' + typed + '/' + arg);
-        for (var i = 0; i < expectedArg.length; i++) {
-          test.is(expectedArg[i], actualArg[i],
-                  'member: "' + typed + '/' + arg + '/' + i);
-        }
-      }
-      else {
-        test.is(expectedArg, actualArg, 'typed: "' + typed + '" arg: ' + arg);
-      }
-    });
-  }
-
-  if (!options.window.document.createElement) {
-    test.log('skipping output tests (missing doc.createElement) for ' + typed);
-    return;
-  }
-
-  var div = options.window.document.createElement('div');
-  output.toDom(div);
-  var displayed = div.textContent.trim();
-
-  if (tests.outputMatch) {
-    function doTest(match, against) {
-      if (!match.test(against)) {
-        test.ok(false, "html output for " + typed + " against " + match.source);
-        console.log("Actual textContent");
-        console.log(against);
-      }
-    }
-    if (Array.isArray(tests.outputMatch)) {
-      tests.outputMatch.forEach(function(match) {
-        doTest(match, displayed);
-      });
-    }
-    else {
-      doTest(tests.outputMatch, displayed);
-    }
-  }
-
-  if (tests.blankOutput != null) {
-    if (!/^$/.test(displayed)) {
-      test.ok(false, "html for " + typed + " (textContent sent to info)");
-      console.log("Actual textContent");
-      console.log(displayed);
-    }
-  }
-};
-
-
-});
-/*
- * Copyright 2009-2011 Mozilla Foundation and contributors
- * Licensed under the New BSD license. See LICENSE.txt or:
- * http://opensource.org/licenses/BSD-3-Clause
- */
-
 define('gclitest/testHistory', ['require', 'exports', 'module' , 'test/assert', 'gcli/history'], function(require, exports, module) {
 
 var test = require('test/assert');
 var History = require('gcli/history').History;
 
 exports.setup = function() {
 };
 
@@ -3998,22 +4086,23 @@ exports.testFindCssSelector = function(o
 
 });
 
 let testModuleNames = [
   'gclitest/index',
   'gclitest/suite',
   'test/examiner',
   'test/assert',
+  'gclitest/testCanon',
+  'gclitest/helpers',
   'gclitest/testCli',
   'gclitest/mockCommands',
   'gclitest/testCompletion',
   'gclitest/testExec',
   'gclitest/testHelp',
-  'gclitest/helpers',
   'gclitest/testHistory',
   'gclitest/testInputter',
   'gclitest/testIntro',
   'gclitest/testJs',
   'gclitest/testKeyboard',
   'gclitest/testPref',
   'gcli/commands/pref',
   'text!gcli/commands/pref_list_outer.html',
--- a/browser/devtools/webconsole/gcli.jsm
+++ b/browser/devtools/webconsole/gcli.jsm
@@ -1033,16 +1033,24 @@ canon.Parameter = Parameter;
 /**
  * Add a command to the canon of known commands.
  * This function is exposed to the outside world (via gcli/index). It is
  * documented in docs/index.md for all the world to see.
  * @param commandSpec The command and its metadata.
  * @return The new command
  */
 canon.addCommand = function addCommand(commandSpec) {
+  if (commands[commandSpec.name] != null) {
+    // Roughly canon.removeCommand() without the event call, which we do later
+    delete commands[commandSpec.name];
+    commandNames = commandNames.filter(function(test) {
+      return test !== commandSpec.name;
+    });
+  }
+
   var command = new Command(commandSpec);
   commands[commandSpec.name] = command;
   commandNames.push(commandSpec.name);
   commandNames.sort();
 
   canon.onCanonChange();
   return command;
 };
@@ -1050,16 +1058,18 @@ canon.addCommand = function addCommand(c
 /**
  * Remove an individual command. The opposite of #addCommand().
  * @param commandOrName Either a command name or the command itself.
  */
 canon.removeCommand = function removeCommand(commandOrName) {
   var name = typeof commandOrName === 'string' ?
           commandOrName :
           commandOrName.name;
+
+  // See start of canon.addCommand if changing this code
   delete commands[name];
   commandNames = commandNames.filter(function(test) {
     return test !== name;
   });
 
   canon.onCanonChange();
 };