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

Issue #2958 - Fix ab_active to populate cookies on first request #2959

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

johngian
Copy link
Contributor

@johngian johngian commented Oct 9, 2019

This PR fixes issue #2958

Proposed PR background

There was an issue on the way that we check if an experiment is active. ab_active helper instead of checking the g.current_experiments dict that we populate before each request, was checking the actual cookies. That means that on the first request ab_active was failing to give the proper result because cookies where not populated.

Changes introduced in the commit:

  • Fix ab_active behaviour
  • Change tests to work for the new behaviour
  • Minor cleanup/docstring changes on tests

I tested in 2 temp firefox containers and it looks like experiment cookies are loaded on first request.

…equest

* Cleanup tests: Instead of mocking `g.user` run `preprocess_request`
@johngian
Copy link
Contributor Author

johngian commented Oct 9, 2019

@karlcow @miketaylr r?

@miketaylr miketaylr self-requested a review October 9, 2019 16:58
Copy link
Member

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. In my testing yesterday, I was able to get expected values out of g.current_experiments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants