Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix A/B test timing bug #601

Merged
merged 2 commits into from Sep 21, 2020
Merged

Fix A/B test timing bug #601

merged 2 commits into from Sep 21, 2020

Conversation

@wlycdgr
Copy link
Member

@wlycdgr wlycdgr commented Sep 2, 2020

conf.save.enable_human-web is set during startup, which is intercepted by the dispatcher, which calls setupABTest, which calls setupHubPromoABTest. Because the call to setupABTest in the dispatcher.on handler is not then()-chained to an abtest.fetch call, setupHubPromoABTest may get called before the A/B tests have been fetched for the first time. The absence of either hub_midnight or hub_plain on the still-empty abtest.tests object would then have been incorrectly interpreted to mean that the user should be placed in the remaining test bucket.

I initially fixed this by putting the setupABTest call in the dispatcher.on handler in a then() clause of an abtest.fetch call, but I prefer the current fix because it avoids making an extra call to the A/B test server on every startup.

  • Have you followed the guidelines in CONTRIBUTING.md?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do?
  • Does your submission pass tests?
  • Did you lint your code prior to submission?
…fetched to avoid misinterpreting absense of 'hub_plain' or 'hub_midnight'
@wlycdgr wlycdgr requested review from jsignanini and zarembsky as code owners Sep 2, 2020
@wlycdgr wlycdgr requested review from christophertino and fcjr Sep 2, 2020
@leuryr leuryr mentioned this pull request Sep 14, 2020
5 of 5 tasks complete
@fcjr
Copy link
Member

@fcjr fcjr commented Sep 18, 2020

Looks to be working as expected

@fcjr
fcjr approved these changes Sep 18, 2020
@christophertino christophertino added this to the 8.5.3 milestone Sep 21, 2020
@christophertino christophertino merged commit 387f037 into develop Sep 21, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@christophertino christophertino deleted the GH-2145/bugfix branch Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants