runat-1128988: Remove isNoDom

Several tests were skipped when there wasn't a DOM to test against. This removes all those test skips. Given the recent fixes, most of the skips could just be removed. Some of the skips are not down to weaknesses in jsdom, but in NodeJS or the requisitionAutomator which is used only in NodeJS, so some of this is s/isNoDom/isNode. Some of the problems were confusion as to an id in the document that we were testing against. #gcli-input was removed some time ago, so we're now using #gcli-root which still exists. Finally I added a trivial converter for the 'pref list' command so it would work on the command line, and be tested properly. Signed-off-by: Joe Walker <[email protected]>
joewalker · Mar 5, 2015 · 30b612e · bgrins · Mar 13, 2015 · 30b612e
1 parent 709687c
commit 30b612e
Show file tree

Hide file tree

Showing 13 changed files with 52 additions and 49 deletions.
diff --git a/lib/gcli/commands/preflist.js b/lib/gcli/commands/preflist.js
@@ -22,7 +22,7 @@ var Promise = require('../util/promise').Promise;
 /**
  * Format a list of settings for display
  */
-var prefsData = {
+var prefsViewConverter = {
   item: 'converter',
   from: 'prefsData',
   to: 'view',
@@ -97,6 +97,22 @@ var prefsData = {
   }
 };
 
+/**
+ * Format a list of settings for display
+ */
+var prefsStringConverter = {
+  item: 'converter',
+  from: 'prefsData',
+  to: 'string',
+  exec: function(prefsData, conversionContext) {
+    var reply = '';
+    prefsData.settings.forEach(function(setting) {
+      reply += setting.name + ' -> ' + setting.value + '\n';
+    });
+    return reply;
+  }
+};
+
 /**
  * 'pref list' command
  */
@@ -194,4 +210,4 @@ PrefList.prototype.onSetClick = function(ev) {
   this.conversionContext.update(typed);
 };
 
-exports.items = [ prefsData, prefList ];
+exports.items = [ prefsViewConverter, prefsStringConverter, prefList ];
diff --git a/lib/gcli/commands/server/firefox.js b/lib/gcli/commands/server/firefox.js
@@ -133,7 +133,7 @@ function createCommonJsToJsTestFilter() {
         '\n' +
         'var exports = {};\n' +
         '\n' +
-        'var TEST_URI = "data:text/html;charset=utf-8,<p id=\'gcli-input\'>gcli-' + name + '</p>";\n' +
+        'var TEST_URI = "data:text/html;charset=utf-8,<p id=\'gcli-root\'>gcli-' + name + '</p>";\n' +
         '\n' +
         'function test() {\n' +
         '  return Task.spawn(*function() {\n' +

diff --git a/lib/gcli/commands/test.js b/lib/gcli/commands/test.js
@@ -74,7 +74,6 @@ exports.items = [
                    process.title.indexOf('node') != -1),
           isFirefox: false,
           isPhantomjs: false,
-          isNoDom: true,
           requisition: new Requisition(context.system)
         };
         options.automator = createRequisitionAutomator(options.requisition);

diff --git a/lib/gcli/test/helpers.js b/lib/gcli/test/helpers.js
@@ -380,34 +380,34 @@ helpers._check = function(options, name, checks) {
   var outstanding = [];
   var suffix = name ? ' (for \'' + name + '\')' : '';
 
-  if (!options.isNoDom && 'input' in checks) {
+  if (!options.isNode && 'input' in checks) {
     assert.is(helpers._actual.input(options), checks.input, 'input' + suffix);
   }
 
-  if (!options.isNoDom && 'cursor' in checks) {
+  if (!options.isNode && 'cursor' in checks) {
     assert.is(helpers._actual.cursor(options), checks.cursor, 'cursor' + suffix);
   }
 
-  if (!options.isNoDom && 'current' in checks) {
+  if (!options.isNode && 'current' in checks) {
     assert.is(helpers._actual.current(options), checks.current, 'current' + suffix);
   }
 
   if ('status' in checks) {
     assert.is(helpers._actual.status(options), checks.status, 'status' + suffix);
   }
 
-  if (!options.isNoDom && 'markup' in checks) {
+  if (!options.isNode && 'markup' in checks) {
     assert.is(helpers._actual.markup(options), checks.markup, 'markup' + suffix);
   }
 
-  if (!options.isNoDom && 'hints' in checks) {
+  if (!options.isNode && 'hints' in checks) {
     var hintCheck = function(actualHints) {
       assert.is(actualHints, checks.hints, 'hints' + suffix);
     };
     outstanding.push(helpers._actual.hints(options).then(hintCheck));
   }
 
-  if (!options.isNoDom && 'predictions' in checks) {
+  if (!options.isNode && 'predictions' in checks) {
     var predictionsCheck = function(actualPredictions) {
       helpers.arrayIs(actualPredictions,
                        checks.predictions,
@@ -416,7 +416,7 @@ helpers._check = function(options, name, checks) {
     outstanding.push(helpers._actual.predictions(options).then(predictionsCheck));
   }
 
-  if (!options.isNoDom && 'predictionsContains' in checks) {
+  if (!options.isNode && 'predictionsContains' in checks) {
     var containsCheck = function(actualPredictions) {
       checks.predictionsContains.forEach(function(prediction) {
         var index = actualPredictions.indexOf(prediction);
@@ -434,26 +434,26 @@ helpers._check = function(options, name, checks) {
   }
 
   /* TODO: Fix this
-  if (!options.isNoDom && 'tooltipState' in checks) {
+  if (!options.isNode && 'tooltipState' in checks) {
     assert.is(helpers._actual.tooltipState(options),
               checks.tooltipState,
               'tooltipState' + suffix);
   }
   */
 
-  if (!options.isNoDom && 'outputState' in checks) {
+  if (!options.isNode && 'outputState' in checks) {
     assert.is(helpers._actual.outputState(options),
               checks.outputState,
               'outputState' + suffix);
   }
 
-  if (!options.isNoDom && 'options' in checks) {
+  if (!options.isNode && 'options' in checks) {
     helpers.arrayIs(helpers._actual.options(options),
                      checks.options,
                      'options' + suffix);
   }
 
-  if (!options.isNoDom && 'error' in checks) {
+  if (!options.isNode && 'error' in checks) {
     assert.is(helpers._actual.message(options), checks.error, 'error' + suffix);
   }
 
@@ -515,7 +515,7 @@ helpers._check = function(options, name, checks) {
                   'arg.' + paramName + '.status' + suffix);
       }
 
-      if (!options.isNoDom && 'message' in check) {
+      if (!options.isNode && 'message' in check) {
         if (typeof check.message.test === 'function') {
           assert.ok(check.message.test(assignment.message),
                     'arg.' + paramName + '.message' + suffix);
@@ -573,7 +573,7 @@ helpers._exec = function(options, name, expected) {
 
       var context = requisition.conversionContext;
       var convertPromise;
-      if (options.isNoDom) {
+      if (options.isNode) {
         convertPromise = output.convert('string', context);
       }
       else {

diff --git a/lib/gcli/test/mockDocument.js b/lib/gcli/test/mockDocument.js
@@ -28,11 +28,11 @@ exports.setup = function() {
   var document = jsdom('' +
        '<html>' +
        '<head>' +
-       '  <style>#gcli-input { color: red; }</style>' +
+       '  <style>#gcli-root { color: red; }</style>' +
        '  <script>var t = 42;</script>' +
        '</head>' +
        '<body>' +
-       '<div id="gcli-input"></div>' +
+       '<div id="gcli-root"></div>' +
        '</body>' +
        '</html>');
 

diff --git a/lib/gcli/test/testCli2.js b/lib/gcli/test/testCli2.js
@@ -369,30 +369,22 @@ exports.testSingleFloat = function(options) {
 };
 
 exports.testElementWeb = function(options) {
-  var inputElement = options.isNoDom ?
-      null :
-      options.window.document.getElementById('gcli-input');
-
   return helpers.audit(options, [
     {
-      skipIf: function gcliInputElementExists() {
-        return inputElement == null;
-      },
-      setup:    'tse #gcli-input',
+      setup:    'tse #gcli-root',
       check: {
-        input:  'tse #gcli-input',
+        input:  'tse #gcli-root',
         hints:                 ' [options]',
-        markup: 'VVVVVVVVVVVVVVV',
-        cursor: 15,
+        markup: 'VVVVVVVVVVVVVV',
+        cursor: 14,
         current: 'node',
         status: 'VALID',
         predictions: [ ],
         unassigned: [ ],
         args: {
           command: { name: 'tse' },
           node: {
-            value: inputElement,
-            arg: ' #gcli-input',
+            arg: ' #gcli-root',
             status: 'VALID',
             message: ''
           },
@@ -407,7 +399,6 @@ exports.testElementWeb = function(options) {
 exports.testElement = function(options) {
   return helpers.audit(options, [
     {
-      skipRemainingIf: options.isNoDom,
       setup:    'tse',
       check: {
         input:  'tse',

diff --git a/lib/gcli/test/testDate.js b/lib/gcli/test/testDate.js
@@ -240,7 +240,7 @@ exports.testIncrDecr = function(options) {
   return helpers.audit(options, [
     {
       // createRequisitionAutomator doesn't fake UP/DOWN well enough
-      skipRemainingIf: options.isNoDom,
+      skipRemainingIf: options.isNode,
       setup:    'tsdate 2001-01-01<UP>',
       check: {
         input:  'tsdate 2001-01-02',

diff --git a/lib/gcli/test/testExec.js b/lib/gcli/test/testExec.js
@@ -375,7 +375,7 @@ exports.testExecScript = function(options) {
 exports.testExecNode = function(options) {
   return helpers.audit(options, [
     {
-      skipIf: options.isNoDom || options.isRemote,
+      skipIf: options.isRemote,
       setup:    'tse :root',
       check: {
         input:  'tse :root',

diff --git a/lib/gcli/test/testHelp.js b/lib/gcli/test/testHelp.js
@@ -77,8 +77,7 @@ exports.testHelpExec = function(options) {
   return helpers.audit(options, [
     {
       skipRemainingIf: function commandHelpMissing() {
-        return options.isNoDom ||
-               options.requisition.system.commands.get('help') == null;
+        return options.requisition.system.commands.get('help') == null;
       },
       setup: 'help',
       check: {
@@ -98,7 +97,7 @@ exports.testHelpExec = function(options) {
         args: { search: { value: 'nomatch' } }
       },
       exec: {
-        output: /No commands starting with 'nomatch'$/
+        output: /No commands starting with 'nomatch'/
       }
     },
     {
@@ -121,7 +120,7 @@ exports.testHelpExec = function(options) {
         args: { search: { value: 'a b' } }
       },
       exec: {
-        output: /No commands starting with 'a b'$/
+        output: /No commands starting with 'a b'/
       }
     },
     {

diff --git a/lib/gcli/test/testJs.js b/lib/gcli/test/testJs.js
@@ -23,7 +23,7 @@ var javascript = require('../types/javascript');
 var tempWindow;
 
 exports.setup = function(options) {
-  if (options.isNoDom) {
+  if (options.isNode) {
     return;
   }
 
@@ -40,7 +40,7 @@ exports.setup = function(options) {
 };
 
 exports.shutdown = function(options) {
-  if (options.isNoDom) {
+  if (options.isNode) {
     return;
   }
 
@@ -50,7 +50,7 @@ exports.shutdown = function(options) {
 };
 
 function jsTestAllowed(options) {
-  return options.isRemote || options.isNoDom ||
+  return options.isRemote || options.isNode ||
          options.requisition.system.commands.get('{') == null;
 }
 
@@ -324,7 +324,7 @@ exports.testDocument = function(options) {
 };
 
 exports.testDonteval = function(options) {
-  if (!options.isNoDom) {
+  if (!options.isNode) {
     // nodom causes an eval here, maybe that's node/v8?
     assert.ok('donteval' in options.window, 'donteval exists');
   }

diff --git a/lib/gcli/test/testNode.js b/lib/gcli/test/testNode.js
@@ -131,7 +131,6 @@ exports.testNode = function(options) {
 exports.testNodeDom = function(options) {
   return helpers.audit(options, [
     {
-      skipRemainingIf: options.isNoDom,
       setup:    'tse :root',
       check: {
         input:  'tse :root',

diff --git a/lib/gcli/test/testPref2.js b/lib/gcli/test/testPref2.js
@@ -93,8 +93,7 @@ exports.testPrefExec = function(options) {
     },
     {
       skipRemainingIf: function commandPrefListMissing() {
-        return options.isNoDom ||
-               options.requisition.system.commands.get('pref list') == null;
+        return options.requisition.system.commands.get('pref list') == null;
       },
       setup:    'pref list tempNum',
       check: {

diff --git a/lib/gcli/test/testResource.js b/lib/gcli/test/testResource.js
@@ -39,7 +39,7 @@ exports.shutdown = function(options) {
 };
 
 exports.testAllPredictions1 = function(options) {
-  if (options.isFirefox || options.isNoDom) {
+  if (options.isFirefox || options.isNode) {
     assert.log('Skipping checks due to firefox document.stylsheets support.');
     return;
   }
@@ -55,7 +55,7 @@ exports.testAllPredictions1 = function(options) {
 };
 
 exports.testScriptPredictions = function(options) {
-  if (options.isFirefox || options.isNoDom) {
+  if (options.isFirefox || options.isNode) {
     assert.log('Skipping checks due to firefox document.stylsheets support.');
     return;
   }
@@ -72,7 +72,7 @@ exports.testScriptPredictions = function(options) {
 };
 
 exports.testStylePredictions = function(options) {
-  if (options.isFirefox || options.isNoDom) {
+  if (options.isFirefox || options.isNode) {
     assert.log('Skipping checks due to firefox document.stylsheets support.');
     return;
   }