Merge pull request #58 from HubSpot/v4

Bump planout.js major version to v4.0.0

CHANGELOG.md

@@ -1,3 +1,13 @@
+##Changes in 4.0##
+- Core compatible bundle fixes:
+  - Core compatible namespace allocations now match core reference namespace allocations from python version of planout.
+  - Core compatible interpreted experiment enrollment now matches core reference interpreted experiment enrollment from python version of planout.
+- Separated the concerns of planout core random operations, and the planout API. The planout.js API (experiment, assignment, namespace, etc) are now composed with the random operations they are passed. This change has no effect on the usage of planout.js and only affects the development experience for contributors.
+- Added ```planoutAPIFactory.js``` to keep planout bundles consistent, and to make it easier to compose new planout bundles with the random operations of choice.
+- Made experiment & namespace names required to fix https://github.com/HubSpot/PlanOut.js/issues/57
+- Fixes ```WeightedChoice``` with false-y choices.
+- Fixes tests on windows + running the travis tests on windows as well if this is possible.
+
 ##Changes in 3.0##
 - Separate out compat + non-compat bundles to reduce filesize of the main bundle by removing the BigNumber dependency (https://github.com/HubSpot/PlanOut.js/pull/29/). The default distribution on npm is the non-compat bundle, but the compat bundle is available in the `dist` folder
 - Move to Babel 6
@@ -15,11 +25,11 @@
 - PlanOut.js now comes packaged with an ExperimentSetup utility that allows the registering of inputs to an experiment after the experiment has already been defined. It currently only works with the Namespace class.
 
 ##Changes in 1.2.3##
-- Exposure now only gets logged on the get function call if it is a valid parameter as defined by getParamNames() 
+- Exposure now only gets logged on the get function call if it is a valid parameter as defined by getParamNames()
 
 * Changes in 1.2
 - The return value of assign now determines whether or not an exposure event will
   be automatically logged. If either a truth-y value or nothing is returned from assign, then exposure
   will be logged. If a false-y value (excluding undefined) is returned then exposure will not be logged.
 
-- getParams(experimentName) is now a function on the namespace class 
+- getParams(experimentName) is now a function on the namespace class

README.md

@@ -54,6 +54,7 @@ class MyExperiment extends PlanOut.Experiment {
 
   setup() {
     //set experiment name, etc.
+    this.setName('MyExperiment');
   }
 
   /*

__tests__/testExperiment.js

@@ -296,7 +296,7 @@ describe("Test the experiment module", function() {
         });
       }
     };
-    experimentTester(TestInterpretedExperiment);
+    experimentTester(TestInterpretedExperiment, true);
   });
 
   it('should not log exposure if "get" is called on a param not in the experiment', function() {

__tests__/testExperimentSetup.js

@@ -66,15 +66,15 @@ class Experiment2 extends BaseExperiment {
 
 class Experiment1Compat extends BaseExperimentCompat {
   assign(params, args) {
-    params.set('foo', new UniformChoice({'choices': ['a', 'b'], 'unit': args.userid}));
+    params.set('foo', new UniformChoiceCompat({'choices': ['a', 'b'], 'unit': args.userid}));
     params.set('paramVal', args.paramVal);
     params.set('funcVal', args.funcVal);
   }
 }
 
 class Experiment2Compat extends BaseExperimentCompat {
   assign(params, args) {
-    params.set('foobar', new UniformChoice({'choices': ['a', 'b'], 'unit': args.userid}));
+    params.set('foobar', new UniformChoiceCompat({'choices': ['a', 'b'], 'unit': args.userid}));
     params.set('paramVal', args.paramVal);
     params.set('funcVal', args.funcVal);
   }
@@ -107,8 +107,8 @@ class BaseTestNamespaceCompat extends NamespaceCompat.SimpleNamespace {
   }
 
   setupExperiments() {
-    this.addExperiment('Experiment1', Experiment1, 50);
-    this.addExperiment('Experiment2', Experiment2, 50);
+    this.addExperiment('Experiment1', Experiment1Compat, 50);
+    this.addExperiment('Experiment2', Experiment2Compat, 50);
   }
 };
 
@@ -146,8 +146,8 @@ describe("Test the experiment setup module", function() {
     }
 
     var namespace2 = new TestNamespace();
-    expect(namespace2.get('foo')).toEqual(undefined);
-    expect(namespace2.get('paramVal')).toEqual(undefined);
+    expect(namespace2.get('foo')).toBeUndefined();
+    expect(namespace2.get('paramVal')).toBeUndefined();
   });
 
   it('works with namespace scoped inputs (compat)', function() {
@@ -163,8 +163,8 @@ describe("Test the experiment setup module", function() {
     }
 
     var namespace2 = new TestNamespace();
-    expect(namespace2.get('foo')).toEqual(undefined);
-    expect(namespace2.get('paramVal')).toEqual(undefined);
+    expect(namespace2.get('foo')).toEqual('b');
+    expect(namespace2.get('paramVal')).toBeUndefined();
   });
 
   it('works with function inputs', function() {

__tests__/testInterpreter.js

@@ -1,6 +1,8 @@
 var Interpreter = require.requireActual('../dist/planout.js').Interpreter;
+var PlanOutOpCommutative = require.requireActual('../dist/planout.js').Ops.Base.PlanOutOpCommutative;
 var InterpreterCompat = require.requireActual('../dist/planout_core_compatible.js').Interpreter;
-var PlanOutOpCommutative = require('../es6/ops/base').PlanOutOpCommutative;
+var PlanOutOpCommutativeCompat = require.requireActual('../dist/planout_core_compatible.js').Ops.Base.PlanOutOpCommutative;
+
 
 class CustomOp extends PlanOutOpCommutative {
   commutativeExecute(values) {
@@ -12,7 +14,17 @@ class CustomOp extends PlanOutOpCommutative {
   }
 }
 
-var compiled = 
+class CustomOpCompat extends PlanOutOpCommutativeCompat {
+  commutativeExecute(values) {
+    var value = 0;
+    for (var i = 0; i < values.length; i++) {
+      value += values[i];
+    }
+    return value;
+  }
+}
+
+var compiled =
 {"op":"seq","seq":[{"op":"set","var":"group_size","value":{"choices":{"op":"array","values":[1,10]},"unit":{"op":"get","var":"userid"},"op":"uniformChoice"}},{"op":"set","var":"specific_goal","value":{"p":0.8,"unit":{"op":"get","var":"userid"},"op":"bernoulliTrial"}},{"op":"cond","cond":[{"if":{"op":"get","var":"specific_goal"},"then":{"op":"seq","seq":[{"op":"set","var":"ratings_per_user_goal","value":{"choices":{"op":"array","values":[8,16,32,64]},"unit":{"op":"get","var":"userid"},"op":"uniformChoice"}},{"op":"set","var":"ratings_goal","value":{"op":"product","values":[{"op":"get","var":"group_size"},{"op":"get","var":"ratings_per_user_goal"}]}}]}}]}]};
 var interpreterSalt = 'foo';
 
@@ -26,9 +38,9 @@ describe("Test interpreter", function() {
   it('should interpret properly (compat)', function() {
     var proc = new InterpreterCompat(compiled, interpreterSalt, { 'userid': 123454});
     expect(proc.getParams().specific_goal).toEqual(1);
-    expect(proc.getParams().ratings_goal).toEqual(64);
+    expect(proc.getParams().ratings_goal).toEqual(320);
   });
-  
+
   it('should allow overrides', function() {
     var proc = new Interpreter(compiled, interpreterSalt, { 'userid': 123454});
     proc.setOverrides({'specific_goal': 0});
@@ -111,7 +123,7 @@ describe("Test interpreter", function() {
   it('should register custom ops (compat)', function() {
     var customOpScript = {"op":"seq","seq":[{"op":"set","var":"x","value":{"values":[2,4],"op":"customOp"}}]};
     var proc = new InterpreterCompat(customOpScript, interpreterSalt, { userId: 123454 });
-    proc.registerCustomOperators({ 'customOp': CustomOp });
+    proc.registerCustomOperators({ 'customOp': CustomOpCompat });
     expect(proc.getParams()['x']).toEqual(6);
   });
 });

__tests__/testNamespace.js

@@ -1,12 +1,14 @@
-var Utils = require('../es6/lib/utils.js');
+// This is not exposed by the built library, so we load it directly here.
+var Utils = require.requireActual('../es6/lib/utils.js');
+
+// Load up our built library implementations for testing
 var Namespace = require.requireActual('../dist/planout.js').Namespace;
 var Experiment = require.requireActual('../dist/planout.js').Experiment;
 var ExperimentSetup = require.requireActual('../dist/planout.js').ExperimentSetup;
 var NamespaceCompat = require.requireActual('../dist/planout_core_compatible.js').Namespace;
 var ExperimentCompat = require.requireActual('../dist/planout_core_compatible.js').Experiment;
 var ExperimentSetupCompat = require.requireActual('../dist/planout_core_compatible.js').ExperimentSetup;
 
-
 class BaseExperiment extends Experiment {
   configureLogger() {
     return;
@@ -183,6 +185,12 @@ describe("Test namespace module", function() {
     validateLog("Experiment2");
     var segValidation = { Experiment1: 50, Experiment2: 50};
     validateSegments(namespace, segValidation);
+
+    expect(new TestNamespace({'userid': 'a'}).get('test')).toEqual(1);
+    expect(new TestNamespace({'userid': 'b'}).get('test')).toEqual(1);
+    expect(new TestNamespace({'userid': 'c'}).get('test')).toEqual(2);
+    expect(new TestNamespace({'userid': 'd'}).get('test')).toEqual(1);
+    expect(new TestNamespace({'userid': 'e'}).get('test')).toEqual(2);
   });
 
   it('Adds two segments correctly (compat)', function() {
@@ -202,6 +210,12 @@ describe("Test namespace module", function() {
     validateLog("Experiment2");
     var segValidation = { Experiment1: 50, Experiment2: 50};
     validateSegments(namespace, segValidation);
+
+    expect(new TestNamespace({'userid': 'a'}).get('test')).toEqual(2);
+    expect(new TestNamespace({'userid': 'b'}).get('test')).toEqual(2);
+    expect(new TestNamespace({'userid': 'c'}).get('test')).toEqual(1);
+    expect(new TestNamespace({'userid': 'd'}).get('test')).toEqual(2);
+    expect(new TestNamespace({'userid': 'e'}).get('test')).toEqual(2);
   });
 
   it('Can remove segment correctly', function() {
@@ -267,7 +281,7 @@ describe("Test namespace module", function() {
     var namespace = new TestNamespace({'userid': 'hi'});
     expect(namespace.get('test2')).toEqual(3);
     expect(globalLog.length).toEqual(1);
-    expect(namespace.get('test'));
+    expect(namespace.get('test')).toBeUndefined();
     validateLog("Experiment3");
   });
 
@@ -284,10 +298,10 @@ describe("Test namespace module", function() {
     }
 
     var namespace = new TestNamespace({'userid': 'hi'});
-    expect(namespace.get('test2')).toEqual(3);
+    expect(namespace.get('test')).toEqual(1);
     expect(globalLog.length).toEqual(1);
-    expect(namespace.get('test'));
-    validateLog("Experiment3");
+    expect(namespace.get('test2')).toBeUndefined();
+    validateLog("Experiment1");
   });
 
   it('Allow experiment overrides in SimpleNamespace', function() {

__tests__/testRandomOps.js

@@ -210,7 +210,7 @@ describe('Test randomization ops', function() {
     testDistributions();
 
     // Test that falsy choices don't get skipped
-    // null is omitted since it gets converted to undefined in Assignement.get()
+    // null is omitted since it gets converted to undefined in Assignment.get()
     var counts = {};
     counts[0] = 0;
     counts[1] = 0;
@@ -256,7 +256,7 @@ describe('Test randomization ops', function() {
     testDistributions();
 
     // Test that falsy choices don't get skipped
-    // null is omitted since it gets converted to undefined in Assignement.get()
+    // null is omitted since it gets converted to undefined in Assignment.get()
     var counts = {};
     counts[0] = 0;
     counts[1] = 0;

bower.json

@@ -1,7 +1,7 @@
 {
   "name": "planout",
   "main": "dist/planout.js",
-  "version": "3.0.2",
+  "version": "4.0.0",
   "homepage": "https://www.github.com/HubSpot/PlanOut.js",
   "author": "Guy Aridor <garidor@gmail.com>",
   "description": "A JavaScript port of Facebook's PlanOut Experimentation Framework",

build/index.js

@@ -1,19 +1,4 @@
-import Experiment from '../es6/experiment';
-import Interpreter from '../es6/interpreter';
-import Random from '../es6/ops/random';
-import Core from '../es6/ops/core';
-import * as Namespace from '../es6/namespace';
-import Assignment from '../es6/assignment';
-import ExperimentSetup from '../es6/experimentSetup';
+import planoutAPIFactory from './planoutAPIFactory';
+import * as Random from '../es6/ops/random';
 
-export default {
-  Namespace: Namespace,
-  Assignment: Assignment,
-  Interpreter: Interpreter,
-  Experiment: Experiment,
-  ExperimentSetup: ExperimentSetup,
-  Ops: {
-    Random: Random,
-    Core: Core
-  }
-};
+export default planoutAPIFactory({Random});

build/index_core_compatible.js

@@ -1,20 +1,4 @@
-import Experiment from '../es6/experiment';
-import Interpreter from '../es6/interpreter';
-import RandomPlanoutCoreCompatible from '../es6/ops/randomPlanoutCoreCompatible';
-import Core from '../es6/ops/core';
-import * as Namespace from '../es6/namespace';
-import Assignment from '../es6/assignment';
-import ExperimentSetup from '../es6/experimentSetup';
-
-export default {
-  Namespace: Namespace,
-  Assignment: Assignment,
-  Interpreter: Interpreter,
-  Experiment: Experiment,
-  ExperimentSetup: ExperimentSetup,
-  Ops: {
-    Random: RandomPlanoutCoreCompatible,
-    Core: Core
-  }
-};
+import planoutAPIFactory from './planoutAPIFactory';
+import * as Random from '../es6/ops/randomPlanoutCoreCompatible';
 
+export default planoutAPIFactory({Random});

build/planoutAPIFactory.js

@@ -0,0 +1,34 @@
+import provideAssignment from '../es6/assignment';
+import provideExperiment from '../es6/experiment';
+import * as ExperimentSetup from '../es6/experimentSetup';
+import provideInterpreter from '../es6/interpreter';
+import * as Base from '../es6/ops/base';
+import * as Core from '../es6/ops/core';
+import * as OpsUtils from '../es6/ops/utils';
+import provideNamespace from '../es6/namespace';
+
+export default ({
+  Random = null
+} = {}) => {
+  // Provide our operations to the OpsUtils module
+  OpsUtils.initializeOperators(Core, Random);
+
+  // Inject our Random and other dependencies into our modules
+  const Assignment = provideAssignment(Random);
+  const Experiment = provideExperiment(Assignment);
+  const Interpreter = provideInterpreter(OpsUtils, Assignment);
+  const Namespace = provideNamespace(Random, Assignment, Experiment);
+
+  return {
+    Assignment,
+    Experiment,
+    ExperimentSetup,
+    Interpreter,
+    Ops: {
+      Random,
+      Core,
+      Base
+    },
+    Namespace
+  };
+};