Merge pull request #15 from HubSpot/guy_check_return_val_of_assign

use the return val of assign to determine exposure logging + random other fixups
rawls238 · Sep 14, 2015 · 505248f · 505248f
2 parents 2672cd6 + 5ad9bb3
commit 505248f
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 182 deletions.
diff --git a/__tests__/testExperiment.js b/__tests__/testExperiment.js
@@ -4,6 +4,24 @@ var Experiment = require('../es6/experiment');
 
 var globalLog = [];
 
+class BaseExperiment extends Experiment {
+  configureLogger() {
+    return;
+  }
+  log(stuff) {
+    globalLog.push(stuff);
+  }
+  previouslyLogged() {
+    return;
+  }
+
+  getParamNames() {
+    return this.getDefaultParamNames();
+  }
+  setup() {
+    this.name = 'test_name';
+  }
+}
 
 describe("Test the experiment module", function() {
   var validateLog;
@@ -44,22 +62,7 @@ describe("Test the experiment module", function() {
   });
 
   it('should work with basic experiments', function() {
-    class TestVanillaExperiment extends Experiment {
-      configureLogger() {
-        return;
-      }
-      log(stuff) {
-        globalLog.push(stuff);
-      }
-      getParamNames() {
-        return this.getDefaultParamNames();
-      }
-      previouslyLogged() {
-        return;
-      }
-      setup() {
-        this.name = 'test_name';
-      }
+    class TestVanillaExperiment extends BaseExperiment {
       assign(params, args) {
         params.set('foo', new UniformChoice({'choices': ['a', 'b'], 'unit': args.i}));
       }
@@ -68,54 +71,18 @@ describe("Test the experiment module", function() {
   });
 
   it('should be able to disable an experiment', function() {
-    class TestVanillaExperiment extends Experiment {
-      configureLogger() {
-        return;
-      }
-      log(stuff) {
-        globalLog.push(stuff);
-      }
-      previouslyLogged() {
-        return;
-      }
-      getParamNames() {
-        return this.getDefaultParamNames();
-      }
-      setup() {
-        this.name = 'test_name';
-      }
+    class TestVanillaExperiment extends BaseExperiment {
       assign(params, args) {
         params.set('foo', new UniformChoice({'choices': ['a', 'b'], 'unit': args.i}));
-        this._inExperiment = false;
+        return false;
       }
     }
     experimentTester(TestVanillaExperiment, false);
   });
 
   it('should only assign once', function() {
 
-    class TestSingleAssignment extends Experiment {
-
-      configureLogger() {
-        return;
-      }
-
-      getParamNames() {
-        return this.getDefaultParamNames();
-      }
-
-      log(stuff) {
-        globalLog.push(stuff);
-      }
-
-      previouslyLogged() {
-        return;
-      }
-
-      setup() {
-        this.name = 'test_name';
-      }
-
+    class TestSingleAssignment extends BaseExperiment {
       assign(params, args) {
         params.set('foo', new UniformChoice({'choices': ['a', 'b'], 'unit': args.i}));
         var counter = args.counter;
@@ -134,28 +101,7 @@ describe("Test the experiment module", function() {
   });
 
   it('should be able to pull experiment parameters', function() {
-    class TestAssignmentRetrieval extends Experiment {
-
-      configureLogger() {
-        return;
-      }
-
-      getParamNames() {
-        return this.getDefaultParamNames();
-      }
-
-      log(stuff) {
-        globalLog.push(stuff);
-      }
-
-      previouslyLogged() {
-        return;
-      }
-
-      setup() {
-        this.name = 'test_name';
-      }
-
+    class TestAssignmentRetrieval extends BaseExperiment {
       assign(params, args) {
         params.set('foo', 'heya');
         if (false) {
@@ -164,27 +110,7 @@ describe("Test the experiment module", function() {
       }
     }
 
-    class TestAssignmentRetrieval2 extends Experiment {
-      configureLogger() {
-        return;
-      }
-
-      log(stuff) {
-        globalLog.push(stuff);
-      }
-
-      previouslyLogged() {
-        return;
-      }
-
-      getParamNames() {
-        return this.getDefaultParamNames();
-      }
-
-      setup() {
-        this.name = 'test_name';
-      }
-
+    class TestAssignmentRetrieval2 extends BaseExperiment {
       assign(params, args) {
         return;
       }
@@ -197,24 +123,7 @@ describe("Test the experiment module", function() {
   });
 
   it('should work with an interpreted experiment', function() {
-    class TestInterpretedExperiment extends Experiment {
-      configureLogger() {
-        return;
-      }
-      log(stuff) {
-        globalLog.push(stuff);
-      }
-      previouslyLogged() {
-        return;
-      }
-
-      getParamNames() {
-        return this.getDefaultParamNames();
-      }
-      setup() {
-        this.name = 'test_name';
-      }
-
+    class TestInterpretedExperiment extends BaseExperiment {
       assign(params, args) {
         var compiled = 
           {"op":"seq",

diff --git a/__tests__/testNamespace.js b/__tests__/testNamespace.js
@@ -2,80 +2,40 @@ var Namespace = require('../es6/namespace.js');
 var Experiment = require('../es6/experiment.js');
 var Utils = require('../es6/lib/utils.js');
 
-var globalLog = [];
-class Experiment1 extends Experiment {
+class BaseExperiment extends Experiment {
   configureLogger() {
     return;
   }
-
-  log(data) {
-    globalLog.push(data);
+  log(stuff) {
+    globalLog.push(stuff);
   }
-
   previouslyLogged() {
     return;
   }
 
   getParamNames() {
     return this.getDefaultParamNames();
   }
-
   setup() {
     this.name = 'test_name';
   }
+};
 
+var globalLog = [];
+class Experiment1 extends BaseExperiment {
   assign(params, args) {
     params.set('test', 1)
   }
 }
 
-class Experiment2 extends Experiment {
-  configureLogger() {
-    return;
-  }
-
-  setup() {
-    this.name = 'test_name';
-  }
-
-  previouslyLogged() {
-    return;
-  }
-
-  log(data) {
-    globalLog.push(data);
-  }
-
-  getParamNames() {
-    return this.getDefaultParamNames();
-  }
+class Experiment2 extends BaseExperiment {
 
   assign(params, args) {
     params.set('test', 2)
   }
 }
 
-class Experiment3 extends Experiment {
-  configureLogger() {
-    return;
-  }
-
-  setup() {
-    this.name = 'test_name';
-  }
-
-  previouslyLogged() {
-    return;
-  }
-
-  log(data) {
-    globalLog.push(data);
-  }
-
-  getParamNames() {
-    return this.getDefaultParamNames();
-  }
-
+class Experiment3 extends BaseExperiment {
   assign(params, args) {
     params.set("test2", 3)
   }

diff --git a/dist/planout.js b/dist/planout.js
@@ -184,7 +184,12 @@ return /******/ (function(modules) { // webpackBootstrap
 	    key: '_assign',
 	    value: function _assign() {
 	      this.configureLogger();
-	      this.assign(this._assignment, this.inputs);
+	      var assignVal = this.assign(this._assignment, this.inputs);
+	      if (assignVal || assignVal === undefined) {
+	        this._inExperiment = true;
+	      } else {
+	        this._inExperiment = false;
+	      }
 	      this._assigned = true;
 	    }
 	  }, {
@@ -314,7 +319,7 @@ return /******/ (function(modules) { // webpackBootstrap
 	  }, {
 	    key: 'logExposure',
 	    value: function logExposure(extras) {
-	      if (!this._inExperiment) {
+	      if (!this.inExperiment()) {
 	        return;
 	      }
 	      this._exposureLogged = true;
@@ -328,7 +333,7 @@ return /******/ (function(modules) { // webpackBootstrap
 	  }, {
 	    key: 'logEvent',
 	    value: function logEvent(eventType, extras) {
-	      if (!this._inExperiment) {
+	      if (!this.inExperiment()) {
 	        return;
 	      }
 
@@ -1535,16 +1540,16 @@ return /******/ (function(modules) { // webpackBootstrap
 
 	var _libUtilsJs = __webpack_require__(8);
 
-	var DefaultExperiment = (function (_Experiment) {
+	var DefaultExperiment = (function (_BaseExperiment) {
 	  function DefaultExperiment() {
 	    _classCallCheck(this, DefaultExperiment);
 
-	    if (_Experiment != null) {
-	      _Experiment.apply(this, arguments);
+	    if (_BaseExperiment != null) {
+	      _BaseExperiment.apply(this, arguments);
 	    }
 	  }
 
-	  _inherits(DefaultExperiment, _Experiment);
+	  _inherits(DefaultExperiment, _BaseExperiment);
 
 	  _createClass(DefaultExperiment, [{
 	    key: "configureLogger",
@@ -1574,7 +1579,7 @@ return /******/ (function(modules) { // webpackBootstrap
 	  }]);
 
 	  return DefaultExperiment;
-	})(_experimentJs2["default"]);
+	})(BaseExperiment);
 
 	var Namespace = (function () {
 	  function Namespace() {
@@ -2000,8 +2005,8 @@ return /******/ (function(modules) { // webpackBootstrap
 	      delete this._data[name];
 	    }
 	  }, {
-	    key: "to_string",
-	    value: function to_string() {
+	    key: "toString",
+	    value: function toString() {
 	      return String(this._data);
 	    }
 	  }, {

diff --git a/dist/planout.map.js b/dist/planout.map.js
diff --git a/dist/planout.min.js b/dist/planout.min.js
diff --git a/es6/assignment.js b/es6/assignment.js
@@ -77,7 +77,7 @@ class Assignment {
     delete this._data[name];
   }
 
-  to_string() {
+  toString() {
     return String(this._data);
   }
 

diff --git a/es6/experiment.js b/es6/experiment.js
@@ -53,7 +53,12 @@ class Experiment {
 
   _assign() {
     this.configureLogger();
-    this.assign(this._assignment, this.inputs);
+    var assignVal = this.assign(this._assignment, this.inputs);
+    if (assignVal || assignVal === undefined) {
+      this._inExperiment = true;
+    } else {
+      this._inExperiment = false;
+    }
     this._assigned = true;
   }
 
@@ -163,7 +168,7 @@ class Experiment {
   }
 
   logExposure(extras) {
-    if (!this._inExperiment) {
+    if (!this.inExperiment()) {
       return;
     }
     this._exposureLogged = true;
@@ -175,7 +180,7 @@ class Experiment {
   }
 
   logEvent(eventType, extras) {
-    if (!this._inExperiment) {
+    if (!this.inExperiment()) {
       return;
     }