fix(jsFlags): un-quote --js-flags flag and automatically merge with presets

jeffbcross · jeffbcross · commit 4509c2771d87 · 2014-12-01T21:58:06.000-08:00
diff --git a/index.js b/index.js
@@ -8,6 +8,10 @@ var ChromeBrowser = function(baseBrowserDecorator, args) {
   this._getOptions = function(url) {
     // Chrome CLI options
     // http://peter.sh/experiments/chromium-command-line-switches/
+    flags.forEach(function(flag, i) {
+      if(isJSFlags(flag)) flags[i] = sanitizeJSFlags(flag);
+    });
+
     return [
       '--user-data-dir=' + this._tempDir,
       '--no-default-browser-check',
@@ -19,6 +23,19 @@ var ChromeBrowser = function(baseBrowserDecorator, args) {
   };
 };
 
+function isJSFlags (flag) {
+  return flag.indexOf('--js-flags=') === 0;
+}
+
+function sanitizeJSFlags (flag) {
+  var test = /--js-flags=(['"])/.exec(flag);
+  if (!test) return flag;
+  var escapeChar = test[1];
+  var endExp = new RegExp(escapeChar + '$');
+  var startExp = new RegExp('--js-flags=' + escapeChar);
+  return flag.replace(startExp, '--js-flags=').replace(endExp, '');
+}
+
 // Return location of chrome.exe file for a given Chrome directory (available: "Chrome", "Chrome SxS").
 function getChromeExe(chromeDirName) {
   if (process.platform !== 'win32') {
@@ -58,11 +75,25 @@ var ChromeCanaryBrowser = function(baseBrowserDecorator, args) {
 
   var parentOptions = this._getOptions;
   this._getOptions = function(url) {
-    // disable crankshaft optimizations, as it causes lot of memory leaks (as of Chrome 23.0)
-    return parentOptions.call(this, url).concat(['--js-flags="--nocrankshaft --noopt"']);
+    return canaryGetOptions.call(this, url, args, parentOptions);
   };
 };
 
+function canaryGetOptions (url, args, parent) {
+  // disable crankshaft optimizations, as it causes lot of memory leaks (as of Chrome 23.0)
+  var flags = args.flags || [];
+  var augmentedFlags;
+  var customFlags = '--nocrankshaft --noopt';
+
+  flags.forEach(function(flag, i) {
+    if (isJSFlags(flag)) {
+      augmentedFlags = sanitizeJSFlags(flag) + ' ' + customFlags;
+    }
+  });
+
+  return parent.call(this, url).concat([augmentedFlags || '--js-flags=' + customFlags]);
+}
+
 ChromeCanaryBrowser.prototype = {
   name: 'ChromeCanary',
 
@@ -103,3 +134,9 @@ module.exports = {
   'launcher:ChromeCanary': ['type', ChromeCanaryBrowser],
   'launcher:Dartium': ['type', DartiumBrowser]
 };
+
+module.exports.test = {
+  isJSFlags: isJSFlags,
+  sanitizeJSFlags: sanitizeJSFlags,
+  canaryGetOptions: canaryGetOptions
+}
diff --git a/package.json b/package.json
@@ -4,7 +4,7 @@
   "description": "A Karma plugin. Launcher for Chrome and Chrome Canary.",
   "main": "index.js",
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "./node_modules/.bin/jasmine-node test"
   },
   "repository": {
     "type": "git",
@@ -25,7 +25,8 @@
     "grunt": "~0.4.1",
     "grunt-npm": "~0.0.2",
     "grunt-bump": "~0.0.6",
-    "grunt-auto-release": "~0.0.2"
+    "grunt-auto-release": "~0.0.2",
+    "jasmine-node": "~1.14.5"
   },
   "contributors": [
     "Friedel Ziegelmayer <friedel.ziegelmayer@gmail.com>",
diff --git a/test/jsflags.spec.js b/test/jsflags.spec.js
@@ -0,0 +1,51 @@
+var launcher = require('../index');
+
+describe('isJSFlags()', function() {
+  var isJSFlags = launcher.test.isJSFlags;
+
+  it('should return true if flag begins with --js-flags=', function() {
+    expect(isJSFlags('--js-flags=--expose-gc')).toBe(true);
+    expect(isJSFlags('--js-flags="--expose-gc"')).toBe(true);
+    expect(isJSFlags("--js-flags='--expose-gc'")).toBe(true);
+  });
+
+  it('should return false if flag does not begin with --js-flags=', function(){
+    expect(isJSFlags(' --js-flags=--expose-gc')).toBe(false);
+    expect(isJSFlags('--js-flags"--expose-gc"')).toBe(false);
+    expect(isJSFlags('--jsflags="--expose-gc"')).toBe(false);
+  });
+});
+
+
+describe('sanitizeJSFlags()', function() {
+  var sanitizeJSFlags = launcher.test.sanitizeJSFlags;
+
+  it('should do nothing if flags are not contained in quotes', function() {
+    expect(sanitizeJSFlags('--js-flags=--expose-gc')).toBe('--js-flags=--expose-gc');
+  });
+
+  it('should symmetrically remove single or double quote if wraps all flags', function() {
+    expect(sanitizeJSFlags("--js-flags='--expose-gc'")).toBe("--js-flags=--expose-gc");
+    expect(sanitizeJSFlags('--js-flags="--expose-gc"')).toBe('--js-flags=--expose-gc');
+  });
+
+  it('should NOT remove anything if the flags are not contained within quote', function() {
+    expect(sanitizeJSFlags('--js-flags=--expose-gc="true"')).toBe('--js-flags=--expose-gc="true"');
+    expect(sanitizeJSFlags("--js-flags=--expose-gc='true'")).toBe("--js-flags=--expose-gc='true'");
+  });
+});
+
+describe('canaryGetOptions', function() {
+  var canaryGetOptions = launcher.test.canaryGetOptions;
+
+  it('should return a merged version of --js-flags', function() {
+    var parent = jasmine.createSpy('parent').andReturn(['-incognito']);
+    var context = {};
+    var url = 'http://localhost:9876';
+    var args = {flags: ['--js-flags="--expose-gc"']};
+    expect(canaryGetOptions.call(context, url, args, parent)).toEqual([
+      '-incognito',
+      '--js-flags=--expose-gc --nocrankshaft --noopt'
+    ]);
+  });
+});
