Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Fix #3674, put 50% of people into control #3723

Merged
merged 1 commit into from
Nov 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions server/src/ab-tests.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
/** A/B test assignment support

This assigns users to tests using the `abTests.updateAbTests(obj)` function

Tests are defined directly in this file, in the `allTests` variable. A comment
below shows what these test objects look like.
*/
// Note: these get turned into Test objects:
let allTests = {
};

/* Example of how this could be set (until we have real tests to serve as docs): */
/* Example of how this could be set: */
/*
let allTests = {
autoOpenSharePanel: {
Expand All @@ -13,14 +20,17 @@ let allTests = {
// this field will be set in the viewers events:
shotField: "cd4",
// If you make updates (like add an option) and increment this, then users
// who were previously in the control group may get put into a new group:
// who were previously excluded may get put into a new group:
version: 1,
// Keep the user in control if they aren't in control in any of these tests:
// Exclude the user if they are in any of these tests:
exclude: ["highlightButtonOnInstall", "myShotsDisplay"],
// Or exclude them if they are in any test:
// exclude: ["*"],
// These are the actual allowed A/B options (control is never specified):
options: [
// The name of the option, and its probabilty (e.g., 10% chance of getting
// into this group)
// into this group). This will cause 5% of people to be in autoopen, 5% of
// people to be in control, and 90% to be in exclude:
{name: "autoopen", probability: 0.1}
]
},
Expand All @@ -39,7 +49,8 @@ let allTests = {
version: 1,
exclude: ["highlightButtonOnInstall", "autoOpenSharePanel"],
options: [
// Note no one will end up in control in this example:
// Note no one will end up in exclude in this example (but 50% will still
// be control):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianb If I'm understanding this correctly, since 10% of the population is in the autoOpenSharePanel test, and another 10% is in the highlightButtonOnInstall test, and people in those tests are excluded from this myShotsDisplay test, then, 50% of the remaining total 80% of users will be in the control group in this test, while the other half will be 9:1 split between the 'intropopup' and 'blink' tests. Does that seem correct?

{name: "intropopup", probability: 0.9},
{name: "blink", probability: 0.1}
]
Expand Down Expand Up @@ -76,20 +87,25 @@ class Test {
return;
}
if (this.shouldExclude(tests)) {
tests[this.name] = this.testWithValue("control");
tests[this.name] = this.testWithValue("exclude");
} else {
let prob = getRandom();
let setAny = false;
for (let option of this.options) {
if (prob < option.probability) {
tests[this.name] = this.testWithValue(option.name)
let controlProb = getRandom();
if (controlProb < 0.5) {
tests[this.name] = this.testWithValue("control");
} else {
tests[this.name] = this.testWithValue(option.name)
}
setAny = true;
break;
}
prob -= option.probability;
}
if (!setAny) {
tests[this.name] = this.testWithValue("control");
tests[this.name] = this.testWithValue("exclude");
}
}
}
Expand All @@ -104,10 +120,17 @@ class Test {

shouldExclude(tests) {
for (let testName of this.exclude) {
if (tests[testName] && tests[testName].value !== "control") {
if (tests[testName] && tests[testName].value !== "exclude") {
return true;
}
}
if (this.exclude.includes("*")) {
for (let testName in tests) {
if (testName != this.name && tests[testName].value !== "exclude") {
return true;
}
}
}
return false;
}

Expand Down
32 changes: 21 additions & 11 deletions test/test-ab-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe("Test Screenshots", function() {
shotField: "cd5",
description: "Test with conflicts",
version: 2,
exclude: ["simpleTest"],
exclude: ["*"],
options: [
{
name: "fireworks",
Expand All @@ -47,15 +47,25 @@ describe("Test Screenshots", function() {
abTests.setRandomSequenceForTesting(undefined);
});

it("should set all to control when low probability", () => {
it("should set all to exclude when low probability", () => {
abTests.setRandomSequenceForTesting([0.9, 0.9]);
let tests = abTests.updateAbTests({});
assert.deepEqual(tests, {
simpleTest: {value: "exclude", gaField: "cd3", version: 1},
breakEverythingTest: {value: "exclude", gaField: "cd4", shotField: "cd5", version: 2}
});
});

it("should set some to control instead of a test", () => {
abTests.setRandomSequenceForTesting([0.1, 0.1]);
let tests = abTests.updateAbTests({});
assert.deepEqual(tests, {
simpleTest: {value: "control", gaField: "cd3", version: 1},
breakEverythingTest: {value: "control", gaField: "cd4", shotField: "cd5", version: 2}
breakEverythingTest: {value: "exclude", gaField: "cd4", shotField: "cd5", version: 2}
});
});


it("should not overwrite existing values", () => {
let tests = abTests.updateAbTests({
simpleTest: {value: "control", gaField: "cd3", version: 1},
Expand All @@ -73,24 +83,24 @@ describe("Test Screenshots", function() {
});
assert.deepEqual(tests, {
simpleTest: {value: "bright", gaField: "cd3", version: 1},
breakEverythingTest: {value: "control", gaField: "cd4", shotField: "cd5", version: 2}
breakEverythingTest: {value: "exclude", gaField: "cd4", shotField: "cd5", version: 2}
});
abTests.setRandomSequenceForTesting([0.35]);
abTests.setRandomSequenceForTesting([0.35, 0.75]);
tests = abTests.updateAbTests({});
assert.deepEqual(tests, {
simpleTest: {value: "dark", gaField: "cd3", version: 1},
breakEverythingTest: {value: "control", gaField: "cd4", shotField: "cd5", version: 2}
breakEverythingTest: {value: "exclude", gaField: "cd4", shotField: "cd5", version: 2}
});
});

it("should try to take someone out of control when version is bumped", () => {
abTests.setRandomSequenceForTesting([0.15]);
it("should try to take someone out of exclude when version is bumped", () => {
abTests.setRandomSequenceForTesting([0.15, 0.75]);
let tests = abTests.updateAbTests({
simpleTest: {value: "control", gaField: "cd3", version: 1},
breakEverythingTest: {value: "control", gaField: "cd4", shotField: "cd5", version: 1}
simpleTest: {value: "exclude", gaField: "cd3", version: 1},
breakEverythingTest: {value: "exclude", gaField: "cd4", shotField: "cd5", version: 1}
});
assert.deepEqual(tests, {
simpleTest: {value: "control", gaField: "cd3", version: 1},
simpleTest: {value: "exclude", gaField: "cd3", version: 1},
breakEverythingTest: {value: "strobe", gaField: "cd4", shotField: "cd5", version: 2}
});
});
Expand Down