diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..452ebb34 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,7 @@ +version: 2 +updates: +- package-ecosystem: bundler + directory: "/" + schedule: + interval: daily + open-pull-requests-limit: 10 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..f21dde75 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,74 @@ +name: split + +on: [push] + +jobs: + test: + strategy: + matrix: + include: + - gemfile: 5.2.gemfile + ruby: 2.5 + + - gemfile: 5.2.gemfile + ruby: 2.6 + + - gemfile: 5.2.gemfile + ruby: 2.7 + + - gemfile: 6.0.gemfile + ruby: 2.5 + + - gemfile: 6.0.gemfile + ruby: 2.6 + + - gemfile: 6.0.gemfile + ruby: 2.7 + + - gemfile: 6.0.gemfile + ruby: '3.0' + + - gemfile: 6.1.gemfile + ruby: '3.0' + + - gemfile: 7.0.gemfile + ruby: '3.0' + + - gemfile: 7.0.gemfile + ruby: '3.1' + + + runs-on: ubuntu-latest + + services: + redis: + image: redis + ports: ['6379:6379'] + options: >- + --health-cmd "redis-cli ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + steps: + - uses: actions/checkout@v3 + + - uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + + - name: Install dependencies + run: | + bundle config set gemfile "${GITHUB_WORKSPACE}/gemfiles/${{ matrix.gemfile }}" + bundle install --jobs 4 --retry 3 + + - name: Display Ruby version + run: ruby -v + + - name: Test + run: bundle exec rspec + env: + REDIS_URL: redis:6379 + + - name: Rubocop + run: bundle exec rubocop diff --git a/.rubocop.yml b/.rubocop.yml index 900eec30..b2b652fb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,180 @@ -inherit_from: .rubocop_todo.yml - AllCops: + TargetRubyVersion: 2.5 + DisabledByDefault: true + SuggestExtensions: false Exclude: - - 'Appraisals' - 'gemfiles/**/*' - - 'spec/**/*.rb' \ No newline at end of file + +Style/AndOr: + Enabled: true + +Layout/CaseIndentation: + Enabled: true + +Layout/ClosingHeredocIndentation: + Enabled: true + +Layout/CommentIndentation: + Enabled: true + +Layout/ElseAlignment: + Enabled: true + +Layout/EndAlignment: + Enabled: true + EnforcedStyleAlignWith: variable + AutoCorrect: true + +Layout/EmptyLineAfterMagicComment: + Enabled: true + +Layout/EmptyLinesAroundAccessModifier: + Enabled: true + EnforcedStyle: only_before + +Layout/EmptyLinesAroundBlockBody: + Enabled: true + +Layout/EmptyLinesAroundClassBody: + Enabled: true + +Layout/EmptyLinesAroundMethodBody: + Enabled: true + +Layout/EmptyLinesAroundModuleBody: + Enabled: true + +Style/HashSyntax: + Enabled: true + +Layout/FirstArgumentIndentation: + Enabled: true + +Layout/IndentationConsistency: + Enabled: true + EnforcedStyle: indented_internal_methods + +Layout/IndentationWidth: + Enabled: true + +Layout/LeadingCommentSpace: + Enabled: true + +Layout/SpaceAfterColon: + Enabled: true + +Layout/SpaceAfterComma: + Enabled: true + +Layout/SpaceAfterSemicolon: + Enabled: true + +Layout/SpaceAroundEqualsInParameterDefault: + Enabled: true + +Layout/SpaceAroundKeyword: + Enabled: true + +Layout/SpaceBeforeComma: + Enabled: true + +Layout/SpaceBeforeComment: + Enabled: true + +Layout/SpaceBeforeFirstArg: + Enabled: true + +Style/DefWithParentheses: + Enabled: true + +Style/MethodDefParentheses: + Enabled: true + +Style/FrozenStringLiteralComment: + Enabled: true + EnforcedStyle: always + +Style/RedundantFreeze: + Enabled: true + +Layout/SpaceBeforeBlockBraces: + Enabled: true + +Layout/SpaceInsideBlockBraces: + Enabled: true + EnforcedStyleForEmptyBraces: space + +Layout/SpaceInsideHashLiteralBraces: + Enabled: true + +Layout/SpaceInsideParens: + Enabled: true + +Style/StringLiterals: + Enabled: true + EnforcedStyle: double_quotes + +Layout/IndentationStyle: + Enabled: true + +Layout/TrailingEmptyLines: + Enabled: true + +Layout/TrailingWhitespace: + Enabled: true + +Style/RedundantPercentQ: + Enabled: true + +Lint/AmbiguousOperator: + Enabled: true + +Lint/AmbiguousRegexpLiteral: + Enabled: true + +Lint/ErbNewArguments: + Enabled: true + +Lint/RequireParentheses: + Enabled: true + +Lint/ShadowingOuterLocalVariable: + Enabled: true + +Lint/RedundantStringCoercion: + Enabled: true + +Lint/UriEscapeUnescape: + Enabled: true + +Lint/UselessAssignment: + Enabled: true + +Lint/DeprecatedClassMethods: + Enabled: true + +Style/ParenthesesAroundCondition: + Enabled: true + +Style/HashTransformKeys: + Enabled: true + +Style/HashTransformValues: + Enabled: true + +Style/RedundantBegin: + Enabled: true + +Style/RedundantReturn: + Enabled: true + AllowMultipleReturnValues: true + +Style/Semicolon: + Enabled: true + AllowAsExpressionSeparator: true + +Style/ColonMethodCall: + Enabled: true + +Style/TrivialAccessors: + Enabled: true diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml deleted file mode 100644 index 7a554e3a..00000000 --- a/.rubocop_todo.yml +++ /dev/null @@ -1,679 +0,0 @@ -# This configuration was generated by -# `rubocop --auto-gen-config` -# on 2019-10-21 18:09:14 -0300 using RuboCop version 0.75.1. -# The point is for the user to remove these configuration records -# one by one as the offenses are removed from the code base. -# Note that changes in the inspected code, or installation of new -# versions of RuboCop, may require this file to be generated again. - -# Offense count: 4 -# Cop supports --auto-correct. -# Configuration parameters: TreatCommentsAsGroupSeparators, Include. -# Include: **/*.gemspec -Gemspec/OrderedDependencies: - Exclude: - - 'split.gemspec' - -# Offense count: 1 -# Configuration parameters: Include. -# Include: **/*.gemspec, -Gemspec/RequiredRubyVersion: - Exclude: - - 'split.gemspec' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, IndentationWidth. -# SupportedStyles: with_first_argument, with_fixed_indentation -Layout/AlignArguments: - Exclude: - - 'lib/split.rb' - - 'lib/split/experiment_catalog.rb' - -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: AllowMultipleStyles, EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle. -# SupportedHashRocketStyles: key, separator, table -# SupportedColonStyles: key, separator, table -# SupportedLastArgumentHashStyles: always_inspect, always_ignore, ignore_implicit, ignore_explicit -Layout/AlignHash: - Exclude: - - 'lib/split/helper.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Layout/CommentIndentation: - Exclude: - - 'lib/split/experiment.rb' - -# Offense count: 5 -# Cop supports --auto-correct. -Layout/ElseAlignment: - Exclude: - - 'lib/split.rb' - - 'lib/split/helper.rb' - - 'lib/split/trial.rb' - -# Offense count: 20 -# Cop supports --auto-correct. -Layout/EmptyLineAfterGuardClause: - Exclude: - - 'lib/split.rb' - - 'lib/split/algorithms/weighted_sample.rb' - - 'lib/split/alternative.rb' - - 'lib/split/combined_experiments_helper.rb' - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/goals_collection.rb' - - 'lib/split/helper.rb' - - 'lib/split/user.rb' - -# Offense count: 25 -# Cop supports --auto-correct. -Layout/EmptyLineAfterMagicComment: - Enabled: false - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: AllowAdjacentOneLineDefs, NumberOfEmptyLines. -Layout/EmptyLineBetweenDefs: - Exclude: - - 'lib/split/helper.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Layout/EmptyLines: - Exclude: - - 'lib/split/helper.rb' - -# Offense count: 8 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: empty_lines, empty_lines_except_namespace, empty_lines_special, no_empty_lines, beginning_only, ending_only -Layout/EmptyLinesAroundClassBody: - Exclude: - - 'lib/split/experiment_catalog.rb' - - 'lib/split/goals_collection.rb' - - 'lib/split/metric.rb' - - 'lib/split/persistence/cookie_adapter.rb' - - 'lib/split/persistence/redis_adapter.rb' - - 'lib/split/persistence/session_adapter.rb' - - 'lib/split/zscore.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -Layout/EmptyLinesAroundMethodBody: - Exclude: - - 'lib/split/dashboard/helpers.rb' - - 'lib/split/zscore.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: empty_lines, empty_lines_except_namespace, empty_lines_special, no_empty_lines -Layout/EmptyLinesAroundModuleBody: - Exclude: - - 'lib/split/encapsulated_helper.rb' - -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyleAlignWith, AutoCorrect, Severity. -# SupportedStylesAlignWith: keyword, variable, start_of_line -Layout/EndAlignment: - Exclude: - - 'lib/split.rb' - - 'lib/split/helper.rb' - - 'lib/split/trial.rb' - -# Offense count: 4 -# Cop supports --auto-correct. -# Configuration parameters: AllowForAlignment, AllowBeforeTrailingComments, ForceEqualSignAlignment. -Layout/ExtraSpacing: - Exclude: - - 'lib/split/algorithms/whiplash.rb' - - 'lib/split/dashboard.rb' - - 'lib/split/metric.rb' - - 'lib/split/trial.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, IndentationWidth. -# SupportedStyles: special_inside_parentheses, consistent, align_braces -Layout/IndentFirstHashElement: - Exclude: - - 'split.gemspec' - -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: Width, IgnoredPatterns. -Layout/IndentationWidth: - Exclude: - - 'lib/split.rb' - - 'lib/split/helper.rb' - - 'lib/split/trial.rb' - -# Offense count: 10 -# Cop supports --auto-correct. -Layout/SpaceAfterComma: - Exclude: - - 'lib/split/algorithms/weighted_sample.rb' - - 'lib/split/configuration.rb' - - 'lib/split/encapsulated_helper.rb' - - 'lib/split/experiment.rb' - - 'lib/split/metric.rb' - - 'lib/split/user.rb' - -# Offense count: 5 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: space, no_space -Layout/SpaceAroundEqualsInParameterDefault: - Exclude: - - 'lib/split/goals_collection.rb' - - 'lib/split/persistence/dual_adapter.rb' - - 'lib/split/persistence/redis_adapter.rb' - - 'lib/split/trial.rb' - - 'lib/split/user.rb' - -# Offense count: 24 -# Cop supports --auto-correct. -# Configuration parameters: AllowForAlignment. -Layout/SpaceAroundOperators: - Exclude: - - 'lib/split/algorithms/whiplash.rb' - - 'lib/split/alternative.rb' - - 'lib/split/metric.rb' - - 'lib/split/trial.rb' - - 'lib/split/zscore.rb' - -# Offense count: 14 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces. -# SupportedStyles: space, no_space -# SupportedStylesForEmptyBraces: space, no_space -Layout/SpaceBeforeBlockBraces: - Exclude: - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/helper.rb' - - 'lib/split/trial.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBrackets. -# SupportedStyles: space, no_space, compact -# SupportedStylesForEmptyBrackets: space, no_space -Layout/SpaceInsideArrayLiteralBrackets: - Exclude: - - 'lib/split/dashboard/helpers.rb' - -# Offense count: 31 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces, SpaceBeforeBlockParameters. -# SupportedStyles: space, no_space -# SupportedStylesForEmptyBraces: space, no_space -Layout/SpaceInsideBlockBraces: - Exclude: - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/helper.rb' - - 'lib/split/trial.rb' - - 'lib/split/user.rb' - -# Offense count: 10 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces. -# SupportedStyles: space, no_space, compact -# SupportedStylesForEmptyBraces: space, no_space -Layout/SpaceInsideHashLiteralBraces: - Exclude: - - 'lib/split/experiment.rb' - - 'lib/split/helper.rb' - - 'lib/split/persistence/redis_adapter.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: final_newline, final_blank_line -Layout/TrailingBlankLines: - Exclude: - - 'Rakefile' - -# Offense count: 8 -# Configuration parameters: AllowSafeAssignment. -Lint/AssignmentInCondition: - Exclude: - - 'lib/split/combined_experiments_helper.rb' - - 'lib/split/configuration.rb' - - 'lib/split/persistence/cookie_adapter.rb' - - 'lib/split/persistence/dual_adapter.rb' - - 'lib/split/persistence/redis_adapter.rb' - -# Offense count: 4 -# Cop supports --auto-correct. -# Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. -Lint/UnusedBlockArgument: - Exclude: - - 'lib/split/algorithms/block_randomization.rb' - - 'lib/split/configuration.rb' - - 'lib/split/engine.rb' - - 'lib/split/metric.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods. -Lint/UnusedMethodArgument: - Exclude: - - 'lib/split/dashboard/helpers.rb' - - 'lib/split/persistence/cookie_adapter.rb' - -# Offense count: 1 -Lint/UselessAssignment: - Exclude: - - 'lib/split/goals_collection.rb' - -# Offense count: 19 -Metrics/AbcSize: - Max: 47 - -# Offense count: 3 -# Configuration parameters: CountComments. -Metrics/ClassLength: - Max: 377 - -# Offense count: 6 -Metrics/CyclomaticComplexity: - Max: 14 - -# Offense count: 23 -# Configuration parameters: CountComments, ExcludedMethods. -Metrics/MethodLength: - Max: 66 - -# Offense count: 1 -# Configuration parameters: CountComments. -Metrics/ModuleLength: - Max: 134 - -# Offense count: 8 -Metrics/PerceivedComplexity: - Max: 16 - -# Offense count: 4 -Naming/AccessorMethodName: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/experiment.rb' - - 'lib/split/persistence/cookie_adapter.rb' - -# Offense count: 1 -Naming/BinaryOperatorParameterName: - Exclude: - - 'lib/split/experiment.rb' - -# Offense count: 4 -# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist, MethodDefinitionMacros. -# NamePrefix: is_, has_, have_ -# NamePrefixBlacklist: is_, has_, have_ -# NameWhitelist: is_a? -# MethodDefinitionMacros: define_method, define_singleton_method -Naming/PredicateName: - Exclude: - - 'spec/**/*' - - 'lib/split/experiment.rb' - - 'lib/split/helper.rb' - -# Offense count: 5 -# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames. -# AllowedNames: io, id, to, by, on, in, at, ip, db -Naming/UncommunicativeMethodParamName: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/zscore.rb' - -# Offense count: 6 -# Configuration parameters: EnforcedStyle. -# SupportedStyles: snake_case, normalcase, non_integer -Naming/VariableNumber: - Exclude: - - 'lib/split/zscore.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: always, conditionals -Style/AndOr: - Exclude: - - 'lib/split/experiment_catalog.rb' - -# Offense count: 2 -# Configuration parameters: AllowedChars. -Style/AsciiComments: - Exclude: - - 'lib/split/zscore.rb' - -# Offense count: 4 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: percent_q, bare_percent -Style/BarePercentLiterals: - Exclude: - - 'lib/split/dashboard/pagination_helpers.rb' - -# Offense count: 8 -Style/CaseEquality: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/helper.rb' - - 'lib/split/metric.rb' - -# Offense count: 5 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: is_a?, kind_of? -Style/ClassCheck: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - - 'lib/split/goals_collection.rb' - - 'lib/split/trial.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Style/ColonMethodCall: - Exclude: - - 'lib/split/combined_experiments_helper.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: Keywords. -# Keywords: TODO, FIXME, OPTIMIZE, HACK, REVIEW -Style/CommentAnnotation: - Exclude: - - 'lib/split/configuration.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, SingleLineConditionsOnly, IncludeTernaryExpressions. -# SupportedStyles: assign_to_condition, assign_inside_condition -Style/ConditionalAssignment: - Exclude: - - 'lib/split/dashboard.rb' - - 'lib/split/persistence/redis_adapter.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Style/DefWithParentheses: - Exclude: - - 'lib/split/helper.rb' - -# Offense count: 29 -Style/Documentation: - Enabled: false - -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: empty, nil, both -Style/EmptyElse: - Exclude: - - 'lib/split/experiment.rb' - - 'lib/split/metric.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Style/Encoding: - Exclude: - - 'split.gemspec' - -# Offense count: 1 -# Cop supports --auto-correct. -Style/ExpandPathArguments: - Exclude: - - 'split.gemspec' - -# Offense count: 13 -# Configuration parameters: MinBodyLength. -Style/GuardClause: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - - 'lib/split/goals_collection.rb' - - 'lib/split/helper.rb' - - 'lib/split/persistence/dual_adapter.rb' - - 'lib/split/trial.rb' - -# Offense count: 25 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols. -# SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys -Style/HashSyntax: - Exclude: - - 'Rakefile' - - 'lib/split.rb' - - 'lib/split/experiment.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/helper.rb' - - 'lib/split/metric.rb' - - 'lib/split/persistence.rb' - - 'lib/split/persistence/redis_adapter.rb' - -# Offense count: 4 -Style/IdenticalConditionalBranches: - Exclude: - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - -# Offense count: 14 -# Cop supports --auto-correct. -Style/IfUnlessModifier: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/experiment.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/goals_collection.rb' - - 'lib/split/metric.rb' - - 'lib/split/trial.rb' - - 'lib/split/user.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: require_parentheses, require_no_parentheses, require_no_parentheses_except_multiline -Style/MethodDefParentheses: - Exclude: - - 'lib/split/configuration.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, Autocorrect. -# SupportedStyles: module_function, extend_self -Style/ModuleFunction: - Exclude: - - 'lib/split.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: literals, strict -Style/MutableConstant: - Exclude: - - 'lib/split/experiment.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: both, prefix, postfix -Style/NegatedIf: - Exclude: - - 'lib/split/user.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: IncludeSemanticChanges. -Style/NonNilCheck: - Exclude: - - 'lib/split/configuration.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Style/Not: - Exclude: - - 'lib/split/experiment_catalog.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: Strict. -Style/NumericLiterals: - MinDigits: 9 - -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, EnforcedStyle, IgnoredMethods. -# SupportedStyles: predicate, comparison -Style/NumericPredicate: - Exclude: - - 'spec/**/*' - - 'lib/split/experiment.rb' - - 'lib/split/trial.rb' - - 'lib/split/user.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -Style/ParallelAssignment: - Exclude: - - 'lib/split/experiment_catalog.rb' - - 'lib/split/persistence/cookie_adapter.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: short, verbose -Style/PreferredHashMethods: - Exclude: - - 'lib/split/configuration.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: compact, exploded -Style/RaiseArgs: - Exclude: - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - -# Offense count: 6 -# Cop supports --auto-correct. -Style/RedundantParentheses: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/zscore.rb' - -# Offense count: 11 -# Cop supports --auto-correct. -# Configuration parameters: AllowMultipleReturnValues. -Style/RedundantReturn: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/experiment.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/helper.rb' - - 'lib/split/metric.rb' - - 'lib/split/zscore.rb' - -# Offense count: 21 -# Cop supports --auto-correct. -Style/RedundantSelf: - Exclude: - - 'lib/split.rb' - - 'lib/split/alternative.rb' - - 'lib/split/configuration.rb' - - 'lib/split/experiment.rb' - - 'lib/split/extensions/string.rb' - - 'lib/split/metric.rb' - - 'lib/split/persistence/dual_adapter.rb' - - 'lib/split/persistence/redis_adapter.rb' - - 'lib/split/trial.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -Style/RescueModifier: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/configuration.rb' - -# Offense count: 5 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: implicit, explicit -Style/RescueStandardError: - Exclude: - - 'lib/split/alternative.rb' - - 'lib/split/experiment.rb' - - 'lib/split/helper.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: ConvertCodeThatCanStartToReturnNil, Whitelist. -# Whitelist: present?, blank?, presence, try, try! -Style/SafeNavigation: - Exclude: - - 'lib/split/configuration.rb' - - 'lib/split/helper.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: AllowAsExpressionSeparator. -Style/Semicolon: - Exclude: - - 'lib/split/algorithms/whiplash.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: . -# SupportedStyles: use_perl_names, use_english_names -Style/SpecialGlobalVars: - EnforcedStyle: use_perl_names - -# Offense count: 86 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline. -# SupportedStyles: single_quotes, double_quotes -Style/StringLiterals: - Enabled: false - -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: IgnoredMethods. -# IgnoredMethods: respond_to, define_method -Style/SymbolProc: - Exclude: - - 'lib/split/experiment.rb' - - 'lib/split/experiment_catalog.rb' - - 'lib/split/metric.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: AllowNamedUnderscoreVariables. -Style/TrailingUnderscoreVariable: - Exclude: - - 'lib/split/helper.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Style/ZeroLengthPredicate: - Exclude: - - 'lib/split/user.rb' - -# Offense count: 74 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. -# URISchemes: http, https -Metrics/LineLength: - Max: 183 diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index c06d4c08..00000000 --- a/.travis.yml +++ /dev/null @@ -1,40 +0,0 @@ -language: ruby -rvm: - - 2.2.2 - - 2.3.8 - - 2.4.9 - - 2.5.7 - - 2.6.5 - -gemfile: - - gemfiles/5.0.gemfile - - gemfiles/5.1.gemfile - - gemfiles/5.2.gemfile - - gemfiles/6.0.gemfile - -matrix: - exclude: - - rvm: 2.2.2 - gemfile: gemfiles/6.0.gemfile - - rvm: 2.3.8 - gemfile: gemfiles/6.0.gemfile - - rvm: 2.4.9 - gemfile: gemfiles/6.0.gemfile - -before_install: - - gem uninstall -v '>= 2' -i $(rvm gemdir)@global -ax bundler || true - - gem install bundler --version=1.17.3 - -before_script: - - curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter - - chmod +x ./cc-test-reporter - - ./cc-test-reporter before-build - -script: - - RAILS_ENV=test bundle exec rake spec - -after_script: - - ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT - -cache: bundler -sudo: false diff --git a/Appraisals b/Appraisals deleted file mode 100644 index 7c855c23..00000000 --- a/Appraisals +++ /dev/null @@ -1,19 +0,0 @@ -appraise "4.2" do - gem "rails", "~> 4.2" -end - -appraise "5.0" do - gem "rails", "~> 5.0" -end - -appraise "5.1" do - gem "rails", "~> 5.1" -end - -appraise "5.2" do - gem "rails", "~> 5.2" -end - -appraise "6.0" do - gem 'rails', '~> 6.0' -end diff --git a/CHANGELOG.md b/CHANGELOG.md index 030e3311..b4567e44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,39 @@ +## 4.0.1 (December 30th, 2021) + +Bugfixes: +- ab_test must return metadata on error or if split is disabled/excluded user (@andrehjr, #622) +- Fix versioned experiments when used with allow_multiple_experiments=control (@andrehjr, #613) +- Only block Pinterest bot (@huoxito, #606) +- Respect experiment defaults when loading experiments in initializer. (@mattwd7, #599) +- Removes metadata key when it updated to nil (@andrehjr, #633) +- Force experiment does not count for metrics (@andrehjr, #637) +- Fix cleanup_old_versions! misbehaviour (@serggl, #661) + +Features: +- Make goals accessible via on_trial_complete callbacks (@robin-phung, #625) +- Replace usage of SimpleRandom with RubyStats(Used for Beta Distribution RNG) (@andrehjr, #616) +- Introduce enable/disable experiment cohorting (@robin-phung, #615) +- Add on_experiment_winner_choose callback (@GenaMinenkov, #574) +- Add Split::Cache to reduce load on Redis (@rdh, #648) +- Caching based optimization in the experiment#save path (@amangup, #652) +- Adds config option for cookie domain (@joedelia, #664) + +Misc: +- Drop support for Ruby < 2.5 (@andrehjr, #627) +- Drop support for Rails < 5 (@andrehjr, #607) +- Bump minimum required redis to 4.2 (@andrehjr, #628) +- Removed repeated loading from config (@robin-phung, #619) +- Simplify RedisInterface usage when persisting Experiment alternatives (@andrehjr, #632) +- Remove redis_url impl. Deprecated on version 2.2 (@andrehjr, #631) +- Remove thread_safe config as redis-rb is thread_safe by default (@andrehjr, #630) +- Fix typo of in `Split::Trial` class variable (TomasBarry, #644) +- Single HSET to update values, instead of multiple ones (@andrehjr, #640) +- Use Redis#hmset to keep compatibility with Redis < 4.0 (@andrehjr, #659) +- Remove 'set' parsing for alternatives. Sets were used as storage and deprecated on 0.x (@andrehjr, #639) +- Adding documentation related to what is stored on cookies. (@andrehjr, #634) +- Keep railtie defined under the Split gem namespace (@avit, #666) +- Update RSpec helper to support block syntax (@clowder, #665) + ## 3.4.1 (November 12th, 2019) Bugfixes: diff --git a/Gemfile b/Gemfile index 6e50cfec..9fdc310a 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,9 @@ # frozen_string_literal: true + source "https://rubygems.org" gemspec -gem "appraisal" +gem "rubocop", require: false +gem "matrix" gem "codeclimate-test-reporter" diff --git a/README.md b/README.md index 960c5de7..a0d3aa76 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # [Split](https://libraries.io/rubygems/split) [![Gem Version](https://badge.fury.io/rb/split.svg)](http://badge.fury.io/rb/split) -[![Build Status](https://secure.travis-ci.org/splitrb/split.svg?branch=master)](https://travis-ci.org/splitrb/split) +![Build status](https://github.com/splitrb/split/actions/workflows/ci.yml/badge.svg?branch=main) [![Code Climate](https://codeclimate.com/github/splitrb/split/badges/gpa.svg)](https://codeclimate.com/github/splitrb/split) [![Test Coverage](https://codeclimate.com/github/splitrb/split/badges/coverage.svg)](https://codeclimate.com/github/splitrb/split/coverage) [![standard-readme compliant](https://img.shields.io/badge/readme%20style-standard-brightgreen.svg?style=flat-square)](https://github.com/RichardLitt/standard-readme) @@ -30,11 +30,13 @@ Ideally the master Split repository will replace SimpleRandom with a suitably-li ### Requirements -Split currently requires Ruby 1.9.3 or higher. If your project requires compatibility with Ruby 1.8.x and Rails 2.3, please use v0.8.0. +Split v4.0+ is currently tested with Ruby >= 2.5 and Rails >= 5.2. + +If your project requires compatibility with Ruby 2.4.x or older Rails versions. You can try v3.0 or v0.8.0(for Ruby 1.9.3) Split uses Redis as a datastore. -Split only supports Redis 2.0 or greater. +Split only supports Redis 4.0 or greater. If you're on OS X, Homebrew is the simplest way to install Redis: @@ -186,8 +188,10 @@ module SplitHelper # use_ab_test(signup_form: "single_page", pricing: "show_enterprise_prices") # def use_ab_test(alternatives_by_experiment) - allow_any_instance_of(Split::Helper).to receive(:ab_test) do |_receiver, experiment| - alternatives_by_experiment.fetch(experiment) { |key| raise "Unknown experiment '#{key}'" } + allow_any_instance_of(Split::Helper).to receive(:ab_test) do |_receiver, experiment, &block| + variant = alternatives_by_experiment.fetch(experiment) { |key| raise "Unknown experiment '#{key}'" } + block.call(variant) unless block.nil? + variant end end end @@ -274,7 +278,7 @@ Split.configure do |config| end ``` -By default, cookies will expire in 1 year. To change that, set the `persistence_cookie_length` in the configuration (unit of time in seconds). +When using the cookie persistence, Split stores data into an anonymous tracking cookie named 'split', which expires in 1 year. To change that, set the `persistence_cookie_length` in the configuration (unit of time in seconds). ```ruby Split.configure do |config| @@ -283,6 +287,8 @@ Split.configure do |config| end ``` +The data stored consists of the experiment name and the variants the user is in. Example: { "experiment_name" => "variant_a" } + __Note:__ Using cookies depends on `ActionDispatch::Cookies` or any identical API #### Redis @@ -657,7 +663,7 @@ The API to define goals for an experiment is this: ab_test({link_color: ["purchase", "refund"]}, "red", "blue") ``` -or you can you can define them in a configuration file: +or you can define them in a configuration file: ```ruby Split.configure do |config| @@ -765,6 +771,20 @@ split_config = YAML.load_file(Rails.root.join('config', 'split.yml')) Split.redis = split_config[Rails.env] ``` +### Redis Caching (v4.0+) + +In some high-volume usage scenarios, Redis load can be incurred by repeated +fetches for fairly static data. Enabling caching will reduce this load. + + ```ruby +Split.configuration.cache = true +```` + +This currently caches: + - `Split::ExperimentCatalog.find` + - `Split::Experiment.start_time` + - `Split::Experiment.winner` + ## Namespaces If you're running multiple, separate instances of Split you may want @@ -781,7 +801,7 @@ library. To configure Split to use `Redis::Namespace`, do the following: ``` 2. Configure `Split.redis` to use a `Redis::Namespace` instance (possible in an - intializer): + initializer): ```ruby redis = Redis.new(url: ENV['REDIS_URL']) # or whatever config you want diff --git a/Rakefile b/Rakefile index 1fc677bd..4555c708 100755 --- a/Rakefile +++ b/Rakefile @@ -1,9 +1,9 @@ #!/usr/bin/env rake # frozen_string_literal: true -require 'bundler/gem_tasks' -require 'rspec/core/rake_task' -require 'appraisal' -RSpec::Core::RakeTask.new('spec') +require "bundler/gem_tasks" +require "rspec/core/rake_task" -task :default => :spec \ No newline at end of file +RSpec::Core::RakeTask.new("spec") + +task default: :spec diff --git a/gemfiles/5.2.gemfile b/gemfiles/5.2.gemfile index 1c5d67bc..2530b2b9 100644 --- a/gemfiles/5.2.gemfile +++ b/gemfiles/5.2.gemfile @@ -1,8 +1,6 @@ -# This file was generated by Appraisal - source "https://rubygems.org" -gem "appraisal" +gem "rubocop", require: false gem "codeclimate-test-reporter" gem "rails", "~> 5.2" diff --git a/gemfiles/6.0.gemfile b/gemfiles/6.0.gemfile index 4960099f..1283d3c9 100644 --- a/gemfiles/6.0.gemfile +++ b/gemfiles/6.0.gemfile @@ -1,8 +1,6 @@ -# This file was generated by Appraisal - source "https://rubygems.org" -gem "appraisal" +gem "rubocop", require: false gem "codeclimate-test-reporter" gem "rails", "~> 6.0" diff --git a/gemfiles/5.1.gemfile b/gemfiles/6.1.gemfile similarity index 51% rename from gemfiles/5.1.gemfile rename to gemfiles/6.1.gemfile index fc8263b9..1d0b5256 100644 --- a/gemfiles/5.1.gemfile +++ b/gemfiles/6.1.gemfile @@ -1,9 +1,7 @@ -# This file was generated by Appraisal - source "https://rubygems.org" -gem "appraisal" +gem "rubocop", require: false gem "codeclimate-test-reporter" -gem "rails", "~> 5.1" +gem "rails", "~> 6.1" gemspec path: "../" diff --git a/gemfiles/5.0.gemfile b/gemfiles/7.0.gemfile similarity index 51% rename from gemfiles/5.0.gemfile rename to gemfiles/7.0.gemfile index a289e546..f9464c11 100644 --- a/gemfiles/5.0.gemfile +++ b/gemfiles/7.0.gemfile @@ -1,9 +1,8 @@ -# This file was generated by Appraisal - source "https://rubygems.org" -gem "appraisal" +gem "rubocop", require: false gem "codeclimate-test-reporter" -gem "rails", "~> 5.0" +gem "rails", "~> 7.0" +gem "matrix" gemspec path: "../" diff --git a/lib/split.rb b/lib/split.rb index 176023fa..1b24b787 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -1,27 +1,30 @@ # frozen_string_literal: true -require 'redis' -require 'split/algorithms/block_randomization' -require 'split/algorithms/weighted_sample' -require 'split/algorithms/whiplash' -require 'split/alternative' -require 'split/configuration' -require 'split/encapsulated_helper' -require 'split/exceptions' -require 'split/experiment' -require 'split/experiment_catalog' -require 'split/extensions/string' -require 'split/goals_collection' -require 'split/helper' -require 'split/combined_experiments_helper' -require 'split/metric' -require 'split/persistence' -require 'split/redis_interface' -require 'split/trial' -require 'split/user' -require 'split/version' -require 'split/zscore' -require 'split/engine' if defined?(Rails) +require "redis" + +require "split/algorithms" +require "split/algorithms/block_randomization" +require "split/algorithms/weighted_sample" +require "split/algorithms/whiplash" +require "split/alternative" +require "split/cache" +require "split/configuration" +require "split/encapsulated_helper" +require "split/exceptions" +require "split/experiment" +require "split/experiment_catalog" +require "split/extensions/string" +require "split/goals_collection" +require "split/helper" +require "split/combined_experiments_helper" +require "split/metric" +require "split/persistence" +require "split/redis_interface" +require "split/trial" +require "split/user" +require "split/version" +require "split/zscore" +require "split/engine" if defined?(Rails) module Split extend self @@ -35,9 +38,9 @@ module Split # `Redis::DistRedis`, or `Redis::Namespace`. def redis=(server) @redis = if server.is_a?(String) - Redis.new(:url => server, :thread_safe => true) + Redis.new(url: server) elsif server.is_a?(Hash) - Redis.new(server.merge(:thread_safe => true)) + Redis.new(server) elsif server.respond_to?(:smembers) server else @@ -64,13 +67,17 @@ def configure self.configuration ||= Configuration.new yield(configuration) end + + def cache(namespace, key, &block) + Split::Cache.fetch(namespace, key, &block) + end end # Check to see if being run in a Rails application. If so, wait until before_initialize to run configuration so Gems that create ENV variables have the chance to initialize first. if defined?(::Rails) - class Railtie < Rails::Railtie - config.before_initialize { Split.configure {} } + class Split::Railtie < Rails::Railtie + config.before_initialize { Split.configure { } } end else - Split.configure {} + Split.configure { } end diff --git a/lib/split/algorithms.rb b/lib/split/algorithms.rb new file mode 100644 index 00000000..14631597 --- /dev/null +++ b/lib/split/algorithms.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +begin + require "matrix" +rescue LoadError => error + if error.message.match?(/matrix/) + $stderr.puts "You don't have matrix installed in your application. Please add it to your Gemfile and run bundle install" + raise + end +end + +require "rubystats" + +module Split + module Algorithms + class << self + def beta_distribution_rng(a, b) + Rubystats::BetaDistribution.new(a, b).rng + end + end + end +end diff --git a/lib/split/algorithms/block_randomization.rb b/lib/split/algorithms/block_randomization.rb index de2bc3db..9cf8acbb 100644 --- a/lib/split/algorithms/block_randomization.rb +++ b/lib/split/algorithms/block_randomization.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + # Selects alternative with minimum count of participants # If all counts are even (i.e. all are minimum), samples from all possible alternatives @@ -11,12 +12,11 @@ def choose_alternative(experiment) end private - - def minimum_participant_alternatives(alternatives) - alternatives_by_count = alternatives.group_by(&:participant_count) - min_group = alternatives_by_count.min_by { |k, v| k } - min_group.last - end + def minimum_participant_alternatives(alternatives) + alternatives_by_count = alternatives.group_by(&:participant_count) + min_group = alternatives_by_count.min_by { |k, v| k } + min_group.last + end end end end diff --git a/lib/split/algorithms/weighted_sample.rb b/lib/split/algorithms/weighted_sample.rb index cbed66d3..9eab9305 100644 --- a/lib/split/algorithms/weighted_sample.rb +++ b/lib/split/algorithms/weighted_sample.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split module Algorithms module WeightedSample @@ -8,7 +9,7 @@ def self.choose_alternative(experiment) total = weights.inject(:+) point = rand * total - experiment.alternatives.zip(weights).each do |n,w| + experiment.alternatives.zip(weights).each do |n, w| return n if w >= point point -= w end diff --git a/lib/split/algorithms/whiplash.rb b/lib/split/algorithms/whiplash.rb index 5373cada..9308b548 100644 --- a/lib/split/algorithms/whiplash.rb +++ b/lib/split/algorithms/whiplash.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true + # A multi-armed bandit implementation inspired by # @aaronsw and victorykit/whiplash -require 'rubystats' module Split module Algorithms @@ -12,26 +12,25 @@ def choose_alternative(experiment) end private + def arm_guess(participants, completions) + a = [participants, 0].max + b = [participants-completions, 0].max + Split::Algorithms.beta_distribution_rng(a + fairness_constant, b + fairness_constant) + end - def arm_guess(participants, completions) - a = [participants, 0].max - b = [participants-completions, 0].max - Rubystats::BetaDistribution.new(a+fairness_constant, b+fairness_constant).rng - end - - def best_guess(alternatives) - guesses = {} - alternatives.each do |alternative| - guesses[alternative.name] = arm_guess(alternative.participant_count, alternative.all_completed_count) + def best_guess(alternatives) + guesses = {} + alternatives.each do |alternative| + guesses[alternative.name] = arm_guess(alternative.participant_count, alternative.all_completed_count) + end + gmax = guesses.values.max + best = guesses.keys.select { |name| guesses[name] == gmax } + best.sample end - gmax = guesses.values.max - best = guesses.keys.select { |name| guesses[name] == gmax } - best.sample - end - def fairness_constant - 7 - end + def fairness_constant + 7 + end end end end diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index 80de5099..3d416188 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split class Alternative attr_accessor :name @@ -37,11 +38,11 @@ def set_p_winner(prob, goal = nil) end def participant_count - Split.redis.hget(key, 'participant_count').to_i + Split.redis.hget(key, "participant_count").to_i end def participant_count=(count) - Split.redis.hset(key, 'participant_count', count.to_i) + Split.redis.hset(key, "participant_count", count.to_i) end def completed_count(goal = nil) @@ -66,13 +67,13 @@ def unfinished_count def set_field(goal) field = "completed_count" field += ":" + goal unless goal.nil? - return field + field end def set_prob_field(goal) field = "p_winner" field += ":" + goal unless goal.nil? - return field + field end def set_completed_count(count, goal = nil) @@ -81,7 +82,7 @@ def set_completed_count(count, goal = nil) end def increment_participation - Split.redis.hincrby key, 'participant_count', 1 + Split.redis.hincrby key, "participant_count", 1 end def increment_completion(goal = nil) @@ -111,7 +112,7 @@ def z_score(goal = nil) control = experiment.control alternative = self - return 'N/A' if control.name == alternative.name + return "N/A" if control.name == alternative.name p_a = alternative.conversion_rate(goal) p_c = control.conversion_rate(goal) @@ -120,13 +121,13 @@ def z_score(goal = nil) n_c = control.participant_count # can't calculate zscore for P(x) > 1 - return 'N/A' if p_a > 1 || p_c > 1 + return "N/A" if p_a > 1 || p_c > 1 Split::Zscore.calculate(p_a, n_a, p_c, n_c) end def extra_info - data = Split.redis.hget(key, 'recorded_info') + data = Split.redis.hget(key, "recorded_info") if data && data.length > 1 begin JSON.parse(data) @@ -148,24 +149,24 @@ def record_extra_info(k, value = 1) @recorded_info[k] = value end - Split.redis.hset key, 'recorded_info', (@recorded_info || {}).to_json + Split.redis.hset key, "recorded_info", (@recorded_info || {}).to_json end def save - Split.redis.hsetnx key, 'participant_count', 0 - Split.redis.hsetnx key, 'completed_count', 0 - Split.redis.hsetnx key, 'p_winner', p_winner - Split.redis.hsetnx key, 'recorded_info', (@recorded_info || {}).to_json + Split.redis.hsetnx key, "participant_count", 0 + Split.redis.hsetnx key, "completed_count", 0 + Split.redis.hsetnx key, "p_winner", p_winner + Split.redis.hsetnx key, "recorded_info", (@recorded_info || {}).to_json end def validate! unless String === @name || hash_with_correct_values?(@name) - raise ArgumentError, 'Alternative must be a string' + raise ArgumentError, "Alternative must be a string" end end def reset - Split.redis.hmset key, 'participant_count', 0, 'completed_count', 0, 'recorded_info', nil + Split.redis.hmset key, "participant_count", 0, "completed_count", 0, "recorded_info", nil unless goals.empty? goals.each do |g| field = "completed_count:#{g}" @@ -179,13 +180,12 @@ def delete end private + def hash_with_correct_values?(name) + Hash === name && String === name.keys.first && Float(name.values.first) rescue false + end - def hash_with_correct_values?(name) - Hash === name && String === name.keys.first && Float(name.values.first) rescue false - end - - def key - "#{experiment_name}:#{name}" - end + def key + "#{experiment_name}:#{name}" + end end end diff --git a/lib/split/cache.rb b/lib/split/cache.rb new file mode 100644 index 00000000..f570af7f --- /dev/null +++ b/lib/split/cache.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Split + class Cache + def self.clear + @cache = nil + end + + def self.fetch(namespace, key) + return yield unless Split.configuration.cache + + @cache ||= {} + @cache[namespace] ||= {} + + value = @cache[namespace][key] + return value if value + + @cache[namespace][key] = yield + end + + def self.clear_key(key) + @cache&.keys&.each do |namespace| + @cache[namespace]&.delete(key) + end + end + end +end diff --git a/lib/split/combined_experiments_helper.rb b/lib/split/combined_experiments_helper.rb index af5b2884..dd35d791 100644 --- a/lib/split/combined_experiments_helper.rb +++ b/lib/split/combined_experiments_helper.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split module CombinedExperimentsHelper def ab_combined_test(metric_descriptor, control = nil, *alternatives) @@ -28,10 +29,10 @@ def ab_combined_test(metric_descriptor, control = nil, *alternatives) end def find_combined_experiment(metric_descriptor) - raise(Split::InvalidExperimentsFormatError, 'Invalid descriptor class (String or Symbol required)') unless metric_descriptor.class == String || metric_descriptor.class == Symbol - raise(Split::InvalidExperimentsFormatError, 'Enable configuration') unless Split.configuration.enabled - raise(Split::InvalidExperimentsFormatError, 'Enable `allow_multiple_experiments`') unless Split.configuration.allow_multiple_experiments - Split::configuration.experiments[metric_descriptor.to_sym] + raise(Split::InvalidExperimentsFormatError, "Invalid descriptor class (String or Symbol required)") unless metric_descriptor.class == String || metric_descriptor.class == Symbol + raise(Split::InvalidExperimentsFormatError, "Enable configuration") unless Split.configuration.enabled + raise(Split::InvalidExperimentsFormatError, "Enable `allow_multiple_experiments`") unless Split.configuration.allow_multiple_experiments + Split.configuration.experiments[metric_descriptor.to_sym] end end end diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index f70b1a66..bbc39b01 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split class Configuration attr_accessor :ignore_ip_addresses @@ -10,6 +11,7 @@ class Configuration attr_accessor :enabled attr_accessor :persistence attr_accessor :persistence_cookie_length + attr_accessor :persistence_cookie_domain attr_accessor :algorithm attr_accessor :store_override attr_accessor :start_manually @@ -27,6 +29,7 @@ class Configuration attr_accessor :winning_alternative_recalculation_interval attr_accessor :redis attr_accessor :dashboard_pagination_default_per_page + attr_accessor :cache attr_reader :experiments @@ -36,83 +39,83 @@ class Configuration def bots @bots ||= { # Indexers - 'AdsBot-Google' => 'Google Adwords', - 'Baidu' => 'Chinese search engine', - 'Baiduspider' => 'Chinese search engine', - 'bingbot' => 'Microsoft bing bot', - 'Butterfly' => 'Topsy Labs', - 'Gigabot' => 'Gigabot spider', - 'Googlebot' => 'Google spider', - 'MJ12bot' => 'Majestic-12 spider', - 'msnbot' => 'Microsoft bot', - 'rogerbot' => 'SeoMoz spider', - 'PaperLiBot' => 'PaperLi is another content curation service', - 'Slurp' => 'Yahoo spider', - 'Sogou' => 'Chinese search engine', - 'spider' => 'generic web spider', - 'UnwindFetchor' => 'Gnip crawler', - 'WordPress' => 'WordPress spider', - 'YandexAccessibilityBot' => 'Yandex accessibility spider', - 'YandexBot' => 'Yandex spider', - 'YandexMobileBot' => 'Yandex mobile spider', - 'ZIBB' => 'ZIBB spider', + "AdsBot-Google" => "Google Adwords", + "Baidu" => "Chinese search engine", + "Baiduspider" => "Chinese search engine", + "bingbot" => "Microsoft bing bot", + "Butterfly" => "Topsy Labs", + "Gigabot" => "Gigabot spider", + "Googlebot" => "Google spider", + "MJ12bot" => "Majestic-12 spider", + "msnbot" => "Microsoft bot", + "rogerbot" => "SeoMoz spider", + "PaperLiBot" => "PaperLi is another content curation service", + "Slurp" => "Yahoo spider", + "Sogou" => "Chinese search engine", + "spider" => "generic web spider", + "UnwindFetchor" => "Gnip crawler", + "WordPress" => "WordPress spider", + "YandexAccessibilityBot" => "Yandex accessibility spider", + "YandexBot" => "Yandex spider", + "YandexMobileBot" => "Yandex mobile spider", + "ZIBB" => "ZIBB spider", # HTTP libraries - 'Apache-HttpClient' => 'Java http library', - 'AppEngine-Google' => 'Google App Engine', - 'curl' => 'curl unix CLI http client', - 'ColdFusion' => 'ColdFusion http library', - 'EventMachine HttpClient' => 'Ruby http library', - 'Go http package' => 'Go http library', - 'Go-http-client' => 'Go http library', - 'Java' => 'Generic Java http library', - 'libwww-perl' => 'Perl client-server library loved by script kids', - 'lwp-trivial' => 'Another Perl library loved by script kids', - 'Python-urllib' => 'Python http library', - 'PycURL' => 'Python http library', - 'Test Certificate Info' => 'C http library?', - 'Typhoeus' => 'Ruby http library', - 'Wget' => 'wget unix CLI http client', + "Apache-HttpClient" => "Java http library", + "AppEngine-Google" => "Google App Engine", + "curl" => "curl unix CLI http client", + "ColdFusion" => "ColdFusion http library", + "EventMachine HttpClient" => "Ruby http library", + "Go http package" => "Go http library", + "Go-http-client" => "Go http library", + "Java" => "Generic Java http library", + "libwww-perl" => "Perl client-server library loved by script kids", + "lwp-trivial" => "Another Perl library loved by script kids", + "Python-urllib" => "Python http library", + "PycURL" => "Python http library", + "Test Certificate Info" => "C http library?", + "Typhoeus" => "Ruby http library", + "Wget" => "wget unix CLI http client", # URL expanders / previewers - 'awe.sm' => 'Awe.sm URL expander', - 'bitlybot' => 'bit.ly bot', - 'bot@linkfluence.net' => 'Linkfluence bot', - 'facebookexternalhit' => 'facebook bot', - 'Facebot' => 'Facebook crawler', - 'Feedfetcher-Google' => 'Google Feedfetcher', - 'https://developers.google.com/+/web/snippet' => 'Google+ Snippet Fetcher', - 'LinkedInBot' => 'LinkedIn bot', - 'LongURL' => 'URL expander service', - 'NING' => 'NING - Yet Another Twitter Swarmer', - 'Pinterestbot' => 'Pinterest Bot', - 'redditbot' => 'Reddit Bot', - 'ShortLinkTranslate' => 'Link shortener', - 'Slackbot' => 'Slackbot link expander', - 'TweetmemeBot' => 'TweetMeMe Crawler', - 'Twitterbot' => 'Twitter URL expander', - 'UnwindFetch' => 'Gnip URL expander', - 'vkShare' => 'VKontake Sharer', + "awe.sm" => "Awe.sm URL expander", + "bitlybot" => "bit.ly bot", + "bot@linkfluence.net" => "Linkfluence bot", + "facebookexternalhit" => "facebook bot", + "Facebot" => "Facebook crawler", + "Feedfetcher-Google" => "Google Feedfetcher", + "https://developers.google.com/+/web/snippet" => "Google+ Snippet Fetcher", + "LinkedInBot" => "LinkedIn bot", + "LongURL" => "URL expander service", + "NING" => "NING - Yet Another Twitter Swarmer", + "Pinterestbot" => "Pinterest Bot", + "redditbot" => "Reddit Bot", + "ShortLinkTranslate" => "Link shortener", + "Slackbot" => "Slackbot link expander", + "TweetmemeBot" => "TweetMeMe Crawler", + "Twitterbot" => "Twitter URL expander", + "UnwindFetch" => "Gnip URL expander", + "vkShare" => "VKontake Sharer", # Uptime monitoring - 'check_http' => 'Nagios monitor', - 'GoogleStackdriverMonitoring' => 'Google Cloud monitor', - 'NewRelicPinger' => 'NewRelic monitor', - 'Panopta' => 'Monitoring service', - 'Pingdom' => 'Pingdom monitoring', - 'SiteUptime' => 'Site monitoring services', - 'UptimeRobot' => 'Monitoring service', + "check_http" => "Nagios monitor", + "GoogleStackdriverMonitoring" => "Google Cloud monitor", + "NewRelicPinger" => "NewRelic monitor", + "Panopta" => "Monitoring service", + "Pingdom" => "Pingdom monitoring", + "SiteUptime" => "Site monitoring services", + "UptimeRobot" => "Monitoring service", # ??? - 'DigitalPersona Fingerprint Software' => 'HP Fingerprint scanner', - 'ShowyouBot' => 'Showyou iOS app spider', - 'ZyBorg' => 'Zyborg? Hmmm....', - 'ELB-HealthChecker' => 'ELB Health Check' + "DigitalPersona Fingerprint Software" => "HP Fingerprint scanner", + "ShowyouBot" => "Showyou iOS app spider", + "ZyBorg" => "Zyborg? Hmmm....", + "ELB-HealthChecker" => "ELB Health Check" } end - def experiments= experiments - raise InvalidExperimentsFormatError.new('Experiments must be a Hash') unless experiments.respond_to?(:keys) + def experiments=(experiments) + raise InvalidExperimentsFormatError.new("Experiments must be a Hash") unless experiments.respond_to?(:keys) @experiments = experiments end @@ -154,8 +157,8 @@ def normalized_experiments @experiments.each do |experiment_name, settings| alternatives = if (alts = value_for(settings, :alternatives)) - normalize_alternatives(alts) - end + normalize_alternatives(alts) + end experiment_data = { alternatives: alternatives, @@ -176,7 +179,7 @@ def normalized_experiments end def normalize_alternatives(alternatives) - given_probability, num_with_probability = alternatives.inject([0,0]) do |a,v| + given_probability, num_with_probability = alternatives.inject([0, 0]) do |a, v| p, n = a if percent = value_for(v, :percent) [p + percent, n + 1] @@ -212,48 +215,38 @@ def robot_regex def initialize @ignore_ip_addresses = [] - @ignore_filter = proc{ |request| is_robot? || is_ignored_ip_address? } + @ignore_filter = proc { |request| is_robot? || is_ignored_ip_address? } @db_failover = false - @db_failover_on_db_error = proc{|error|} # e.g. use Rails logger here - @on_experiment_reset = proc{|experiment|} - @on_experiment_delete = proc{|experiment|} - @on_before_experiment_reset = proc{|experiment|} - @on_before_experiment_delete = proc{|experiment|} - @on_experiment_winner_choose = proc{|experiment|} + @db_failover_on_db_error = proc { |error| } # e.g. use Rails logger here + @on_experiment_reset = proc { |experiment| } + @on_experiment_delete = proc { |experiment| } + @on_before_experiment_reset = proc { |experiment| } + @on_before_experiment_delete = proc { |experiment| } + @on_experiment_winner_choose = proc { |experiment| } @db_failover_allow_parameter_override = false @allow_multiple_experiments = false @enabled = true @experiments = {} @persistence = Split::Persistence::SessionAdapter @persistence_cookie_length = 31536000 # One year from now + @persistence_cookie_domain = nil @algorithm = Split::Algorithms::WeightedSample @include_rails_helper = true @beta_probability_simulations = 10000 @winning_alternative_recalculation_interval = 60 * 60 * 24 # 1 day - @redis = ENV.fetch(ENV.fetch('REDIS_PROVIDER', 'REDIS_URL'), 'redis://localhost:6379') + @redis = ENV.fetch(ENV.fetch("REDIS_PROVIDER", "REDIS_URL"), "redis://localhost:6379") @dashboard_pagination_default_per_page = 10 end - def redis_url=(value) - warn '[DEPRECATED] `redis_url=` is deprecated in favor of `redis=`' - self.redis = value - end - - def redis_url - warn '[DEPRECATED] `redis_url` is deprecated in favor of `redis`' - self.redis - end - private - - def value_for(hash, key) - if hash.kind_of?(Hash) - hash.has_key?(key.to_s) ? hash[key.to_s] : hash[key.to_sym] + def value_for(hash, key) + if hash.kind_of?(Hash) + hash.has_key?(key.to_s) ? hash[key.to_s] : hash[key.to_sym] + end end - end - def escaped_bots - bots.map { |key, _| Regexp.escape(key) } - end + def escaped_bots + bots.map { |key, _| Regexp.escape(key) } + end end end diff --git a/lib/split/dashboard.rb b/lib/split/dashboard.rb index a6a9962f..fcb53c2b 100755 --- a/lib/split/dashboard.rb +++ b/lib/split/dashboard.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true -require 'sinatra/base' -require 'split' -require 'bigdecimal' -require 'split/dashboard/helpers' -require 'split/dashboard/pagination_helpers' + +require "sinatra/base" +require "split" +require "bigdecimal" +require "split/dashboard/helpers" +require "split/dashboard/pagination_helpers" module Split class Dashboard < Sinatra::Base @@ -17,7 +18,7 @@ class Dashboard < Sinatra::Base helpers Split::DashboardHelpers helpers Split::DashboardPaginationHelpers - get '/' do + get "/" do # Display experiments without a winner at the top of the dashboard @experiments = Split::ExperimentCatalog.all_active_first @unintialized_experiments = Split.configuration.experiments.keys - @experiments.map(&:name) @@ -25,7 +26,7 @@ class Dashboard < Sinatra::Base @metrics = Split::Metric.all # Display Rails Environment mode (or Rack version if not using Rails) - if Object.const_defined?('Rails') + if Object.const_defined?("Rails") @current_env = Rails.env.titlecase else @current_env = "Rack: #{Rack.version}" @@ -33,45 +34,48 @@ class Dashboard < Sinatra::Base erb :index end - post '/initialize_experiment' do + post "/initialize_experiment" do Split::ExperimentCatalog.find_or_create(params[:experiment]) unless params[:experiment].nil? || params[:experiment].empty? - redirect url('/') + redirect url("/") end - post '/force_alternative' do + post "/force_alternative" do experiment = Split::ExperimentCatalog.find(params[:experiment]) alternative = Split::Alternative.new(params[:alternative], experiment.name) - alternative.increment_participation - Split::User.new(self)[experiment.key] = alternative.name - redirect url('/') + + cookies = JSON.parse(request.cookies["split_override"]) rescue {} + cookies[experiment.name] = alternative.name + response.set_cookie("split_override", { value: cookies.to_json, path: "/" }) + + redirect url("/") end - post '/experiment' do + post "/experiment" do @experiment = Split::ExperimentCatalog.find(params[:experiment]) @alternative = Split::Alternative.new(params[:alternative], params[:experiment]) @experiment.winner = @alternative.name - redirect url('/') + redirect url("/") end - post '/start' do + post "/start" do @experiment = Split::ExperimentCatalog.find(params[:experiment]) @experiment.start - redirect url('/') + redirect url("/") end - post '/reset' do + post "/reset" do @experiment = Split::ExperimentCatalog.find(params[:experiment]) @experiment.reset - redirect url('/') + redirect url("/") end - post '/reopen' do + post "/reopen" do @experiment = Split::ExperimentCatalog.find(params[:experiment]) @experiment.reset_winner - redirect url('/') + redirect url("/") end - post '/update_cohorting' do + post "/update_cohorting" do @experiment = Split::ExperimentCatalog.find(params[:experiment]) case params[:cohorting_action].downcase when "enable" @@ -79,13 +83,13 @@ class Dashboard < Sinatra::Base when "disable" @experiment.disable_cohorting end - redirect url('/') + redirect url("/") end - delete '/experiment' do + delete "/experiment" do @experiment = Split::ExperimentCatalog.find(params[:experiment]) @experiment.delete - redirect url('/') + redirect url("/") end end end diff --git a/lib/split/dashboard/helpers.rb b/lib/split/dashboard/helpers.rb index 0c05481b..15727c72 100644 --- a/lib/split/dashboard/helpers.rb +++ b/lib/split/dashboard/helpers.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split module DashboardHelpers def h(text) @@ -6,11 +7,11 @@ def h(text) end def url(*path_parts) - [ path_prefix, path_parts ].join("/").squeeze('/') + [ path_prefix, path_parts ].join("/").squeeze("/") end def path_prefix - request.env['SCRIPT_NAME'] + request.env["SCRIPT_NAME"] end def number_to_percentage(number, precision = 2) @@ -31,15 +32,14 @@ def confidence_level(z_score) z = round(z_score.to_s.to_f, 3).abs if z >= 2.58 - '99% confidence' + "99% confidence" elsif z >= 1.96 - '95% confidence' + "95% confidence" elsif z >= 1.65 - '90% confidence' + "90% confidence" else - 'Insufficient confidence' + "Insufficient confidence" end - end end end diff --git a/lib/split/dashboard/pagination_helpers.rb b/lib/split/dashboard/pagination_helpers.rb index 3386879a..79d6393d 100644 --- a/lib/split/dashboard/pagination_helpers.rb +++ b/lib/split/dashboard/pagination_helpers.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true -require 'split/dashboard/paginator' + +require "split/dashboard/paginator" module Split module DashboardPaginationHelpers @@ -29,58 +30,57 @@ def pagination(collection) end private - - def show_first_page_tag? - page_number > 2 - end - - def first_page_tag - %Q(1) - end - - def show_first_ellipsis_tag? - page_number >= 4 - end - - def ellipsis_tag - '...' - end - - def show_prev_page_tag? - page_number > 1 - end - - def prev_page_tag - %Q(#{page_number - 1}) - end - - def current_page_tag - "#{page_number}" - end - - def show_next_page_tag?(collection) - (page_number * pagination_per) < collection.count - end - - def next_page_tag - %Q(#{page_number + 1}) - end - - def show_last_ellipsis_tag?(collection) - (total_pages(collection) - page_number) >= 3 - end - - def total_pages(collection) - collection.count / pagination_per + ((collection.count % pagination_per).zero? ? 0 : 1) - end - - def show_last_page_tag?(collection) - page_number < (total_pages(collection) - 1) - end - - def last_page_tag(collection) - total = total_pages(collection) - %Q(#{total}) - end + def show_first_page_tag? + page_number > 2 + end + + def first_page_tag + %Q(1) + end + + def show_first_ellipsis_tag? + page_number >= 4 + end + + def ellipsis_tag + "..." + end + + def show_prev_page_tag? + page_number > 1 + end + + def prev_page_tag + %Q(#{page_number - 1}) + end + + def current_page_tag + "#{page_number}" + end + + def show_next_page_tag?(collection) + (page_number * pagination_per) < collection.count + end + + def next_page_tag + %Q(#{page_number + 1}) + end + + def show_last_ellipsis_tag?(collection) + (total_pages(collection) - page_number) >= 3 + end + + def total_pages(collection) + collection.count / pagination_per + ((collection.count % pagination_per).zero? ? 0 : 1) + end + + def show_last_page_tag?(collection) + page_number < (total_pages(collection) - 1) + end + + def last_page_tag(collection) + total = total_pages(collection) + %Q(#{total}) + end end end diff --git a/lib/split/dashboard/paginator.rb b/lib/split/dashboard/paginator.rb index c76f9f70..b55578cf 100644 --- a/lib/split/dashboard/paginator.rb +++ b/lib/split/dashboard/paginator.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split class DashboardPaginator def initialize(collection, page_number, per) diff --git a/lib/split/dashboard/public/style.css b/lib/split/dashboard/public/style.css index 0dfe4fec..71273f53 100644 --- a/lib/split/dashboard/public/style.css +++ b/lib/split/dashboard/public/style.css @@ -320,7 +320,6 @@ a.button.green:focus, button.green:focus, input[type="submit"].green:focus { margin-top: 10px; } - .pagination { text-align: center; font-size: 15px; diff --git a/lib/split/dashboard/views/index.erb b/lib/split/dashboard/views/index.erb index ad2f8d68..b34f5d60 100644 --- a/lib/split/dashboard/views/index.erb +++ b/lib/split/dashboard/views/index.erb @@ -38,4 +38,4 @@ -
\ No newline at end of file +
diff --git a/lib/split/encapsulated_helper.rb b/lib/split/encapsulated_helper.rb index e41380f2..e200e0fe 100644 --- a/lib/split/encapsulated_helper.rb +++ b/lib/split/encapsulated_helper.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require "split/helper" # Split's helper exposes all kinds of methods we don't want to @@ -14,7 +15,6 @@ # module Split module EncapsulatedHelper - class ContextShim include Split::Helper public :ab_test, :ab_finished @@ -28,15 +28,14 @@ def ab_user end end - def ab_test(*arguments,&block) - split_context_shim.ab_test(*arguments,&block) + def ab_test(*arguments, &block) + split_context_shim.ab_test(*arguments, &block) end private - - # instantiate and memoize a context shim in case of multiple ab_test* calls - def split_context_shim - @split_context_shim ||= ContextShim.new(self) - end + # instantiate and memoize a context shim in case of multiple ab_test* calls + def split_context_shim + @split_context_shim ||= ContextShim.new(self) + end end end diff --git a/lib/split/engine.rb b/lib/split/engine.rb index 3663cc94..0d31bcd9 100644 --- a/lib/split/engine.rb +++ b/lib/split/engine.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split class Engine < ::Rails::Engine initializer "split" do |app| diff --git a/lib/split/exceptions.rb b/lib/split/exceptions.rb index 3330f836..7b0d98b2 100644 --- a/lib/split/exceptions.rb +++ b/lib/split/exceptions.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split class InvalidPersistenceAdapterError < StandardError; end class ExperimentNotFound < StandardError; end diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index befdf3d5..ba2cb7f5 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'rubystats' - module Split class Experiment attr_accessor :name @@ -15,10 +13,17 @@ class Experiment attr_reader :retain_user_alternatives_after_reset DEFAULT_OPTIONS = { - :resettable => true, - :retain_user_alternatives_after_reset => false, + resettable: true, + retain_user_alternatives_after_reset: false } + def self.find(name) + Split.cache(:experiments, name) do + return unless Split.redis.exists?(name) + Experiment.new(name).tap { |exp| exp.load_from_redis } + end + end + def initialize(name, options = {}) options = DEFAULT_OPTIONS.merge(options) @@ -50,7 +55,7 @@ def extract_alternatives_from_options(options) if alts.length == 1 if alts[0].is_a? Hash - alts = alts[0].map{|k,v| {k => v} } + alts = alts[0].map { |k, v| { k => v } } end end @@ -87,9 +92,9 @@ def save persist_experiment_configuration end - redis.hset(experiment_config_key, :retain_user_alternatives_after_reset, retain_user_alternatives_after_reset) - redis.hset(experiment_config_key, :resettable, resettable) - redis.hset(experiment_config_key, :algorithm, algorithm.to_s) + redis.hmset(experiment_config_key, :retain_user_alternatives_after_reset, retain_user_alternatives_after_reset, + :resettable, resettable, + :algorithm, algorithm.to_s) self end @@ -97,12 +102,12 @@ def validate! if @alternatives.empty? && Split.configuration.experiment_for(@name).nil? raise ExperimentNotFound.new("Experiment #{@name} not found") end - @alternatives.each {|a| a.validate! } + @alternatives.each { |a| a.validate! } goals_collection.validate! end def new_record? - !redis.exists(name) + ExperimentCatalog.find(name).nil? end def ==(obj) @@ -110,7 +115,7 @@ def ==(obj) end def [](name) - alternatives.find{|a| a.name == name} + alternatives.find { |a| a.name == name } end def algorithm @@ -122,7 +127,7 @@ def algorithm=(algorithm) end def resettable=(resettable) - @resettable = resettable.is_a?(String) ? resettable == 'true' : resettable + @resettable = resettable.is_a?(String) ? resettable == "true" : resettable end def retain_user_alternatives_after_reset=(value) @@ -140,11 +145,13 @@ def alternatives=(alts) end def winner - experiment_winner = redis.hget(:experiment_winner, name) - if experiment_winner - Split::Alternative.new(experiment_winner, name) - else - nil + Split.cache(:experiment_winner, name) do + experiment_winner = redis.hget(:experiment_winner, name) + if experiment_winner + Split::Alternative.new(experiment_winner, name) + else + nil + end end end @@ -160,7 +167,7 @@ def winner=(winner_name) end def participant_count - alternatives.inject(0){|sum,a| sum + a.participant_count} + alternatives.inject(0) { |sum, a| sum + a.participant_count } end def control @@ -170,6 +177,7 @@ def control def reset_winner redis.hdel(:experiment_winner, name) @has_winner = false + Split::Cache.clear_key(@name) end def start @@ -177,13 +185,15 @@ def start end def start_time - t = redis.hget(:experiment_start_times, @name) - if t - # Check if stored time is an integer - if t =~ /^[-+]?[0-9]+$/ - Time.at(t.to_i) - else - Time.parse(t) + Split.cache(:experiment_start_times, @name) do + t = redis.hget(:experiment_start_times, @name) + if t + # Check if stored time is an integer + if t =~ /^[-+]?[0-9]+$/ + Time.at(t.to_i) + else + Time.parse(t) + end end end end @@ -238,6 +248,7 @@ def resettable? def reset Split.configuration.on_before_experiment_reset.call(self) + Split::Cache.clear_key(@name) alternatives.each(&:reset) reset_winner Split.configuration.on_experiment_reset.call(self) @@ -265,9 +276,9 @@ def load_from_redis exp_config = redis.hgetall(experiment_config_key) options = { - retain_user_alternatives_after_reset: exp_config['retain_user_alternatives_after_reset'], - resettable: exp_config['resettable'], - algorithm: exp_config['algorithm'], + retain_user_alternatives_after_reset: exp_config["retain_user_alternatives_after_reset"], + resettable: exp_config["resettable"], + algorithm: exp_config["algorithm"], friendly_name: load_friendly_name_from_redis, alternatives: load_alternatives_from_redis, goals: Split::GoalsCollection.new(@name).load_from_redis, @@ -333,11 +344,11 @@ def calc_alternative_probabilities(winning_counts, number_of_simulations) winning_counts.each do |alternative, wins| alternative_probabilities[alternative] = wins / number_of_simulations.to_f end - return alternative_probabilities + alternative_probabilities end def count_simulated_wins(winning_alternatives) - # initialize a hash to keep track of winning alternative in simulations + # initialize a hash to keep track of winning alternative in simulations winning_counts = {} alternatives.each do |alternative| winning_counts[alternative] = 0 @@ -346,19 +357,19 @@ def count_simulated_wins(winning_alternatives) winning_alternatives.each do |alternative| winning_counts[alternative] += 1 end - return winning_counts + winning_counts end def find_simulated_winner(simulated_cr_hash) # figure out which alternative had the highest simulated conversion rate - winning_pair = ["",0.0] + winning_pair = ["", 0.0] simulated_cr_hash.each do |alternative, rate| if rate > winning_pair[1] winning_pair = [alternative, rate] end end winner = winning_pair[0] - return winner + winner end def calc_simulated_conversion_rates(beta_params) @@ -368,11 +379,11 @@ def calc_simulated_conversion_rates(beta_params) beta_params.each do |alternative, params| alpha = params[0] beta = params[1] - simulated_conversion_rate = Rubystats::BetaDistribution.new(alpha, beta).rng + simulated_conversion_rate = Split::Algorithms.beta_distribution_rng(alpha, beta) simulated_cr_hash[alternative] = simulated_conversion_rate end - return simulated_cr_hash + simulated_cr_hash end def calc_beta_params(goal = nil) @@ -386,7 +397,7 @@ def calc_beta_params(goal = nil) beta_params[alternative] = params end - return beta_params + beta_params end def calc_time=(time) @@ -399,11 +410,11 @@ def calc_time def jstring(goal = nil) js_id = if goal.nil? - name - else - name + "-" + goal - end - js_id.gsub('/', '--') + name + else + name + "-" + goal + end + js_id.gsub("/", "--") end def cohorting_disabled? @@ -424,99 +435,94 @@ def enable_cohorting end protected + def experiment_config_key + "experiment_configurations/#{@name}" + end - def experiment_config_key - "experiment_configurations/#{@name}" - end - - def load_metadata_from_configuration - Split.configuration.experiment_for(@name)[:metadata] - end + def load_metadata_from_configuration + Split.configuration.experiment_for(@name)[:metadata] + end - def load_metadata_from_redis - meta = redis.get(metadata_key) - JSON.parse(meta) unless meta.nil? - end + def load_metadata_from_redis + meta = redis.get(metadata_key) + JSON.parse(meta) unless meta.nil? + end - def load_friendly_name_from_configuration - Split.configuration.experiment_for(@name)[:friendly_name] - end + def load_friendly_name_from_configuration + Split.configuration.experiment_for(@name)[:friendly_name] + end - def load_friendly_name_from_redis - redis.get(friendly_name_key) - end + def load_friendly_name_from_redis + redis.get(friendly_name_key) + end - def load_alternatives_from_configuration - alts = Split.configuration.experiment_for(@name)[:alternatives] - raise ArgumentError, "Experiment configuration is missing :alternatives array" unless alts - if alts.is_a?(Hash) - alts.keys - else - alts.flatten + def load_alternatives_from_configuration + alts = Split.configuration.experiment_for(@name)[:alternatives] + raise ArgumentError, "Experiment configuration is missing :alternatives array" unless alts + if alts.is_a?(Hash) + alts.keys + else + alts.flatten + end end - end - def load_alternatives_from_redis - alternatives = case redis.type(@name) - when 'set' # convert legacy sets to lists - alts = redis.smembers(@name) - redis.del(@name) - alts.reverse.each {|a| redis.lpush(@name, a) } - redis.lrange(@name, 0, -1) - else - redis.lrange(@name, 0, -1) - end - alternatives.map do |alt| - alt = begin - JSON.parse(alt) - rescue - alt - end - Split::Alternative.new(alt, @name) + def load_alternatives_from_redis + alternatives = redis.lrange(@name, 0, -1) + alternatives.map do |alt| + alt = begin + JSON.parse(alt) + rescue + alt + end + Split::Alternative.new(alt, @name) + end end - end private + def redis + Split.redis + end - def redis - Split.redis - end + def redis_interface + RedisInterface.new + end - def redis_interface - RedisInterface.new - end + def persist_experiment_configuration + redis_interface.add_to_set(:experiments, name) + redis_interface.persist_list(name, @alternatives.map { |alt| { alt.name => alt.weight }.to_json }) + goals_collection.save - def persist_experiment_configuration - redis_interface.add_to_set(:experiments, name) - redis_interface.persist_list(name, @alternatives.map{|alt| {alt.name => alt.weight}.to_json}) - goals_collection.save - redis.set(metadata_key, @metadata.to_json) unless @metadata.nil? - redis.set(friendly_name_key, self.friendly_name) - end + if @metadata + redis.set(metadata_key, @metadata.to_json) + else + delete_metadata + end - def remove_experiment_configuration - @alternatives.each(&:delete) - goals_collection.delete - delete_metadata - redis.del(@name) - end + redis.set(friendly_name_key, self.friendly_name) + end - def experiment_configuration_has_changed? - existing_alternatives = load_alternatives_from_redis - existing_goals = Split::GoalsCollection.new(@name).load_from_redis - existing_metadata = load_metadata_from_redis - existing_alternatives.map(&:to_s) != @alternatives.map(&:to_s) || - existing_goals != @goals || - existing_metadata != @metadata - end + def remove_experiment_configuration + @alternatives.each(&:delete) + goals_collection.delete + delete_metadata + redis.del(@name) + end - def goals_collection - Split::GoalsCollection.new(@name, @goals) - end + def experiment_configuration_has_changed? + existing_experiment = Experiment.find(@name) - def remove_experiment_cohorting - @cohorting_disabled = false - redis.hdel(experiment_config_key, :cohorting) - end + existing_experiment.alternatives.map(&:to_s) != @alternatives.map(&:to_s) || + existing_experiment.goals != @goals || + existing_experiment.metadata != @metadata + end + + def goals_collection + Split::GoalsCollection.new(@name, @goals) + end + + def remove_experiment_cohorting + @cohorting_disabled = false + redis.hdel(experiment_config_key, :cohorting) + end end end diff --git a/lib/split/experiment_catalog.rb b/lib/split/experiment_catalog.rb index e5652f31..7a8674cc 100644 --- a/lib/split/experiment_catalog.rb +++ b/lib/split/experiment_catalog.rb @@ -1,33 +1,33 @@ # frozen_string_literal: true + module Split class ExperimentCatalog # Return all experiments def self.all # Call compact to prevent nil experiments from being returned -- seems to happen during gem upgrades - Split.redis.smembers(:experiments).map {|e| find(e)}.compact + Split.redis.smembers(:experiments).map { |e| find(e) }.compact end # Return experiments without a winner (considered "active") first def self.all_active_first - all.partition{|e| not e.winner}.map{|es| es.sort_by(&:name)}.flatten + all.partition { |e| not e.winner }.map { |es| es.sort_by(&:name) }.flatten end def self.find(name) - return unless Split.redis.exists(name) - Experiment.new(name).tap { |exp| exp.load_from_redis } + Experiment.find(name) end def self.find_or_initialize(metric_descriptor, control = nil, *alternatives) # Check if array is passed to ab_test # e.g. ab_test('name', ['Alt 1', 'Alt 2', 'Alt 3']) - if control.is_a? Array and alternatives.length.zero? + if control.is_a?(Array) && alternatives.length.zero? control, alternatives = control.first, control[1..-1] end experiment_name_with_version, goals = normalize_experiment(metric_descriptor) - experiment_name = experiment_name_with_version.to_s.split(':')[0] + experiment_name = experiment_name_with_version.to_s.split(":")[0] Split::Experiment.new(experiment_name, - :alternatives => [control].compact + alternatives, :goals => goals) + alternatives: [control].compact + alternatives, goals: goals) end def self.find_or_create(metric_descriptor, control = nil, *alternatives) @@ -46,6 +46,5 @@ def self.normalize_experiment(metric_descriptor) return experiment_name, goals end private_class_method :normalize_experiment - end end diff --git a/lib/split/extensions/string.rb b/lib/split/extensions/string.rb index fbad4784..e20de92c 100644 --- a/lib/split/extensions/string.rb +++ b/lib/split/extensions/string.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true + class String # Constatntize is often provided by ActiveSupport, but ActiveSupport is not a dependency of Split. unless method_defined?(:constantize) def constantize - names = self.split('::') + names = self.split("::") names.shift if names.empty? || names.first.empty? constant = Object diff --git a/lib/split/goals_collection.rb b/lib/split/goals_collection.rb index 12507c0d..8aa787a4 100644 --- a/lib/split/goals_collection.rb +++ b/lib/split/goals_collection.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true + module Split class GoalsCollection - - def initialize(experiment_name, goals=nil) + def initialize(experiment_name, goals = nil) @experiment_name = experiment_name @goals = goals end @@ -14,10 +14,10 @@ def load_from_redis def load_from_configuration goals = Split.configuration.experiment_for(@experiment_name)[:goals] - if goals.nil? - goals = [] - else + if goals goals.flatten + else + [] end end @@ -28,7 +28,7 @@ def save def validate! unless @goals.nil? || @goals.kind_of?(Array) - raise ArgumentError, 'Goals must be an array' + raise ArgumentError, "Goals must be an array" end end @@ -37,9 +37,8 @@ def delete end private - - def goals_key - "#{@experiment_name}:goals" - end + def goals_key + "#{@experiment_name}:goals" + end end end diff --git a/lib/split/helper.rb b/lib/split/helper.rb index 4e53a5ec..f9c728c4 100644 --- a/lib/split/helper.rb +++ b/lib/split/helper.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split module Helper OVERRIDE_PARAM_NAME = "ab_test" @@ -11,9 +12,9 @@ def ab_test(metric_descriptor, control = nil, *alternatives) alternative = if Split.configuration.enabled && !exclude_visitor? experiment.save raise(Split::InvalidExperimentsFormatError) unless (Split.configuration.experiments || {}).fetch(experiment.name.to_sym, {})[:combined_experiments].nil? - trial = Trial.new(:user => ab_user, :experiment => experiment, - :override => override_alternative(experiment.name), :exclude => !is_qualified?, - :disabled => split_generically_disabled?) + trial = Trial.new(user: ab_user, experiment: experiment, + override: override_alternative(experiment.name), exclude: exclude_visitor? || !is_qualified?, + disabled: split_generically_disabled?) alt = trial.choose!(self) alt ? alt.name : nil else @@ -43,7 +44,7 @@ def reset!(experiment) ab_user.delete(experiment.key) end - def finish_experiment(experiment, options = {:reset => true}) + def finish_experiment(experiment, options = { reset: true }) return false if active_experiments[experiment.name].nil? return true if experiment.has_winner? should_reset = experiment.resettable? && options[:reset] @@ -57,14 +58,14 @@ def finish_experiment(experiment, options = {:reset => true}) end if ab_user[user_experiment_finished_key] && !should_reset - return true + true else alternative_name = ab_user[user_experiment_key] trial = Trial.new( - :user => ab_user, - :experiment => experiment, - :alternative => alternative_name, - :goals => options[:goals], + user: ab_user, + experiment: experiment, + alternative: alternative_name, + goals: options[:goals], ) trial.complete!(self) @@ -77,14 +78,15 @@ def finish_experiment(experiment, options = {:reset => true}) end end - def ab_finished(metric_descriptor, options = {:reset => true}) + def ab_finished(metric_descriptor, options = { reset: true }) return if exclude_visitor? || Split.configuration.disabled? metric_descriptor, goals = normalize_metric(metric_descriptor) experiments = Metric.possible_experiments(metric_descriptor) if experiments.any? experiments.each do |experiment| - finish_experiment(experiment, options.merge(:goals => goals)) + next if override_present?(experiment.key) + finish_experiment(experiment, options.merge(goals: goals)) end end rescue => e @@ -102,7 +104,7 @@ def ab_record_extra_info(metric_descriptor, key, value = 1) alternative_name = ab_user[experiment.key] if alternative_name - alternative = experiment.alternatives.find{|alt| alt.name == alternative_name} + alternative = experiment.alternatives.find { |alt| alt.name == alternative_name } alternative.record_extra_info(key, value) if alternative end end @@ -112,24 +114,36 @@ def ab_record_extra_info(metric_descriptor, key, value = 1) Split.configuration.db_failover_on_db_error.call(e) end - def ab_active_experiments() + def ab_active_experiments ab_user.active_experiments rescue => e raise unless Split.configuration.db_failover Split.configuration.db_failover_on_db_error.call(e) end - def override_present?(experiment_name) - override_alternative(experiment_name) + override_alternative_by_params(experiment_name) || override_alternative_by_cookies(experiment_name) end def override_alternative(experiment_name) + override_alternative_by_params(experiment_name) || override_alternative_by_cookies(experiment_name) + end + + def override_alternative_by_params(experiment_name) defined?(params) && params[OVERRIDE_PARAM_NAME] && params[OVERRIDE_PARAM_NAME][experiment_name] end + def override_alternative_by_cookies(experiment_name) + return unless defined?(request) + + if request.cookies && request.cookies.key?("split_override") + experiments = JSON.parse(request.cookies["split_override"]) rescue {} + experiments[experiment_name] + end + end + def split_generically_disabled? - defined?(params) && params['SPLIT_DISABLE'] + defined?(params) && params["SPLIT_DISABLE"] end def ab_user @@ -145,7 +159,7 @@ def is_robot? end def is_preview? - defined?(request) && defined?(request.headers) && request.headers['x-purpose'] == 'preview' + defined?(request) && defined?(request.headers) && request.headers["x-purpose"] == "preview" end def is_ignored_ip_address? diff --git a/lib/split/metric.rb b/lib/split/metric.rb index 79b3d1f0..ed9e48ac 100644 --- a/lib/split/metric.rb +++ b/lib/split/metric.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true + module Split class Metric attr_accessor :name attr_accessor :experiments def initialize(attrs = {}) - attrs.each do |key,value| + attrs.each do |key, value| if self.respond_to?("#{key}=") self.send("#{key}=", value) end @@ -15,13 +16,13 @@ def initialize(attrs = {}) def self.load_from_redis(name) metric = Split.redis.hget(:metrics, name) if metric - experiment_names = metric.split(',') + experiment_names = metric.split(",") experiments = experiment_names.collect do |experiment_name| Split::ExperimentCatalog.find(experiment_name) end - Split::Metric.new(:name => name, :experiments => experiments) + Split::Metric.new(name: name, experiments: experiments) else nil end @@ -30,7 +31,7 @@ def self.load_from_redis(name) def self.load_from_configuration(name) metrics = Split.configuration.metrics if metrics && metrics[name] - Split::Metric.new(:experiments => metrics[name], :name => name) + Split::Metric.new(experiments: metrics[name], name: name) else nil end @@ -76,7 +77,7 @@ def self.possible_experiments(metric_name) end def save - Split.redis.hset(:metrics, name, experiments.map(&:name).join(',')) + Split.redis.hset(:metrics, name, experiments.map(&:name).join(",")) end def complete! @@ -96,6 +97,5 @@ def self.normalize_metric(label) return metric_name, goals end private_class_method :normalize_metric - end end diff --git a/lib/split/persistence.rb b/lib/split/persistence.rb index a464c463..f9e52b33 100644 --- a/lib/split/persistence.rb +++ b/lib/split/persistence.rb @@ -2,14 +2,16 @@ module Split module Persistence - require 'split/persistence/cookie_adapter' - require 'split/persistence/dual_adapter' - require 'split/persistence/redis_adapter' - require 'split/persistence/session_adapter' + require "split/persistence/cookie_adapter" + require "split/persistence/dual_adapter" + require "split/persistence/redis_adapter" + require "split/persistence/session_adapter" ADAPTERS = { - :cookie => Split::Persistence::CookieAdapter, - :session => Split::Persistence::SessionAdapter + cookie: Split::Persistence::CookieAdapter, + session: Split::Persistence::SessionAdapter, + redis: Split::Persistence::RedisAdapter, + dual_adapter: Split::Persistence::DualAdapter }.freeze def self.adapter diff --git a/lib/split/persistence/cookie_adapter.rb b/lib/split/persistence/cookie_adapter.rb index ffe7be65..2eac7fca 100644 --- a/lib/split/persistence/cookie_adapter.rb +++ b/lib/split/persistence/cookie_adapter.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true + require "json" module Split module Persistence class CookieAdapter - def initialize(context) @context = context @request, @response = context.request, context.response @@ -29,48 +29,46 @@ def keys end private + def set_cookie(value = {}) + cookie_key = :split.to_s + cookie_value = default_options.merge(value: JSON.generate(value)) + if action_dispatch? + # The "send" is necessary when we call ab_test from the controller + # and thus @context is a rails controller, because then "cookies" is + # a private method. + @context.send(:cookies)[cookie_key] = cookie_value + else + set_cookie_via_rack(cookie_key, cookie_value) + end + end - def set_cookie(value = {}) - cookie_key = :split.to_s - cookie_value = default_options.merge(value: JSON.generate(value)) - if action_dispatch? - # The "send" is necessary when we call ab_test from the controller - # and thus @context is a rails controller, because then "cookies" is - # a private method. - @context.send(:cookies)[cookie_key] = cookie_value - else - set_cookie_via_rack(cookie_key, cookie_value) + def default_options + { expires: @expires, path: "/", domain: cookie_domain_config }.compact end - end - def default_options - { expires: @expires, path: '/' } - end + def set_cookie_via_rack(key, value) + delete_cookie_header!(@response.header, key, value) + Rack::Utils.set_cookie_header!(@response.header, key, value) + end - def set_cookie_via_rack(key, value) - delete_cookie_header!(@response.header, key, value) - Rack::Utils.set_cookie_header!(@response.header, key, value) - end + # Use Rack::Utils#make_delete_cookie_header after Rack 2.0.0 + def delete_cookie_header!(header, key, value) + cookie_header = header["Set-Cookie"] + case cookie_header + when nil, "" + cookies = [] + when String + cookies = cookie_header.split("\n") + when Array + cookies = cookie_header + end - # Use Rack::Utils#make_delete_cookie_header after Rack 2.0.0 - def delete_cookie_header!(header, key, value) - cookie_header = header['Set-Cookie'] - case cookie_header - when nil, '' - cookies = [] - when String - cookies = cookie_header.split("\n") - when Array - cookies = cookie_header + cookies.reject! { |cookie| cookie =~ /\A#{Rack::Utils.escape(key)}=/ } + header["Set-Cookie"] = cookies.join("\n") end - cookies.reject! { |cookie| cookie =~ /\A#{Rack::Utils.escape(key)}=/ } - header['Set-Cookie'] = cookies.join("\n") - end - - def hash - @hash ||= begin - if cookies = @cookies[:split.to_s] + def hash + @hash ||= if cookies = @cookies[:split.to_s] begin JSON.parse(cookies) rescue JSON::ParserError @@ -80,15 +78,18 @@ def hash {} end end - end - def cookie_length_config - Split.configuration.persistence_cookie_length - end + def cookie_length_config + Split.configuration.persistence_cookie_length + end - def action_dispatch? - defined?(Rails) && @response.is_a?(ActionDispatch::Response) - end + def cookie_domain_config + Split.configuration.persistence_cookie_domain + end + + def action_dispatch? + defined?(Rails) && @response.is_a?(ActionDispatch::Response) + end end end end diff --git a/lib/split/persistence/dual_adapter.rb b/lib/split/persistence/dual_adapter.rb index d5449c14..af8bd683 100644 --- a/lib/split/persistence/dual_adapter.rb +++ b/lib/split/persistence/dual_adapter.rb @@ -3,7 +3,7 @@ module Split module Persistence class DualAdapter - def self.with_config(options={}) + def self.with_config(options = {}) self.config.merge!(options) self end @@ -72,14 +72,13 @@ def delete(key) end private + def decrement_participation?(old_value, value) + !old_value.nil? && !value.nil? && old_value != value + end - def decrement_participation?(old_value, value) - !old_value.nil? && !value.nil? && old_value != value - end - - def decrement_participation(key, value) - Split.redis.hincrby("#{key}:#{value}", 'participant_count', -1) - end + def decrement_participation(key, value) + Split.redis.hincrby("#{key}:#{value}", "participant_count", -1) + end end end end diff --git a/lib/split/persistence/redis_adapter.rb b/lib/split/persistence/redis_adapter.rb index 3fe367b9..e314d882 100644 --- a/lib/split/persistence/redis_adapter.rb +++ b/lib/split/persistence/redis_adapter.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true + module Split module Persistence class RedisAdapter - DEFAULT_CONFIG = {:namespace => 'persistence'}.freeze + DEFAULT_CONFIG = { namespace: "persistence" }.freeze attr_reader :redis_key @@ -39,7 +40,11 @@ def keys Split.redis.hkeys(redis_key) end - def self.with_config(options={}) + def self.find(user_id) + new(nil, user_id) + end + + def self.with_config(options = {}) self.config.merge!(options) self end @@ -51,7 +56,6 @@ def self.config def self.reset_config! @config = DEFAULT_CONFIG.dup end - end end end diff --git a/lib/split/persistence/session_adapter.rb b/lib/split/persistence/session_adapter.rb index 6722db5c..38a5b86b 100644 --- a/lib/split/persistence/session_adapter.rb +++ b/lib/split/persistence/session_adapter.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true + module Split module Persistence class SessionAdapter - def initialize(context) @session = context.session @session[:split] ||= {} @@ -23,7 +23,6 @@ def delete(key) def keys @session[:split].keys end - end end end diff --git a/lib/split/redis_interface.rb b/lib/split/redis_interface.rb index 1796a980..a0b0f54b 100644 --- a/lib/split/redis_interface.rb +++ b/lib/split/redis_interface.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split # Simplifies the interface to Redis. class RedisInterface @@ -7,44 +8,22 @@ def initialize end def persist_list(list_name, list_values) - max_index = list_length(list_name) - 1 - list_values.each_with_index do |value, index| - if index > max_index - add_to_list(list_name, value) - else - set_list_index(list_name, index, value) + if list_values.length > 0 + redis.multi do |multi| + tmp_list = "#{list_name}_tmp" + multi.rpush(tmp_list, list_values) + multi.rename(tmp_list, list_name) end end - make_list_length(list_name, list_values.length) - list_values - end - - def add_to_list(list_name, value) - redis.rpush(list_name, value) - end - - def set_list_index(list_name, index, value) - redis.lset(list_name, index, value) - end - def list_length(list_name) - redis.llen(list_name) - end - - def remove_last_item_from_list(list_name) - redis.rpop(list_name) - end - - def make_list_length(list_name, new_length) - redis.ltrim(list_name, 0, new_length - 1) + list_values end def add_to_set(set_name, value) - redis.sadd(set_name, value) unless redis.sismember(set_name, value) + redis.sadd(set_name, value) end private - - attr_accessor :redis + attr_accessor :redis end end diff --git a/lib/split/trial.rb b/lib/split/trial.rb index 911315d5..575c820f 100644 --- a/lib/split/trial.rb +++ b/lib/split/trial.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Split class Trial attr_accessor :goals @@ -14,7 +15,7 @@ def initialize(attrs = {}) @user = attrs.delete(:user) @options = attrs - @alternative_choosen = false + @alternative_chosen = false end def metadata @@ -23,15 +24,15 @@ def metadata def alternative @alternative ||= if @experiment.has_winner? - @experiment.winner - end + @experiment.winner + end end def alternative=(alternative) @alternative = if alternative.kind_of?(Split::Alternative) alternative else - @experiment.alternatives.find{|a| a.name == alternative } + @experiment.alternatives.find { |a| a.name == alternative } end end @@ -42,7 +43,7 @@ def complete!(context = nil) if Array(goals).empty? alternative.increment_completion else - Array(goals).each {|g| alternative.increment_completion(g) } + Array(goals).each { |g| alternative.increment_completion(g) } end end @@ -57,7 +58,7 @@ def complete!(context = nil) def choose!(context = nil) @user.cleanup_old_experiments! # Only run the process once - return alternative if @alternative_choosen + return alternative if @alternative_chosen user_experiment_key = @experiment.retain_user_alternatives_after_reset ? @user.alternative_key_for_experiment(@experiment) : @experiment.key new_participant = @user[user_experiment_key].nil? @@ -120,43 +121,42 @@ def within_conversion_time_frame? end private + def user_experiment_key + @user_experiment_key ||= @user.alternative_key_for_experiment(@experiment) + end - def user_experiment_key - @user_experiment_key ||= @user.alternative_key_for_experiment(@experiment) - end - - def delete_time_of_assignment_key - @user.delete("#{user_experiment_key}:time_of_assignment") - end + def delete_time_of_assignment_key + @user.delete("#{user_experiment_key}:time_of_assignment") + end - def save_time_that_user_is_assigned - @user["#{user_experiment_key}:time_of_assignment"] = Time.now.to_s - end + def save_time_that_user_is_assigned + @user["#{user_experiment_key}:time_of_assignment"] = Time.now.to_s + end - def run_callback(context, callback_name) - context.send(callback_name, self) if callback_name && context.respond_to?(callback_name, true) - end + def run_callback(context, callback_name) + context.send(callback_name, self) if callback_name && context.respond_to?(callback_name, true) + end - def override_is_alternative? - @experiment.alternatives.map(&:name).include?(@options[:override]) - end + def override_is_alternative? + @experiment.alternatives.map(&:name).include?(@options[:override]) + end - def should_store_alternative? - if @options[:override] || @options[:disabled] - Split.configuration.store_override - else - !exclude_user? + def should_store_alternative? + if @options[:override] || @options[:disabled] + Split.configuration.store_override + else + !exclude_user? + end end - end - def cleanup_old_versions - if @experiment.version > 0 - @user.cleanup_old_versions!(@experiment) + def cleanup_old_versions + if @experiment.version > 0 + @user.cleanup_old_versions!(@experiment) + end end - end - def exclude_user? - @options[:exclude] || @experiment.start_time.nil? || @user.max_experiments_reached?(@experiment.key) - end + def exclude_user? + @options[:exclude] || @experiment.start_time.nil? || @user.max_experiments_reached?(@experiment.key) + end end end diff --git a/lib/split/user.rb b/lib/split/user.rb index 8e138afe..f3c3f24f 100644 --- a/lib/split/user.rb +++ b/lib/split/user.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true -require 'forwardable' + +require "forwardable" module Split class User @@ -7,7 +8,7 @@ class User def_delegators :@user, :keys, :[], :[]=, :delete attr_reader :user - def initialize(context, adapter=nil) + def initialize(context, adapter = nil) @user = adapter || Split::Persistence.adapter.new(context) @cleaned_up = false end @@ -26,10 +27,10 @@ def cleanup_old_experiments! end def max_experiments_reached?(experiment_key) - if Split.configuration.allow_multiple_experiments == 'control' + if Split.configuration.allow_multiple_experiments == "control" experiments = active_experiments experiment_key_without_version = key_without_version(experiment_key) - count_control = experiments.count {|k,v| k == experiment_key_without_version || v == 'control'} + count_control = experiments.count { |k, v| k == experiment_key_without_version || v == "control" } experiments.size > count_control else !Split.configuration.allow_multiple_experiments && @@ -80,33 +81,42 @@ def alternative_key_for_experiment(experiment) end end - private + def self.find(user_id, adapter) + adapter = adapter.is_a?(Symbol) ? Split::Persistence::ADAPTERS[adapter] : adapter - def keys_without_experiment(keys, experiment_key) - if experiment_key.include?(':') - sub_keys = keys.reject { |k| k == experiment_key } - sub_keys.reject do |k| - sub_str = k.partition(':').last - - k.match(Regexp.new("^#{experiment_key}:")) && sub_str.scan(Regexp.new("\\D")).any? - end + if adapter.respond_to?(:find) + User.new(nil, adapter.find(user_id)) else - keys.select do |k| - k.match(Regexp.new("^#{experiment_key}:\\d+(:|$)")) || - k.partition(':').first != experiment_key - end + nil end end - def experiment_keys(keys) - keys.reject do |k| - sub_str = k.partition(':').last - sub_str.scan(Regexp.new("\\D")).any? + private + def keys_without_experiment(keys, experiment_key) + if experiment_key.include?(':') + sub_keys = keys.reject { |k| k == experiment_key } + sub_keys.reject do |k| + sub_str = k.partition(':').last + + k.match(Regexp.new("^#{experiment_key}:")) && sub_str.scan(Regexp.new("\\D")).any? + end + else + keys.select do |k| + k.match(Regexp.new("^#{experiment_key}:\\d+(:|$)")) || + k.partition(':').first != experiment_key + end + end end - end - def key_without_version(key) - key.split(/\:\d(?!\:)/)[0] - end + def experiment_keys(keys) + keys.reject do |k| + sub_str = k.partition(':').last + sub_str.scan(Regexp.new("\\D")).any? + end + end + + def key_without_version(key) + key.split(/\:\d(?!\:)/)[0] + end end end diff --git a/lib/split/version.rb b/lib/split/version.rb index 3e2d5737..037a710e 100644 --- a/lib/split/version.rb +++ b/lib/split/version.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true + module Split - MAJOR = 3 - MINOR = 4 - PATCH = 1 - VERSION = [MAJOR, MINOR, PATCH].join('.') + VERSION = "4.0.1" end diff --git a/lib/split/zscore.rb b/lib/split/zscore.rb index fbd84873..eac9160b 100644 --- a/lib/split/zscore.rb +++ b/lib/split/zscore.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true + module Split class Zscore - include Math def self.calculate(p1, n1, p2, n2) @@ -50,8 +50,7 @@ def self.calculate(p1, n1, p2, n2) # Calculate z-score z_score = (p_1 - p_2)/(se) - return z_score - + z_score end end end diff --git a/spec/algorithms/block_randomization_spec.rb b/spec/algorithms/block_randomization_spec.rb index c19935c7..9417710c 100644 --- a/spec/algorithms/block_randomization_spec.rb +++ b/spec/algorithms/block_randomization_spec.rb @@ -1,11 +1,12 @@ +# frozen_string_literal: true + require "spec_helper" describe Split::Algorithms::BlockRandomization do - - let(:experiment) { Split::Experiment.new 'experiment' } - let(:alternative_A) { Split::Alternative.new 'A', 'experiment' } - let(:alternative_B) { Split::Alternative.new 'B', 'experiment' } - let(:alternative_C) { Split::Alternative.new 'C', 'experiment' } + let(:experiment) { Split::Experiment.new "experiment" } + let(:alternative_A) { Split::Alternative.new "A", "experiment" } + let(:alternative_B) { Split::Alternative.new "B", "experiment" } + let(:alternative_C) { Split::Alternative.new "C", "experiment" } before :each do allow(experiment).to receive(:alternatives) { [alternative_A, alternative_B, alternative_C] } diff --git a/spec/algorithms/weighted_sample_spec.rb b/spec/algorithms/weighted_sample_spec.rb index d350482b..53a20170 100644 --- a/spec/algorithms/weighted_sample_spec.rb +++ b/spec/algorithms/weighted_sample_spec.rb @@ -1,19 +1,20 @@ # frozen_string_literal: true + require "spec_helper" describe Split::Algorithms::WeightedSample do it "should return an alternative" do - experiment = Split::ExperimentCatalog.find_or_create('link_color', {'blue' => 100}, {'red' => 0 }) + experiment = Split::ExperimentCatalog.find_or_create("link_color", { "blue" => 100 }, { "red" => 0 }) expect(Split::Algorithms::WeightedSample.choose_alternative(experiment).class).to eq(Split::Alternative) end it "should always return a heavily weighted option" do - experiment = Split::ExperimentCatalog.find_or_create('link_color', {'blue' => 100}, {'red' => 0 }) - expect(Split::Algorithms::WeightedSample.choose_alternative(experiment).name).to eq('blue') + experiment = Split::ExperimentCatalog.find_or_create("link_color", { "blue" => 100 }, { "red" => 0 }) + expect(Split::Algorithms::WeightedSample.choose_alternative(experiment).name).to eq("blue") end it "should return one of the results" do - experiment = Split::ExperimentCatalog.find_or_create('link_color', {'blue' => 1}, {'red' => 1 }) - expect(['red', 'blue']).to include Split::Algorithms::WeightedSample.choose_alternative(experiment).name + experiment = Split::ExperimentCatalog.find_or_create("link_color", { "blue" => 1 }, { "red" => 1 }) + expect(["red", "blue"]).to include Split::Algorithms::WeightedSample.choose_alternative(experiment).name end end diff --git a/spec/algorithms/whiplash_spec.rb b/spec/algorithms/whiplash_spec.rb index 07ca83aa..2d8b5291 100644 --- a/spec/algorithms/whiplash_spec.rb +++ b/spec/algorithms/whiplash_spec.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true + require "spec_helper" describe Split::Algorithms::Whiplash do - it "should return an algorithm" do - experiment = Split::ExperimentCatalog.find_or_create('link_color', {'blue' => 1}, {'red' => 1 }) + experiment = Split::ExperimentCatalog.find_or_create("link_color", { "blue" => 1 }, { "red" => 1 }) expect(Split::Algorithms::Whiplash.choose_alternative(experiment).class).to eq(Split::Alternative) end it "should return one of the results" do - experiment = Split::ExperimentCatalog.find_or_create('link_color', {'blue' => 1}, {'red' => 1 }) - expect(['red', 'blue']).to include Split::Algorithms::Whiplash.choose_alternative(experiment).name + experiment = Split::ExperimentCatalog.find_or_create("link_color", { "blue" => 1 }, { "red" => 1 }) + expect(["red", "blue"]).to include Split::Algorithms::Whiplash.choose_alternative(experiment).name end it "should guess floats" do @@ -20,5 +20,4 @@ expect(Split::Algorithms::Whiplash.send(:arm_guess, 1000, 5).class).to eq(Float) expect(Split::Algorithms::Whiplash.send(:arm_guess, 10, -2).class).to eq(Float) end - end diff --git a/spec/alternative_spec.rb b/spec/alternative_spec.rb index 9f00869a..1714c2fb 100644 --- a/spec/alternative_spec.rb +++ b/spec/alternative_spec.rb @@ -1,19 +1,19 @@ # frozen_string_literal: true -require 'spec_helper' -require 'split/alternative' -describe Split::Alternative do +require "spec_helper" +require "split/alternative" +describe Split::Alternative do let(:alternative) { - Split::Alternative.new('Basket', 'basket_text') + Split::Alternative.new("Basket", "basket_text") } let(:alternative2) { - Split::Alternative.new('Cart', 'basket_text') + Split::Alternative.new("Cart", "basket_text") } let!(:experiment) { - Split::ExperimentCatalog.find_or_create({"basket_text" => ["purchase", "refund"]}, "Basket", "Cart") + Split::ExperimentCatalog.find_or_create({ "basket_text" => ["purchase", "refund"] }, "Basket", "Cart") } let(:goal1) { "purchase" } @@ -24,48 +24,48 @@ end it "should have and only return the name" do - expect(alternative.name).to eq('Basket') + expect(alternative.name).to eq("Basket") end - describe 'weights' do + describe "weights" do it "should set the weights" do - experiment = Split::Experiment.new('basket_text', :alternatives => [{'Basket' => 0.6}, {"Cart" => 0.4}]) + experiment = Split::Experiment.new("basket_text", alternatives: [{ "Basket" => 0.6 }, { "Cart" => 0.4 }]) first = experiment.alternatives[0] - expect(first.name).to eq('Basket') + expect(first.name).to eq("Basket") expect(first.weight).to eq(0.6) second = experiment.alternatives[1] - expect(second.name).to eq('Cart') + expect(second.name).to eq("Cart") expect(second.weight).to eq(0.4) end it "accepts probability on alternatives" do Split.configuration.experiments = { - :my_experiment => { - :alternatives => [ - { :name => "control_opt", :percent => 67 }, - { :name => "second_opt", :percent => 10 }, - { :name => "third_opt", :percent => 23 }, + my_experiment: { + alternatives: [ + { name: "control_opt", percent: 67 }, + { name: "second_opt", percent: 10 }, + { name: "third_opt", percent: 23 }, ] } } experiment = Split::Experiment.new(:my_experiment) first = experiment.alternatives[0] - expect(first.name).to eq('control_opt') + expect(first.name).to eq("control_opt") expect(first.weight).to eq(0.67) second = experiment.alternatives[1] - expect(second.name).to eq('second_opt') + expect(second.name).to eq("second_opt") expect(second.weight).to eq(0.1) end it "accepts probability on some alternatives" do Split.configuration.experiments = { - :my_experiment => { - :alternatives => [ - { :name => "control_opt", :percent => 34 }, + my_experiment: { + alternatives: [ + { name: "control_opt", percent: 34 }, "second_opt", - { :name => "third_opt", :percent => 23 }, + { name: "third_opt", percent: 23 }, "fourth_opt", ], } @@ -87,11 +87,11 @@ # it "allows name param without probability" do Split.configuration.experiments = { - :my_experiment => { - :alternatives => [ - { :name => "control_opt" }, + my_experiment: { + alternatives: [ + { name: "control_opt" }, "second_opt", - { :name => "third_opt", :percent => 64 }, + { name: "third_opt", percent: 64 }, ], } } @@ -126,7 +126,7 @@ it "should save to redis" do alternative.save - expect(Split.redis.exists('basket_text:Basket')).to be true + expect(Split.redis.exists?("basket_text:Basket")).to be true end it "should increment participation count" do @@ -166,7 +166,7 @@ expect(alternative2.control?).to be_falsey end - describe 'unfinished_count' do + describe "unfinished_count" do it "should be difference between participant and completed counts" do alternative.increment_participation expect(alternative.unfinished_count).to eq(alternative.participant_count) @@ -182,7 +182,7 @@ end end - describe 'conversion rate' do + describe "conversion rate" do it "should be 0 if there are no conversions" do expect(alternative.completed_count).to eq(0) expect(alternative.conversion_rate).to eq(0) @@ -201,8 +201,31 @@ end end - describe 'z score' do + describe "probability winner" do + before do + experiment.calc_winning_alternatives + end + + it "should have a probability of being the winning alternative (p_winner)" do + expect(alternative.p_winner).not_to be_nil + end + + it "should have a probability of being the winner for each goal" do + expect(alternative.p_winner(goal1)).not_to be_nil + end + + it "should be possible to set the p_winner" do + alternative.set_p_winner(0.5) + expect(alternative.p_winner).to eq(0.5) + end + + it "should be possible to set the p_winner for each goal" do + alternative.set_p_winner(0.5, goal1) + expect(alternative.p_winner(goal1)).to eq(0.5) + end + end + describe "z score" do it "should return an error string when the control has 0 people" do expect(alternative2.z_score).to eq("Needs 30+ participants.") expect(alternative2.z_score(goal1)).to eq("Needs 30+ participants.") @@ -245,9 +268,9 @@ it "should be N/A for the control" do control = experiment.control - expect(control.z_score).to eq('N/A') - expect(control.z_score(goal1)).to eq('N/A') - expect(control.z_score(goal2)).to eq('N/A') + expect(control.z_score).to eq("N/A") + expect(control.z_score(goal1)).to eq("N/A") + expect(control.z_score(goal2)).to eq("N/A") end it "should not blow up for Conversion Rates > 1" do @@ -265,8 +288,8 @@ describe "extra_info" do it "reads saved value of recorded_info in redis" do - saved_recorded_info = {"key_1" => 1, "key_2" => "2"} - Split.redis.hset "#{alternative.experiment_name}:#{alternative.name}", 'recorded_info', saved_recorded_info.to_json + saved_recorded_info = { "key_1" => 1, "key_2" => "2" } + Split.redis.hset "#{alternative.experiment_name}:#{alternative.name}", "recorded_info", saved_recorded_info.to_json extra_info = alternative.extra_info expect(extra_info).to eql(saved_recorded_info) diff --git a/spec/cache_spec.rb b/spec/cache_spec.rb new file mode 100644 index 00000000..48b6cd46 --- /dev/null +++ b/spec/cache_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe Split::Cache do + let(:namespace) { :test_namespace } + let(:key) { :test_key } + let(:now) { 1606189017 } + + before { allow(Time).to receive(:now).and_return(now) } + + describe "clear" do + before { Split.configuration.cache = true } + + it "clears the cache" do + expect(Time).to receive(:now).and_return(now).exactly(2).times + Split::Cache.fetch(namespace, key) { Time.now } + Split::Cache.clear + Split::Cache.fetch(namespace, key) { Time.now } + end + end + + describe "clear_key" do + before { Split.configuration.cache = true } + + it "clears the cache" do + expect(Time).to receive(:now).and_return(now).exactly(3).times + Split::Cache.fetch(namespace, :key1) { Time.now } + Split::Cache.fetch(namespace, :key2) { Time.now } + Split::Cache.clear_key(:key1) + + Split::Cache.fetch(namespace, :key1) { Time.now } + Split::Cache.fetch(namespace, :key2) { Time.now } + end + end + + describe "fetch" do + subject { Split::Cache.fetch(namespace, key) { Time.now } } + + context "when cache disabled" do + before { Split.configuration.cache = false } + + it "returns the yield" do + expect(subject).to eql(now) + end + + it "yields every time" do + expect(Time).to receive(:now).and_return(now).exactly(2).times + Split::Cache.fetch(namespace, key) { Time.now } + Split::Cache.fetch(namespace, key) { Time.now } + end + end + + context "when cache enabled" do + before { Split.configuration.cache = true } + + it "returns the yield" do + expect(subject).to eql(now) + end + + it "yields once" do + expect(Time).to receive(:now).and_return(now).once + Split::Cache.fetch(namespace, key) { Time.now } + Split::Cache.fetch(namespace, key) { Time.now } + end + + it "honors namespace" do + expect(Split::Cache.fetch(:a, key) { :a }).to eql(:a) + expect(Split::Cache.fetch(:b, key) { :b }).to eql(:b) + + expect(Split::Cache.fetch(:a, key) { :a }).to eql(:a) + expect(Split::Cache.fetch(:b, key) { :b }).to eql(:b) + end + + it "honors key" do + expect(Split::Cache.fetch(namespace, :a) { :a }).to eql(:a) + expect(Split::Cache.fetch(namespace, :b) { :b }).to eql(:b) + + expect(Split::Cache.fetch(namespace, :a) { :a }).to eql(:a) + expect(Split::Cache.fetch(namespace, :b) { :b }).to eql(:b) + end + end + end +end diff --git a/spec/combined_experiments_helper_spec.rb b/spec/combined_experiments_helper_spec.rb index c9d0d3b3..651b5649 100644 --- a/spec/combined_experiments_helper_spec.rb +++ b/spec/combined_experiments_helper_spec.rb @@ -1,57 +1,58 @@ # frozen_string_literal: true -require 'spec_helper' -require 'split/combined_experiments_helper' + +require "spec_helper" +require "split/combined_experiments_helper" describe Split::CombinedExperimentsHelper do include Split::CombinedExperimentsHelper - describe 'ab_combined_test' do + describe "ab_combined_test" do let!(:config_enabled) { true } - let!(:combined_experiments) { [:exp_1_click, :exp_1_scroll ]} + let!(:combined_experiments) { [:exp_1_click, :exp_1_scroll ] } let!(:allow_multiple_experiments) { true } before do Split.configuration.experiments = { - :combined_exp_1 => { - :alternatives => [ {"control"=> 0.5}, {"test-alt"=> 0.5} ], - :metric => :my_metric, - :combined_experiments => combined_experiments + combined_exp_1: { + alternatives: [ { "control"=> 0.5 }, { "test-alt"=> 0.5 } ], + metric: :my_metric, + combined_experiments: combined_experiments } } Split.configuration.enabled = config_enabled Split.configuration.allow_multiple_experiments = allow_multiple_experiments end - context 'without config enabled' do + context "without config enabled" do let!(:config_enabled) { false } it "raises an error" do - expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError ) + expect { ab_combined_test :combined_exp_1 }.to raise_error(Split::InvalidExperimentsFormatError) end end - context 'multiple experiments disabled' do + context "multiple experiments disabled" do let!(:allow_multiple_experiments) { false } it "raises an error if multiple experiments is disabled" do - expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError) + expect { ab_combined_test :combined_exp_1 }.to raise_error(Split::InvalidExperimentsFormatError) end end - context 'without combined experiments' do + context "without combined experiments" do let!(:combined_experiments) { nil } it "raises an error" do - expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError ) + expect { ab_combined_test :combined_exp_1 }.to raise_error(Split::InvalidExperimentsFormatError) end end it "uses same alternative for all sub experiments and returns the alternative" do allow(self).to receive(:get_alternative) { "test-alt" } - expect(self).to receive(:ab_test).with(:exp_1_click, {"control"=>0.5}, {"test-alt"=>0.5}) { "test-alt" } - expect(self).to receive(:ab_test).with(:exp_1_scroll, [{"control" => 0, "test-alt" => 1}]) + expect(self).to receive(:ab_test).with(:exp_1_click, { "control"=>0.5 }, { "test-alt"=>0.5 }) { "test-alt" } + expect(self).to receive(:ab_test).with(:exp_1_scroll, [{ "control" => 0, "test-alt" => 1 }]) - expect(ab_combined_test('combined_exp_1')).to eq('test-alt') + expect(ab_combined_test("combined_exp_1")).to eq("test-alt") end end end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index bff3002f..4e4af75c 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require 'spec_helper' -describe Split::Configuration do +require "spec_helper" +describe Split::Configuration do before(:each) { @config = Split::Configuration.new } it "should provide a default value for ignore_ip_addresses" do @@ -58,17 +58,15 @@ end it "should load a metric" do - @config.experiments = {:my_experiment=> - {:alternatives=>["control_opt", "other_opt"], :metric=>:my_metric}} + @config.experiments = { my_experiment: { alternatives: ["control_opt", "other_opt"], metric: :my_metric } } expect(@config.metrics).not_to be_nil expect(@config.metrics.keys).to eq([:my_metric]) end it "should allow loading of experiment using experment_for" do - @config.experiments = {:my_experiment=> - {:alternatives=>["control_opt", "other_opt"], :metric=>:my_metric}} - expect(@config.experiment_for(:my_experiment)).to eq({:alternatives=>["control_opt", ["other_opt"]]}) + @config.experiments = { my_experiment: { alternatives: ["control_opt", "other_opt"], metric: :my_metric } } + expect(@config.experiment_for(:my_experiment)).to eq({ alternatives: ["control_opt", ["other_opt"]] }) end context "when experiments are defined via YAML" do @@ -88,7 +86,7 @@ end it 'should normalize experiments' do - expect(@config.normalized_experiments).to eq({:my_experiment=>{:resettable=>false,:retain_user_alternatives_after_reset=>true,:alternatives=>["Control Opt", ["Alt One", "Alt Two"]]}}) + expect(@config.normalized_experiments).to eq({ my_experiment: { resettable: false, retain_user_alternatives_after_reset: true, alternatives: ["Control Opt", ["Alt One", "Alt Two"]] } }) end end @@ -111,14 +109,14 @@ Alt Two: text: 'Alternative Two' resettable: false - eos + eos @config.experiments = YAML.load(experiments_yaml) end - it 'should have metadata on the experiment' do + it "should have metadata on the experiment" do meta = @config.normalized_experiments[:my_experiment][:metadata] expect(meta).to_not be nil - expect(meta['Control Opt']['text']).to eq('Control Option') + expect(meta["Control Opt"]["text"]).to eq("Control Option") end end @@ -140,25 +138,23 @@ alternatives: - a - b - eos + eos @config.experiments = YAML.load(experiments_yaml) end it "should normalize experiments" do - expect(@config.normalized_experiments).to eq({:my_experiment=>{:resettable=>false,:retain_user_alternatives_after_reset=>true,:alternatives=>[{"Control Opt"=>0.67}, - [{"Alt One"=>0.1}, {"Alt Two"=>0.23}]]}, :another_experiment=>{:alternatives=>["a", ["b"]]}}) + expect(@config.normalized_experiments).to eq({ my_experiment: { resettable: false, retain_user_alternatives_after_reset: true, alternatives: [{ "Control Opt"=>0.67 }, + [{ "Alt One"=>0.1 }, { "Alt Two"=>0.23 }]] }, another_experiment: { alternatives: ["a", ["b"]] } }) end it "should recognize metrics" do expect(@config.metrics).not_to be_nil expect(@config.metrics.keys).to eq([:my_metric]) end - end end context "as symbols" do - context "with valid YAML" do before do experiments_yaml = <<-eos @@ -168,21 +164,20 @@ - Alt One - Alt Two :resettable: false - eos + eos @config.experiments = YAML.load(experiments_yaml) end it "should normalize experiments" do - expect(@config.normalized_experiments).to eq({:my_experiment=>{:resettable=>false,:alternatives=>["Control Opt", ["Alt One", "Alt Two"]]}}) + expect(@config.normalized_experiments).to eq({ my_experiment: { resettable: false, alternatives: ["Control Opt", ["Alt One", "Alt Two"]] } }) end end context "with invalid YAML" do - let(:yaml) { YAML.load(input) } context "with an empty string" do - let(:input) { '' } + let(:input) { "" } it "should raise an error" do expect { @config.experiments = yaml }.to raise_error(Split::InvalidExperimentsFormatError) @@ -190,7 +185,7 @@ end context "with just the YAML header" do - let(:input) { '---' } + let(:input) { "---" } it "should raise an error" do expect { @config.experiments = yaml }.to raise_error(Split::InvalidExperimentsFormatError) @@ -202,35 +197,24 @@ it "should normalize experiments" do @config.experiments = { - :my_experiment => { - :alternatives => [ - { :name => "control_opt", :percent => 67 }, - { :name => "second_opt", :percent => 10 }, - { :name => "third_opt", :percent => 23 }, + my_experiment: { + alternatives: [ + { name: "control_opt", percent: 67 }, + { name: "second_opt", percent: 10 }, + { name: "third_opt", percent: 23 }, ], } } - expect(@config.normalized_experiments).to eq({:my_experiment=>{:alternatives=>[{"control_opt"=>0.67}, [{"second_opt"=>0.1}, {"third_opt"=>0.23}]]}}) - end - - context 'redis_url configuration [DEPRECATED]' do - it 'should warn on set and assign to #redis' do - expect(@config).to receive(:warn).with(/\[DEPRECATED\]/) { nil } - @config.redis_url = 'example_url' - expect(@config.redis).to eq('example_url') - end - - it 'should warn on get and return #redis' do - expect(@config).to receive(:warn).with(/\[DEPRECATED\]/) { nil } - @config.redis = 'example_url' - expect(@config.redis_url).to eq('example_url') - end + expect(@config.normalized_experiments).to eq({ my_experiment: { alternatives: [{ "control_opt"=>0.67 }, [{ "second_opt"=>0.1 }, { "third_opt"=>0.23 }]] } }) end context "redis configuration" do it "should default to local redis server" do - expect(@config.redis).to eq("redis://localhost:6379") + old_redis_url = ENV["REDIS_URL"] + ENV.delete("REDIS_URL") + expect(Split::Configuration.new.redis).to eq("redis://localhost:6379") + ENV["REDIS_URL"] = old_redis_url end it "should allow for redis url to be configured" do @@ -240,8 +224,10 @@ context "provided REDIS_URL environment variable" do it "should use the ENV variable" do - ENV['REDIS_URL'] = "env_redis_url" + old_redis_url = ENV["REDIS_URL"] + ENV["REDIS_URL"] = "env_redis_url" expect(Split::Configuration.new.redis).to eq("env_redis_url") + ENV["REDIS_URL"] = old_redis_url end end end @@ -257,4 +243,14 @@ end end + context "persistence cookie domain" do + it "should default to nil" do + expect(@config.persistence_cookie_domain).to eq(nil) + end + + it "should allow the persistence cookie domain to be configured" do + @config.persistence_cookie_domain = ".acme.com" + expect(@config.persistence_cookie_domain).to eq(".acme.com") + end + end end diff --git a/spec/dashboard/pagination_helpers_spec.rb b/spec/dashboard/pagination_helpers_spec.rb index 315595e0..cf5d81db 100644 --- a/spec/dashboard/pagination_helpers_spec.rb +++ b/spec/dashboard/pagination_helpers_spec.rb @@ -1,108 +1,110 @@ -require 'spec_helper' -require 'split/dashboard/pagination_helpers' +# frozen_string_literal: true + +require "spec_helper" +require "split/dashboard/pagination_helpers" describe Split::DashboardPaginationHelpers do include Split::DashboardPaginationHelpers - let(:url) { '/split/' } + let(:url) { "/split/" } - describe '#pagination_per' do - context 'when params empty' do + describe "#pagination_per" do + context "when params empty" do let(:params) { Hash[] } - it 'returns the default (10)' do + it "returns the default (10)" do default_per_page = Split.configuration.dashboard_pagination_default_per_page expect(pagination_per).to eql default_per_page expect(pagination_per).to eql 10 end end - context 'when params[:per] is 5' do + context "when params[:per] is 5" do let(:params) { Hash[per: 5] } - it 'returns 5' do + it "returns 5" do expect(pagination_per).to eql 5 end end end - describe '#page_number' do - context 'when params empty' do + describe "#page_number" do + context "when params empty" do let(:params) { Hash[] } - it 'returns 1' do + it "returns 1" do expect(page_number).to eql 1 end end context 'when params[:page] is "2"' do - let(:params) { Hash[page: '2'] } + let(:params) { Hash[page: "2"] } - it 'returns 2' do + it "returns 2" do expect(page_number).to eql 2 end end end - describe '#paginated' do + describe "#paginated" do let(:collection) { (1..20).to_a } - let(:params) { Hash[per: '5', page: '3'] } + let(:params) { Hash[per: "5", page: "3"] } it { expect(paginated(collection)).to eql [11, 12, 13, 14, 15] } end - describe '#show_first_page_tag?' do - context 'when page is 1' do + describe "#show_first_page_tag?" do + context "when page is 1" do it { expect(show_first_page_tag?).to be false } end - context 'when page is 3' do - let(:params) { Hash[page: '3'] } + context "when page is 3" do + let(:params) { Hash[page: "3"] } it { expect(show_first_page_tag?).to be true } end end - describe '#first_page_tag' do + describe "#first_page_tag" do it { expect(first_page_tag).to eql '1' } end - describe '#show_first_ellipsis_tag?' do - context 'when page is 1' do + describe "#show_first_ellipsis_tag?" do + context "when page is 1" do it { expect(show_first_ellipsis_tag?).to be false } end - context 'when page is 4' do - let(:params) { Hash[page: '4'] } + context "when page is 4" do + let(:params) { Hash[page: "4"] } it { expect(show_first_ellipsis_tag?).to be true } end end - describe '#ellipsis_tag' do - it { expect(ellipsis_tag).to eql '...' } + describe "#ellipsis_tag" do + it { expect(ellipsis_tag).to eql "..." } end - describe '#show_prev_page_tag?' do - context 'when page is 1' do + describe "#show_prev_page_tag?" do + context "when page is 1" do it { expect(show_prev_page_tag?).to be false } end - context 'when page is 2' do - let(:params) { Hash[page: '2'] } + context "when page is 2" do + let(:params) { Hash[page: "2"] } it { expect(show_prev_page_tag?).to be true } end end - describe '#prev_page_tag' do - context 'when page is 2' do - let(:params) { Hash[page: '2'] } + describe "#prev_page_tag" do + context "when page is 2" do + let(:params) { Hash[page: "2"] } it do expect(prev_page_tag).to eql '1' end end - context 'when page is 3' do - let(:params) { Hash[page: '3'] } + context "when page is 3" do + let(:params) { Hash[page: "3"] } it do expect(prev_page_tag).to eql '2' @@ -110,90 +112,90 @@ end end - describe '#show_prev_page_tag?' do - context 'when page is 1' do + describe "#show_prev_page_tag?" do + context "when page is 1" do it { expect(show_prev_page_tag?).to be false } end - context 'when page is 2' do - let(:params) { Hash[page: '2'] } + context "when page is 2" do + let(:params) { Hash[page: "2"] } it { expect(show_prev_page_tag?).to be true } end end - describe '#current_page_tag' do - context 'when page is 1' do - let(:params) { Hash[page: '1'] } - it { expect(current_page_tag).to eql '1' } + describe "#current_page_tag" do + context "when page is 1" do + let(:params) { Hash[page: "1"] } + it { expect(current_page_tag).to eql "1" } end - context 'when page is 2' do - let(:params) { Hash[page: '2'] } - it { expect(current_page_tag).to eql '2' } + context "when page is 2" do + let(:params) { Hash[page: "2"] } + it { expect(current_page_tag).to eql "2" } end end - describe '#show_next_page_tag?' do - context 'when page is 2' do - let(:params) { Hash[page: '2'] } + describe "#show_next_page_tag?" do + context "when page is 2" do + let(:params) { Hash[page: "2"] } - context 'when collection length is 20' do + context "when collection length is 20" do let(:collection) { (1..20).to_a } it { expect(show_next_page_tag?(collection)).to be false } end - context 'when collection length is 25' do + context "when collection length is 25" do let(:collection) { (1..25).to_a } it { expect(show_next_page_tag?(collection)).to be true } end end end - describe '#next_page_tag' do - context 'when page is 1' do - let(:params) { Hash[page: '1'] } + describe "#next_page_tag" do + context "when page is 1" do + let(:params) { Hash[page: "1"] } it { expect(next_page_tag).to eql '2' } end - context 'when page is 2' do - let(:params) { Hash[page: '2'] } + context "when page is 2" do + let(:params) { Hash[page: "2"] } it { expect(next_page_tag).to eql '3' } end end - describe '#total_pages' do - context 'when collection length is 30' do + describe "#total_pages" do + context "when collection length is 30" do let(:collection) { (1..30).to_a } it { expect(total_pages(collection)).to eql 3 } end - context 'when collection length is 35' do + context "when collection length is 35" do let(:collection) { (1..35).to_a } it { expect(total_pages(collection)).to eql 4 } end end - describe '#show_last_ellipsis_tag?' do + describe "#show_last_ellipsis_tag?" do let(:collection) { (1..30).to_a } - let(:params) { Hash[per: '5', page: '2'] } + let(:params) { Hash[per: "5", page: "2"] } it { expect(show_last_ellipsis_tag?(collection)).to be true } end - describe '#show_last_page_tag?' do + describe "#show_last_page_tag?" do let(:collection) { (1..30).to_a } - context 'when page is 5/6' do - let(:params) { Hash[per: '5', page: '5'] } + context "when page is 5/6" do + let(:params) { Hash[per: "5", page: "5"] } it { expect(show_last_page_tag?(collection)).to be false } end - context 'when page is 4/6' do - let(:params) { Hash[per: '5', page: '4'] } + context "when page is 4/6" do + let(:params) { Hash[per: "5", page: "4"] } it { expect(show_last_page_tag?(collection)).to be true } end end - describe '#last_page_tag' do + describe "#last_page_tag" do let(:collection) { (1..30).to_a } it { expect(last_page_tag(collection)).to eql '3' } end diff --git a/spec/dashboard/paginator_spec.rb b/spec/dashboard/paginator_spec.rb index b46bffe8..634a6433 100644 --- a/spec/dashboard/paginator_spec.rb +++ b/spec/dashboard/paginator_spec.rb @@ -1,34 +1,35 @@ # frozen_string_literal: true -require 'spec_helper' -require 'split/dashboard/paginator' + +require "spec_helper" +require "split/dashboard/paginator" describe Split::DashboardPaginator do - context 'when collection is 1..20' do + context "when collection is 1..20" do let(:collection) { (1..20).to_a } - context 'when per 5 for page' do + context "when per 5 for page" do let(:per) { 5 } - it 'when page number is 1 result is [1, 2, 3, 4, 5]' do + it "when page number is 1 result is [1, 2, 3, 4, 5]" do result = Split::DashboardPaginator.new(collection, 1, per).paginate expect(result).to eql [1, 2, 3, 4, 5] end - it 'when page number is 2 result is [6, 7, 8, 9, 10]' do + it "when page number is 2 result is [6, 7, 8, 9, 10]" do result = Split::DashboardPaginator.new(collection, 2, per).paginate expect(result).to eql [6, 7, 8, 9, 10] end end - context 'when per 10 for page' do + context "when per 10 for page" do let(:per) { 10 } - it 'when page number is 1 result is [1..10]' do + it "when page number is 1 result is [1..10]" do result = Split::DashboardPaginator.new(collection, 1, per).paginate expect(result).to eql [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] end - it 'when page number is 2 result is [10..20]' do + it "when page number is 2 result is [10..20]" do result = Split::DashboardPaginator.new(collection, 2, per).paginate expect(result).to eql [11, 12, 13, 14, 15, 16, 17, 18, 19, 20] end diff --git a/spec/dashboard_helpers_spec.rb b/spec/dashboard_helpers_spec.rb index 3643c232..4894db01 100644 --- a/spec/dashboard_helpers_spec.rb +++ b/spec/dashboard_helpers_spec.rb @@ -1,41 +1,42 @@ # frozen_string_literal: true -require 'spec_helper' -require 'split/dashboard/helpers' + +require "spec_helper" +require "split/dashboard/helpers" include Split::DashboardHelpers describe Split::DashboardHelpers do - describe 'confidence_level' do - it 'should handle very small numbers' do - expect(confidence_level(Complex(2e-18, -0.03))).to eq('Insufficient confidence') + describe "confidence_level" do + it "should handle very small numbers" do + expect(confidence_level(Complex(2e-18, -0.03))).to eq("Insufficient confidence") end it "should consider a z-score of 1.65 <= z < 1.96 as 90% confident" do - expect(confidence_level(1.65)).to eq('90% confidence') - expect(confidence_level(1.80)).to eq('90% confidence') + expect(confidence_level(1.65)).to eq("90% confidence") + expect(confidence_level(1.80)).to eq("90% confidence") end it "should consider a z-score of 1.96 <= z < 2.58 as 95% confident" do - expect(confidence_level(1.96)).to eq('95% confidence') - expect(confidence_level(2.00)).to eq('95% confidence') + expect(confidence_level(1.96)).to eq("95% confidence") + expect(confidence_level(2.00)).to eq("95% confidence") end it "should consider a z-score of z >= 2.58 as 99% confident" do - expect(confidence_level(2.58)).to eq('99% confidence') - expect(confidence_level(3.00)).to eq('99% confidence') + expect(confidence_level(2.58)).to eq("99% confidence") + expect(confidence_level(3.00)).to eq("99% confidence") end - describe '#round' do - it 'can round number strings' do - expect(round('3.1415')).to eq BigDecimal('3.14') + describe "#round" do + it "can round number strings" do + expect(round("3.1415")).to eq BigDecimal("3.14") end - it 'can round number strings for precsion' do - expect(round('3.1415', 1)).to eq BigDecimal('3.1') + it "can round number strings for precsion" do + expect(round("3.1415", 1)).to eq BigDecimal("3.1") end - it 'can handle invalid number strings' do - expect(round('N/A')).to be_zero + it "can handle invalid number strings" do + expect(round("N/A")).to be_zero end end end diff --git a/spec/dashboard_spec.rb b/spec/dashboard_spec.rb index 267f1386..ed0fcd40 100644 --- a/spec/dashboard_spec.rb +++ b/spec/dashboard_spec.rb @@ -1,13 +1,22 @@ # frozen_string_literal: true -require 'spec_helper' -require 'rack/test' -require 'split/dashboard' + +require "spec_helper" +require "rack/test" +require "split/dashboard" describe Split::Dashboard do include Rack::Test::Methods + class TestDashboard < Split::Dashboard + include Split::Helper + + get "/my_experiment" do + ab_test(params[:experiment], "blue", "red") + end + end + def app - @app ||= Split::Dashboard + @app ||= TestDashboard end def link(color) @@ -19,11 +28,11 @@ def link(color) } let(:experiment_with_goals) { - Split::ExperimentCatalog.find_or_create({"link_color" => ["goal_1", "goal_2"]}, "blue", "red") + Split::ExperimentCatalog.find_or_create({ "link_color" => ["goal_1", "goal_2"] }, "blue", "red") } let(:metric) { - Split::Metric.find_or_create(name: 'testmetric', experiments: [experiment, experiment_with_goals]) + Split::Metric.find_or_create(name: "testmetric", experiments: [experiment, experiment_with_goals]) } let(:red_link) { link("red") } @@ -34,7 +43,7 @@ def link(color) end it "should respond to /" do - get '/' + get "/" expect(last_response).to be_ok end @@ -46,33 +55,33 @@ def link(color) context "experiment without goals" do it "should display a Start button" do experiment - get '/' - expect(last_response.body).to include('Start') + get "/" + expect(last_response.body).to include("Start") post "/start?experiment=#{experiment.name}" - get '/' - expect(last_response.body).to include('Reset Data') - expect(last_response.body).not_to include('Metrics:') + get "/" + expect(last_response.body).to include("Reset Data") + expect(last_response.body).not_to include("Metrics:") end end context "experiment with metrics" do it "should display the names of associated metrics" do metric - get '/' - expect(last_response.body).to include('Metrics:testmetric') + get "/" + expect(last_response.body).to include("Metrics:testmetric") end end context "with goals" do it "should display a Start button" do experiment_with_goals - get '/' - expect(last_response.body).to include('Start') + get "/" + expect(last_response.body).to include("Start") post "/start?experiment=#{experiment.name}" - get '/' - expect(last_response.body).to include('Reset Data') + get "/" + expect(last_response.body).to include("Reset Data") end end end @@ -80,7 +89,7 @@ def link(color) describe "force alternative" do context "initial version" do let!(:user) do - Split::User.new(@app, { experiment.name => 'red' }) + Split::User.new(@app, { experiment.name => "red" }) end before do @@ -90,15 +99,24 @@ def link(color) it "should set current user's alternative" do blue_link.participant_count = 7 post "/force_alternative?experiment=#{experiment.name}", alternative: "blue" - expect(user[experiment.key]).to eq("blue") - expect(blue_link.participant_count).to eq(8) + + get "/my_experiment?experiment=#{experiment.name}" + expect(last_response.body).to include("blue") + end + + it "should not modify an existing user" do + blue_link.participant_count = 7 + post "/force_alternative?experiment=#{experiment.name}", alternative: "blue" + + expect(user[experiment.key]).to eq("red") + expect(blue_link.participant_count).to eq(7) end end context "incremented version" do let!(:user) do experiment.increment_version - Split::User.new(@app, { "#{experiment.name}:#{experiment.version}" => 'red' }) + Split::User.new(@app, { "#{experiment.name}:#{experiment.version}" => "red" }) end before do @@ -108,36 +126,37 @@ def link(color) it "should set current user's alternative" do blue_link.participant_count = 7 post "/force_alternative?experiment=#{experiment.name}", alternative: "blue" - expect(user[experiment.key]).to eq("blue") - expect(blue_link.participant_count).to eq(8) + + get "/my_experiment?experiment=#{experiment.name}" + expect(last_response.body).to include("blue") end end end describe "index page" do context "with winner" do - before { experiment.winner = 'red' } + before { experiment.winner = "red" } it "displays `Reopen Experiment` button" do - get '/' + get "/" - expect(last_response.body).to include('Reopen Experiment') + expect(last_response.body).to include("Reopen Experiment") end end context "without winner" do it "should not display `Reopen Experiment` button" do - get '/' + get "/" - expect(last_response.body).to_not include('Reopen Experiment') + expect(last_response.body).to_not include("Reopen Experiment") end end end describe "reopen experiment" do - before { experiment.winner = 'red' } + before { experiment.winner = "red" } - it 'redirects' do + it "redirects" do post "/reopen?experiment=#{experiment.name}" expect(last_response).to be_redirect @@ -152,7 +171,7 @@ def link(color) it "keeps existing stats" do red_link.participant_count = 5 blue_link.participant_count = 7 - experiment.winner = 'blue' + experiment.winner = "blue" post "/reopen?experiment=#{experiment.name}" @@ -170,7 +189,6 @@ def link(color) it "calls disable of cohorting when action is disable" do post "/update_cohorting?experiment=#{experiment.name}", { "cohorting_action": "disable" } - expect(experiment.cohorting_disabled?).to eq true end @@ -184,10 +202,10 @@ def link(color) end describe "initialize experiment" do - before do + before do Split.configuration.experiments = { - :my_experiment => { - :alternatives => [ "control", "alternative" ], + my_experiment: { + alternatives: [ "control", "alternative" ], } } end @@ -195,29 +213,29 @@ def link(color) it "initializes the experiment when the experiment is given" do expect(Split::ExperimentCatalog.find("my_experiment")).to be nil - post "/initialize_experiment", { experiment: "my_experiment"} + post "/initialize_experiment", { experiment: "my_experiment" } experiment = Split::ExperimentCatalog.find("my_experiment") expect(experiment).to be_a(Split::Experiment) end it "does not attempt to intialize the experiment when empty experiment is given" do - expect(Split::ExperimentCatalog).to_not receive(:find_or_create) + post "/initialize_experiment", { experiment: "" } - post "/initialize_experiment", { experiment: ""} + expect(Split::ExperimentCatalog).to_not receive(:find_or_create) end it "does not attempt to intialize the experiment when no experiment is given" do - expect(Split::ExperimentCatalog).to_not receive(:find_or_create) - post "/initialize_experiment" + + expect(Split::ExperimentCatalog).to_not receive(:find_or_create) end end it "should reset an experiment" do red_link.participant_count = 5 blue_link.participant_count = 7 - experiment.winner = 'blue' + experiment.winner = "blue" post "/reset?experiment=#{experiment.name}" @@ -239,16 +257,16 @@ def link(color) it "should mark an alternative as the winner" do expect(experiment.winner).to be_nil - post "/experiment?experiment=#{experiment.name}", :alternative => 'red' + post "/experiment?experiment=#{experiment.name}", alternative: "red" expect(last_response).to be_redirect - expect(experiment.winner.name).to eq('red') + expect(experiment.winner.name).to eq("red") end it "should display the start date" do experiment.start - get '/' + get "/" expect(last_response.body).to include("#{experiment.start_time.strftime('%Y-%m-%d')}") end @@ -256,8 +274,8 @@ def link(color) it "should handle experiments without a start date" do Split.redis.hdel(:experiment_start_times, experiment.name) - get '/' + get "/" - expect(last_response.body).to include('Unknown') + expect(last_response.body).to include("Unknown") end end diff --git a/spec/encapsulated_helper_spec.rb b/spec/encapsulated_helper_spec.rb index a39d98c9..df9dfa9a 100644 --- a/spec/encapsulated_helper_spec.rb +++ b/spec/encapsulated_helper_spec.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true -require 'spec_helper' + +require "spec_helper" describe Split::EncapsulatedHelper do include Split::EncapsulatedHelper - def params - raise NoMethodError, 'This method is not really defined' + raise NoMethodError, "This method is not really defined" end describe "ab_test" do @@ -16,37 +16,35 @@ def params end it "should not raise an error when params raises an error" do - expect{ params }.to raise_error(NoMethodError) - expect(lambda { ab_test('link_color', 'blue', 'red') }).not_to raise_error + expect { params }.to raise_error(NoMethodError) + expect { ab_test("link_color", "blue", "red") }.not_to raise_error end it "calls the block with selected alternative" do - expect{|block| ab_test('link_color', 'red', 'red', &block) }.to yield_with_args('red', {}) + expect { |block| ab_test("link_color", "red", "red", &block) }.to yield_with_args("red", {}) end context "inside a view" do - it "works inside ERB" do - require 'erb' - template = ERB.new(<<-ERB.split(/\s+/s).map(&:strip).join(' '), nil, "%") + require "erb" + template = ERB.new(<<-ERB.split(/\s+/s).map(&:strip).join(" "), nil, "%") foo <% ab_test(:foo, '1', '2') do |alt, meta| %> static <%= alt %> <% end %> ERB expect(template.result(binding)).to match(/foo static \d/) end - end end describe "context" do - it 'is passed in shim' do - ctx = Class.new{ + it "is passed in shim" do + ctx = Class.new { include Split::EncapsulatedHelper public :session }.new - expect(ctx).to receive(:session){{}} - expect{ ctx.ab_test('link_color', 'blue', 'red') }.not_to raise_error + expect(ctx).to receive(:session) { {} } + expect { ctx.ab_test("link_color", "blue", "red") }.not_to raise_error end end end diff --git a/spec/experiment_catalog_spec.rb b/spec/experiment_catalog_spec.rb index 16cc837c..192d0ffc 100644 --- a/spec/experiment_catalog_spec.rb +++ b/spec/experiment_catalog_spec.rb @@ -1,53 +1,54 @@ # frozen_string_literal: true -require 'spec_helper' + +require "spec_helper" describe Split::ExperimentCatalog do subject { Split::ExperimentCatalog } describe ".find_or_create" do it "should not raise an error when passed strings for alternatives" do - expect { subject.find_or_create('xyz', '1', '2', '3') }.not_to raise_error + expect { subject.find_or_create("xyz", "1", "2", "3") }.not_to raise_error end it "should not raise an error when passed an array for alternatives" do - expect { subject.find_or_create('xyz', ['1', '2', '3']) }.not_to raise_error + expect { subject.find_or_create("xyz", ["1", "2", "3"]) }.not_to raise_error end it "should raise the appropriate error when passed integers for alternatives" do - expect { subject.find_or_create('xyz', 1, 2, 3) }.to raise_error(ArgumentError) + expect { subject.find_or_create("xyz", 1, 2, 3) }.to raise_error(ArgumentError) end it "should raise the appropriate error when passed symbols for alternatives" do - expect { subject.find_or_create('xyz', :a, :b, :c) }.to raise_error(ArgumentError) + expect { subject.find_or_create("xyz", :a, :b, :c) }.to raise_error(ArgumentError) end it "should not raise error when passed an array for goals" do - expect { subject.find_or_create({'link_color' => ["purchase", "refund"]}, 'blue', 'red') } + expect { subject.find_or_create({ "link_color" => ["purchase", "refund"] }, "blue", "red") } .not_to raise_error end it "should not raise error when passed just one goal" do - expect { subject.find_or_create({'link_color' => "purchase"}, 'blue', 'red') } + expect { subject.find_or_create({ "link_color" => "purchase" }, "blue", "red") } .not_to raise_error end it "constructs a new experiment" do - expect(subject.find_or_create('my_exp', 'control me').control.to_s).to eq('control me') + expect(subject.find_or_create("my_exp", "control me").control.to_s).to eq("control me") end end - describe '.find' do + describe ".find" do it "should return an existing experiment" do - experiment = Split::Experiment.new('basket_text', alternatives: ['blue', 'red', 'green']) + experiment = Split::Experiment.new("basket_text", alternatives: ["blue", "red", "green"]) experiment.save - experiment = subject.find('basket_text') + experiment = subject.find("basket_text") - expect(experiment.name).to eq('basket_text') + expect(experiment.name).to eq("basket_text") end it "should return nil if experiment not exist" do - expect(subject.find('non_existent_experiment')).to be_nil + expect(subject.find("non_existent_experiment")).to be_nil end end end diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index 80a5b42e..77035221 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -1,14 +1,15 @@ # frozen_string_literal: true -require 'spec_helper' -require 'time' + +require "spec_helper" +require "time" describe Split::Experiment do - def new_experiment(goals=[]) - Split::Experiment.new('link_color', :alternatives => ['blue', 'red', 'green'], :goals => goals) + def new_experiment(goals = []) + Split::Experiment.new("link_color", alternatives: ["blue", "red", "green"], goals: goals) end def alternative(color) - Split::Alternative.new(color, 'link_color') + Split::Alternative.new(color, "link_color") end let(:experiment) { new_experiment } @@ -17,10 +18,10 @@ def alternative(color) let(:green) { alternative("green") } context "with an experiment" do - let(:experiment) { Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"]) } + let(:experiment) { Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"]) } it "should have a name" do - expect(experiment.name).to eq('basket_text') + expect(experiment.name).to eq("basket_text") end it "should have alternatives" do @@ -28,7 +29,7 @@ def alternative(color) end it "should have alternatives with correct names" do - expect(experiment.alternatives.collect{|a| a.name}).to eq(['Basket', 'Cart']) + expect(experiment.alternatives.collect { |a| a.name }).to eq(["Basket", "Cart"]) end it "should be resettable by default" do @@ -41,7 +42,7 @@ def alternative(color) it "should save to redis" do experiment.save - expect(Split.redis.exists('basket_text')).to be true + expect(Split.redis.exists?("basket_text")).to be true end it "should save the start time to redis" do @@ -49,14 +50,14 @@ def alternative(color) expect(Time).to receive(:now).and_return(experiment_start_time) experiment.save - expect(Split::ExperimentCatalog.find('basket_text').start_time).to eq(experiment_start_time) + expect(Split::ExperimentCatalog.find("basket_text").start_time).to eq(experiment_start_time) end it "should not save the start time to redis when start_manually is enabled" do expect(Split.configuration).to receive(:start_manually).and_return(true) experiment.save - expect(Split::ExperimentCatalog.find('basket_text').start_time).to be_nil + expect(Split::ExperimentCatalog.find("basket_text").start_time).to be_nil end it "should save the selected algorithm to redis" do @@ -64,7 +65,7 @@ def alternative(color) experiment.algorithm = experiment_algorithm experiment.save - expect(Split::ExperimentCatalog.find('basket_text').algorithm).to eq(experiment_algorithm) + expect(Split::ExperimentCatalog.find("basket_text").algorithm).to eq(experiment_algorithm) end it "should handle having a start time stored as a string" do @@ -73,7 +74,7 @@ def alternative(color) experiment.save Split.redis.hset(:experiment_start_times, experiment.name, experiment_start_time) - expect(Split::ExperimentCatalog.find('basket_text').start_time).to eq(experiment_start_time) + expect(Split::ExperimentCatalog.find("basket_text").start_time).to eq(experiment_start_time) end it "should handle not having a start time" do @@ -83,17 +84,17 @@ def alternative(color) Split.redis.hdel(:experiment_start_times, experiment.name) - expect(Split::ExperimentCatalog.find('basket_text').start_time).to be_nil + expect(Split::ExperimentCatalog.find("basket_text").start_time).to be_nil end it "should not create duplicates when saving multiple times" do experiment.save experiment.save - expect(Split.redis.exists('basket_text')).to be true - expect(Split.redis.lrange('basket_text', 0, -1)).to eq(['{"Basket":1}', '{"Cart":1}']) + expect(Split.redis.exists?("basket_text")).to be true + expect(Split.redis.lrange("basket_text", 0, -1)).to eq(['{"Basket":1}', '{"Cart":1}']) end - describe 'new record?' do + describe "new record?" do it "should know if it hasn't been saved yet" do expect(experiment.new_record?).to be_truthy end @@ -104,49 +105,49 @@ def alternative(color) end end - describe 'control' do - it 'should be the first alternative' do + describe "control" do + it "should be the first alternative" do experiment.save - expect(experiment.control.name).to eq('Basket') + expect(experiment.control.name).to eq("Basket") end end end - describe 'initialization' do + describe "initialization" do it "should set the algorithm when passed as an option to the initializer" do - experiment = Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"], :algorithm => Split::Algorithms::Whiplash) - expect(experiment.algorithm).to eq(Split::Algorithms::Whiplash) + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], algorithm: Split::Algorithms::Whiplash) + expect(experiment.algorithm).to eq(Split::Algorithms::Whiplash) end it "should be possible to make an experiment not resettable" do - experiment = Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"], :resettable => false) + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], resettable: false) expect(experiment.resettable).to be_falsey end it "should be possible to make an experiment retain user alternatives after reset" do - experiment = Split::Experiment.new("basket_text", :alternatives => ["Basket", "Cart"], :retain_user_alternatives_after_reset => true) + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], retain_user_alternatives_after_reset: true) expect(experiment.retain_user_alternatives_after_reset).to be_truthy end it "sets friendly_name" do - experiment = Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"], :friendly_name => "foo") + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], friendly_name: "foo") expect(experiment.friendly_name).to eq("foo") end - context 'from configuration' do + context "from configuration" do let(:experiment_name) { :my_experiment } let(:experiments) do { experiment_name => { - :alternatives => ['Control Opt', 'Alt one'], - :friendly_name => "foo" + alternatives: ["Control Opt", "Alt one"], + friendly_name: "foo" } } end before { Split.configuration.experiments = experiments } - it 'assigns default values to the experiment' do + it "assigns default values to the experiment" do expect(Split::Experiment.new(experiment_name).resettable).to eq(true) expect(Split::Experiment.new(experiment_name).retain_user_alternatives_after_reset).to eq(false) end @@ -156,28 +157,26 @@ def alternative(color) end end - context 'when no friendly name is defined' do + context "when no friendly name is defined" do it "defaults to experiment name" do - experiment = Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"], :resettable => false) + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], resettable: false) expect(experiment.friendly_name).to eql("basket_text") end end end - describe 'persistent configuration' do - + describe "persistent configuration" do it "should persist resettable in redis" do - experiment = Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"], :resettable => false) + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], resettable: false) experiment.save - e = Split::ExperimentCatalog.find('basket_text') + e = Split::ExperimentCatalog.find("basket_text") expect(e).to eq(experiment) expect(e.resettable).to be_falsey - end it "should persist retain_user_alternatives_after_reset in redis" do - experiment = Split::Experiment.new("basket_text", :alternatives => ['Basket', "Cart"], :retain_user_alternatives_after_reset => true) + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], retain_user_alternatives_after_reset: true) experiment.save e = Split::ExperimentCatalog.find("basket_text") @@ -185,23 +184,35 @@ def alternative(color) expect(e.retain_user_alternatives_after_reset).to be_truthy end - describe '#metadata' do - let(:experiment) { Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"], :algorithm => Split::Algorithms::Whiplash, :metadata => meta) } - context 'simple hash' do - let(:meta) { { 'basket' => 'a', 'cart' => 'b' } } + describe "#metadata" do + let(:experiment) { Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], algorithm: Split::Algorithms::Whiplash, metadata: meta) } + let(:meta) { { a: "b" } } + + before do + experiment.save + end + + it "should delete the key when metadata is removed" do + experiment.metadata = nil + experiment.save + + expect(Split.redis.exists?(experiment.metadata_key)).to be_falsey + end + + context "simple hash" do + let(:meta) { { "basket" => "a", "cart" => "b" } } + it "should persist metadata in redis" do - experiment.save - e = Split::ExperimentCatalog.find('basket_text') + e = Split::ExperimentCatalog.find("basket_text") expect(e).to eq(experiment) expect(e.metadata).to eq(meta) end end - context 'nested hash' do - let(:meta) { { 'basket' => { 'one' => 'two' }, 'cart' => 'b' } } + context "nested hash" do + let(:meta) { { "basket" => { "one" => "two" }, "cart" => "b" } } it "should persist metadata in redis" do - experiment.save - e = Split::ExperimentCatalog.find('basket_text') + e = Split::ExperimentCatalog.find("basket_text") expect(e).to eq(experiment) expect(e.metadata).to eq(meta) end @@ -209,32 +220,32 @@ def alternative(color) end it "should persist algorithm in redis" do - experiment = Split::Experiment.new('basket_text', :alternatives => ['Basket', "Cart"], :algorithm => Split::Algorithms::Whiplash) + experiment = Split::Experiment.new("basket_text", alternatives: ["Basket", "Cart"], algorithm: Split::Algorithms::Whiplash) experiment.save - e = Split::ExperimentCatalog.find('basket_text') + e = Split::ExperimentCatalog.find("basket_text") expect(e).to eq(experiment) expect(e.algorithm).to eq(Split::Algorithms::Whiplash) end it "should persist a new experiment in redis, that does not exist in the configuration file" do - experiment = Split::Experiment.new('foobar', :alternatives => ['tra', 'la'], :algorithm => Split::Algorithms::Whiplash) + experiment = Split::Experiment.new("foobar", alternatives: ["tra", "la"], algorithm: Split::Algorithms::Whiplash) experiment.save - e = Split::ExperimentCatalog.find('foobar') + e = Split::ExperimentCatalog.find("foobar") expect(e).to eq(experiment) - expect(e.alternatives.collect{|a| a.name}).to eq(['tra', 'la']) + expect(e.alternatives.collect { |a| a.name }).to eq(["tra", "la"]) end end - describe 'deleting' do - it 'should delete itself' do - experiment = Split::Experiment.new('basket_text', :alternatives => [ 'Basket', "Cart"]) + describe "deleting" do + it "should delete itself" do + experiment = Split::Experiment.new("basket_text", alternatives: [ "Basket", "Cart"]) experiment.save experiment.delete - expect(Split.redis.exists('link_color')).to be false - expect(Split::ExperimentCatalog.find('link_color')).to be_nil + expect(Split.redis.exists?("link_color")).to be false + expect(Split::ExperimentCatalog.find("link_color")).to be_nil end it "should increment the version" do @@ -253,7 +264,7 @@ def alternative(color) experiment.delete end - it 'should reset the start time if the experiment should be manually started' do + it "should reset the start time if the experiment should be manually started" do Split.configuration.start_manually = true experiment.start experiment.delete @@ -268,75 +279,75 @@ def alternative(color) end end - describe 'winner' do + describe "winner" do it "should have no winner initially" do expect(experiment.winner).to be_nil end end - describe 'winner=' do - it 'should allow you to specify a winner' do + describe "winner=" do + it "should allow you to specify a winner" do experiment.save - experiment.winner = 'red' - expect(experiment.winner.name).to eq('red') + experiment.winner = "red" + expect(experiment.winner.name).to eq("red") end - it 'should call the on_experiment_winner_choose hook' do + it "should call the on_experiment_winner_choose hook" do expect(Split.configuration.on_experiment_winner_choose).to receive(:call) - experiment.winner = 'green' + experiment.winner = "green" end - context 'when has_winner state is memoized' do + context "when has_winner state is memoized" do before { expect(experiment).to_not have_winner } - it 'should keep has_winner state consistent' do - experiment.winner = 'red' + it "should keep has_winner state consistent" do + experiment.winner = "red" expect(experiment).to have_winner end end end - describe 'reset_winner' do - before { experiment.winner = 'green' } + describe "reset_winner" do + before { experiment.winner = "green" } - it 'should reset the winner' do + it "should reset the winner" do experiment.reset_winner expect(experiment.winner).to be_nil end - context 'when has_winner state is memoized' do + context "when has_winner state is memoized" do before { expect(experiment).to have_winner } - it 'should keep has_winner state consistent' do + it "should keep has_winner state consistent" do experiment.reset_winner expect(experiment).to_not have_winner end end end - describe 'has_winner?' do - context 'with winner' do - before { experiment.winner = 'red' } + describe "has_winner?" do + context "with winner" do + before { experiment.winner = "red" } - it 'returns true' do + it "returns true" do expect(experiment).to have_winner end end - context 'without winner' do - it 'returns false' do + context "without winner" do + it "returns false" do expect(experiment).to_not have_winner end end - it 'memoizes has_winner state' do + it "memoizes has_winner state" do expect(experiment).to receive(:winner).once expect(experiment).to_not have_winner expect(experiment).to_not have_winner end end - describe 'reset' do + describe "reset" do let(:reset_manually) { false } before do @@ -346,10 +357,10 @@ def alternative(color) green.increment_participation end - it 'should reset all alternatives' do - experiment.winner = 'green' + it "should reset all alternatives" do + experiment.winner = "green" - expect(experiment.next_alternative.name).to eq('green') + expect(experiment.next_alternative.name).to eq("green") green.increment_participation experiment.reset @@ -358,10 +369,10 @@ def alternative(color) expect(green.completed_count).to eq(0) end - it 'should reset the winner' do - experiment.winner = 'green' + it "should reset the winner" do + experiment.winner = "green" - expect(experiment.next_alternative.name).to eq('green') + expect(experiment.next_alternative.name).to eq("green") green.increment_participation experiment.reset @@ -386,14 +397,14 @@ def alternative(color) end end - describe 'algorithm' do - let(:experiment) { Split::ExperimentCatalog.find_or_create('link_color', 'blue', 'red', 'green') } + describe "algorithm" do + let(:experiment) { Split::ExperimentCatalog.find_or_create("link_color", "blue", "red", "green") } - it 'should use the default algorithm if none is specified' do + it "should use the default algorithm if none is specified" do expect(experiment.algorithm).to eq(Split.configuration.algorithm) end - it 'should use the user specified algorithm for this experiment if specified' do + it "should use the user specified algorithm for this experiment if specified" do experiment.algorithm = Split::Algorithms::Whiplash expect(experiment.algorithm).to eq(Split::Algorithms::Whiplash) end @@ -427,37 +438,37 @@ def alternative(color) end end - describe '#next_alternative' do - context 'with multiple alternatives' do - let(:experiment) { Split::ExperimentCatalog.find_or_create('link_color', 'blue', 'red', 'green') } + describe "#next_alternative" do + context "with multiple alternatives" do + let(:experiment) { Split::ExperimentCatalog.find_or_create("link_color", "blue", "red", "green") } - context 'with winner' do + context "with winner" do it "should always return the winner" do - green = Split::Alternative.new('green', 'link_color') - experiment.winner = 'green' + green = Split::Alternative.new("green", "link_color") + experiment.winner = "green" - expect(experiment.next_alternative.name).to eq('green') + expect(experiment.next_alternative.name).to eq("green") green.increment_participation - expect(experiment.next_alternative.name).to eq('green') + expect(experiment.next_alternative.name).to eq("green") end end - context 'without winner' do + context "without winner" do it "should use the specified algorithm" do experiment.algorithm = Split::Algorithms::Whiplash - expect(experiment.algorithm).to receive(:choose_alternative).and_return(Split::Alternative.new('green', 'link_color')) - expect(experiment.next_alternative.name).to eq('green') + expect(experiment.algorithm).to receive(:choose_alternative).and_return(Split::Alternative.new("green", "link_color")) + expect(experiment.next_alternative.name).to eq("green") end end end - context 'with single alternative' do - let(:experiment) { Split::ExperimentCatalog.find_or_create('link_color', 'blue') } + context "with single alternative" do + let(:experiment) { Split::ExperimentCatalog.find_or_create("link_color", "blue") } it "should always return the only alternative" do - expect(experiment.next_alternative.name).to eq('blue') - expect(experiment.next_alternative.name).to eq('blue') + expect(experiment.next_alternative.name).to eq("blue") + expect(experiment.next_alternative.name).to eq("blue") end end end @@ -478,16 +489,16 @@ def alternative(color) end end - describe 'changing an existing experiment' do + describe "changing an existing experiment" do def same_but_different_alternative - Split::ExperimentCatalog.find_or_create('link_color', 'blue', 'yellow', 'orange') + Split::ExperimentCatalog.find_or_create("link_color", "blue", "yellow", "orange") end it "should reset an experiment if it is loaded with different alternatives" do experiment.save blue.participant_count = 5 same_experiment = same_but_different_alternative - expect(same_experiment.alternatives.map(&:name)).to eq(['blue', 'yellow', 'orange']) + expect(same_experiment.alternatives.map(&:name)).to eq(["blue", "yellow", "orange"]) expect(blue.participant_count).to eq(0) end @@ -500,7 +511,22 @@ def same_but_different_alternative expect(same_experiment_again.version).to eq(1) end - context 'when experiment configuration is changed' do + context "when metadata is changed" do + it "should increase version" do + experiment.save + experiment.metadata = { "foo" => "bar" } + + expect { experiment.save }.to change { experiment.version }.by(1) + end + + it "does not increase version" do + experiment.metadata = nil + experiment.save + expect { experiment.save }.to change { experiment.version }.by(0) + end + end + + context "when experiment configuration is changed" do let(:reset_manually) { false } before do @@ -513,15 +539,15 @@ def same_but_different_alternative experiment.save end - it 'resets all alternatives' do + it "resets all alternatives" do expect(green.participant_count).to eq(0) expect(green.completed_count).to eq(0) end - context 'when reset_manually is set' do + context "when reset_manually is set" do let(:reset_manually) { true } - it 'does not reset alternatives' do + it "does not reset alternatives" do expect(green.participant_count).to eq(2) expect(green.completed_count).to eq(0) end @@ -529,16 +555,16 @@ def same_but_different_alternative end end - describe 'alternatives passed as non-strings' do + describe "alternatives passed as non-strings" do it "should throw an exception if an alternative is passed that is not a string" do - expect(lambda { Split::ExperimentCatalog.find_or_create('link_color', :blue, :red) }).to raise_error(ArgumentError) - expect(lambda { Split::ExperimentCatalog.find_or_create('link_enabled', true, false) }).to raise_error(ArgumentError) + expect { Split::ExperimentCatalog.find_or_create("link_color", :blue, :red) }.to raise_error(ArgumentError) + expect { Split::ExperimentCatalog.find_or_create("link_enabled", true, false) }.to raise_error(ArgumentError) end end - describe 'specifying weights' do + describe "specifying weights" do let(:experiment_with_weight) { - Split::ExperimentCatalog.find_or_create('link_color', {'blue' => 1}, {'red' => 2 }) + Split::ExperimentCatalog.find_or_create("link_color", { "blue" => 1 }, { "red" => 2 }) } it "should work for a new experiment" do @@ -557,7 +583,7 @@ def same_but_different_alternative } context "saving experiment" do - let(:same_but_different_goals) { Split::ExperimentCatalog.find_or_create({'link_color' => ["purchase", "refund"]}, 'blue', 'red', 'green') } + let(:same_but_different_goals) { Split::ExperimentCatalog.find_or_create({ "link_color" => ["purchase", "refund"] }, "blue", "red", "green") } before { experiment.save } @@ -569,7 +595,6 @@ def same_but_different_alternative same_but_different_goals expect(Split::ExperimentCatalog.find("link_color").goals).to eq(["purchase", "refund"]) end - end it "should have goals" do @@ -578,9 +603,9 @@ def same_but_different_alternative context "find or create experiment" do it "should have correct goals" do - experiment = Split::ExperimentCatalog.find_or_create({'link_color3' => ["purchase", "refund"]}, 'blue', 'red', 'green') + experiment = Split::ExperimentCatalog.find_or_create({ "link_color3" => ["purchase", "refund"] }, "blue", "red", "green") expect(experiment.goals).to eq(["purchase", "refund"]) - experiment = Split::ExperimentCatalog.find_or_create('link_color3', 'blue', 'red', 'green') + experiment = Split::ExperimentCatalog.find_or_create("link_color3", "blue", "red", "green") expect(experiment.goals).to eq([]) end end @@ -588,19 +613,19 @@ def same_but_different_alternative describe "beta probability calculation" do it "should return a hash with the probability of each alternative being the best" do - experiment = Split::ExperimentCatalog.find_or_create('mathematicians', 'bernoulli', 'poisson', 'lagrange') + experiment = Split::ExperimentCatalog.find_or_create("mathematicians", "bernoulli", "poisson", "lagrange") experiment.calc_winning_alternatives expect(experiment.alternative_probabilities).not_to be_nil end it "should return between 46% and 54% probability for an experiment with 2 alternatives and no data" do - experiment = Split::ExperimentCatalog.find_or_create('scientists', 'einstein', 'bohr') + experiment = Split::ExperimentCatalog.find_or_create("scientists", "einstein", "bohr") experiment.calc_winning_alternatives expect(experiment.alternatives[0].p_winner).to be_within(0.04).of(0.50) end - it "should calculate the probability of being the winning alternative separately for each goal", :skip => true do - experiment = Split::ExperimentCatalog.find_or_create({'link_color3' => ["purchase", "refund"]}, 'blue', 'red', 'green') + it "should calculate the probability of being the winning alternative separately for each goal", skip: true do + experiment = Split::ExperimentCatalog.find_or_create({ "link_color3" => ["purchase", "refund"] }, "blue", "red", "green") goal1 = experiment.goals[0] goal2 = experiment.goals[1] experiment.alternatives.each do |alternative| @@ -616,7 +641,7 @@ def same_but_different_alternative end it "should return nil and not re-calculate probabilities if they have already been calculated today" do - experiment = Split::ExperimentCatalog.find_or_create({'link_color3' => ["purchase", "refund"]}, 'blue', 'red', 'green') + experiment = Split::ExperimentCatalog.find_or_create({ "link_color3" => ["purchase", "refund"] }, "blue", "red", "green") expect(experiment.calc_winning_alternatives).not_to be nil expect(experiment.calc_winning_alternatives).to be nil end diff --git a/spec/goals_collection_spec.rb b/spec/goals_collection_spec.rb index 6c5807dd..26f9d223 100644 --- a/spec/goals_collection_spec.rb +++ b/spec/goals_collection_spec.rb @@ -1,41 +1,43 @@ -require 'spec_helper' -require 'split/goals_collection' -require 'time' +# frozen_string_literal: true + +require "spec_helper" +require "split/goals_collection" +require "time" describe Split::GoalsCollection do - let(:experiment_name) { 'experiment_name' } + let(:experiment_name) { "experiment_name" } - describe 'initialization' do + describe "initialization" do let(:goals_collection) { - Split::GoalsCollection.new('experiment_name', ['goal1', 'goal2']) + Split::GoalsCollection.new("experiment_name", ["goal1", "goal2"]) } it "should have an experiment_name" do expect(goals_collection.instance_variable_get(:@experiment_name)). - to eq('experiment_name') + to eq("experiment_name") end it "should have a list of goals" do expect(goals_collection.instance_variable_get(:@goals)). - to eq(['goal1', 'goal2']) + to eq(["goal1", "goal2"]) end end describe "#validate!" do it "should't raise ArgumentError if @goals is nil?" do - goals_collection = Split::GoalsCollection.new('experiment_name') + goals_collection = Split::GoalsCollection.new("experiment_name") expect { goals_collection.validate! }.not_to raise_error end it "should raise ArgumentError if @goals is not an Array" do goals_collection = Split::GoalsCollection. - new('experiment_name', 'not an array') + new("experiment_name", "not an array") expect { goals_collection.validate! }.to raise_error(ArgumentError) end it "should't raise ArgumentError if @goals is an array" do goals_collection = Split::GoalsCollection. - new('experiment_name', ['an array']) + new("experiment_name", ["an array"]) expect { goals_collection.validate! }.not_to raise_error end end @@ -44,11 +46,11 @@ let(:goals_key) { "#{experiment_name}:goals" } it "should delete goals from redis" do - goals_collection = Split::GoalsCollection.new(experiment_name, ['goal1']) + goals_collection = Split::GoalsCollection.new(experiment_name, ["goal1"]) goals_collection.save goals_collection.delete - expect(Split.redis.exists(goals_key)).to be false + expect(Split.redis.exists?(goals_key)).to be false end end @@ -63,7 +65,7 @@ end it "should save goals to redis if @goals is valid" do - goals = ['valid goal 1', 'valid goal 2'] + goals = ["valid goal 1", "valid goal 2"] collection = Split::GoalsCollection.new(experiment_name, goals) collection.save @@ -72,9 +74,9 @@ it "should return @goals if @goals is valid" do goals_collection = Split::GoalsCollection. - new(experiment_name, ['valid goal']) + new(experiment_name, ["valid goal"]) - expect(goals_collection.save).to eq(['valid goal']) + expect(goals_collection.save).to eq(["valid goal"]) end end end diff --git a/spec/helper_spec.rb b/spec/helper_spec.rb index 56068b57..e5104513 100755 --- a/spec/helper_spec.rb +++ b/spec/helper_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' + +require "spec_helper" # TODO change some of these tests to use Rack::Test @@ -7,156 +8,156 @@ include Split::Helper let(:experiment) { - Split::ExperimentCatalog.find_or_create('link_color', 'blue', 'red') + Split::ExperimentCatalog.find_or_create("link_color", "blue", "red") } describe "ab_test" do it "should not raise an error when passed strings for alternatives" do - expect(lambda { ab_test('xyz', '1', '2', '3') }).not_to raise_error + expect { ab_test("xyz", "1", "2", "3") }.not_to raise_error end it "should not raise an error when passed an array for alternatives" do - expect(lambda { ab_test('xyz', ['1', '2', '3']) }).not_to raise_error + expect { ab_test("xyz", ["1", "2", "3"]) }.not_to raise_error end it "should raise the appropriate error when passed integers for alternatives" do - expect(lambda { ab_test('xyz', 1, 2, 3) }).to raise_error(ArgumentError) + expect { ab_test("xyz", 1, 2, 3) }.to raise_error(ArgumentError) end it "should raise the appropriate error when passed symbols for alternatives" do - expect(lambda { ab_test('xyz', :a, :b, :c) }).to raise_error(ArgumentError) + expect { ab_test("xyz", :a, :b, :c) }.to raise_error(ArgumentError) end it "should not raise error when passed an array for goals" do - expect(lambda { ab_test({'link_color' => ["purchase", "refund"]}, 'blue', 'red') }).not_to raise_error + expect { ab_test({ "link_color" => ["purchase", "refund"] }, "blue", "red") }.not_to raise_error end it "should not raise error when passed just one goal" do - expect(lambda { ab_test({'link_color' => "purchase"}, 'blue', 'red') }).not_to raise_error + expect { ab_test({ "link_color" => "purchase" }, "blue", "red") }.not_to raise_error end it "raises an appropriate error when processing combined expirements" do Split.configuration.experiments = { - :combined_exp_1 => { - :alternatives => [ { name: "control", percent: 50 }, { name: "test-alt", percent: 50 } ], - :metric => :my_metric, - :combined_experiments => [:combined_exp_1_sub_1] + combined_exp_1: { + alternatives: [ { name: "control", percent: 50 }, { name: "test-alt", percent: 50 } ], + metric: :my_metric, + combined_experiments: [:combined_exp_1_sub_1] } } - Split::ExperimentCatalog.find_or_create('combined_exp_1') - expect(lambda { ab_test('combined_exp_1')}).to raise_error(Split::InvalidExperimentsFormatError ) + Split::ExperimentCatalog.find_or_create("combined_exp_1") + expect { ab_test("combined_exp_1") }.to raise_error(Split::InvalidExperimentsFormatError) end it "should assign a random alternative to a new user when there are an equal number of alternatives assigned" do - ab_test('link_color', 'blue', 'red') - expect(['red', 'blue']).to include(ab_user['link_color']) + ab_test("link_color", "blue", "red") + expect(["red", "blue"]).to include(ab_user["link_color"]) end it "should increment the participation counter after assignment to a new user" do - previous_red_count = Split::Alternative.new('red', 'link_color').participant_count - previous_blue_count = Split::Alternative.new('blue', 'link_color').participant_count + previous_red_count = Split::Alternative.new("red", "link_color").participant_count + previous_blue_count = Split::Alternative.new("blue", "link_color").participant_count - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") - new_red_count = Split::Alternative.new('red', 'link_color').participant_count - new_blue_count = Split::Alternative.new('blue', 'link_color').participant_count + new_red_count = Split::Alternative.new("red", "link_color").participant_count + new_blue_count = Split::Alternative.new("blue", "link_color").participant_count expect((new_red_count + new_blue_count)).to eq(previous_red_count + previous_blue_count + 1) end - it 'should not increment the counter for an experiment that the user is not participating in' do - ab_test('link_color', 'blue', 'red') - e = Split::ExperimentCatalog.find_or_create('button_size', 'small', 'big') - expect(lambda { + it "should not increment the counter for an experiment that the user is not participating in" do + ab_test("link_color", "blue", "red") + e = Split::ExperimentCatalog.find_or_create("button_size", "small", "big") + expect { # User shouldn't participate in this second experiment - ab_test('button_size', 'small', 'big') - }).not_to change { e.participant_count } + ab_test("button_size", "small", "big") + }.not_to change { e.participant_count } end - it 'should not increment the counter for an ended experiment' do - e = Split::ExperimentCatalog.find_or_create('button_size', 'small', 'big') - e.winner = 'small' - expect(lambda { - a = ab_test('button_size', 'small', 'big') - expect(a).to eq('small') - }).not_to change { e.participant_count } + it "should not increment the counter for an ended experiment" do + e = Split::ExperimentCatalog.find_or_create("button_size", "small", "big") + e.winner = "small" + expect { + a = ab_test("button_size", "small", "big") + expect(a).to eq("small") + }.not_to change { e.participant_count } end - it 'should not increment the counter for an not started experiment' do + it "should not increment the counter for an not started experiment" do expect(Split.configuration).to receive(:start_manually).and_return(true) - e = Split::ExperimentCatalog.find_or_create('button_size', 'small', 'big') - expect(lambda { - a = ab_test('button_size', 'small', 'big') - expect(a).to eq('small') - }).not_to change { e.participant_count } + e = Split::ExperimentCatalog.find_or_create("button_size", "small", "big") + expect { + a = ab_test("button_size", "small", "big") + expect(a).to eq("small") + }.not_to change { e.participant_count } end it "should return the given alternative for an existing user" do - expect(ab_test('link_color', 'blue', 'red')).to eq ab_test('link_color', 'blue', 'red') + expect(ab_test("link_color", "blue", "red")).to eq ab_test("link_color", "blue", "red") end - it 'should always return the winner if one is present' do + it "should always return the winner if one is present" do experiment.winner = "orange" - expect(ab_test('link_color', 'blue', 'red')).to eq('orange') + expect(ab_test("link_color", "blue", "red")).to eq("orange") end it "should allow the alternative to be forced by passing it in the params" do # ?ab_test[link_color]=blue - @params = { 'ab_test' => { 'link_color' => 'blue' } } + @params = { "ab_test" => { "link_color" => "blue" } } - alternative = ab_test('link_color', 'blue', 'red') - expect(alternative).to eq('blue') + alternative = ab_test("link_color", "blue", "red") + expect(alternative).to eq("blue") - alternative = ab_test('link_color', {'blue' => 1}, 'red' => 5) - expect(alternative).to eq('blue') + alternative = ab_test("link_color", { "blue" => 1 }, "red" => 5) + expect(alternative).to eq("blue") - @params = { 'ab_test' => { 'link_color' => 'red' } } + @params = { "ab_test" => { "link_color" => "red" } } - alternative = ab_test('link_color', 'blue', 'red') - expect(alternative).to eq('red') + alternative = ab_test("link_color", "blue", "red") + expect(alternative).to eq("red") - alternative = ab_test('link_color', {'blue' => 5}, 'red' => 1) - expect(alternative).to eq('red') + alternative = ab_test("link_color", { "blue" => 5 }, "red" => 1) + expect(alternative).to eq("red") end it "should not allow an arbitrary alternative" do - @params = { 'ab_test' => { 'link_color' => 'pink' } } - alternative = ab_test('link_color', 'blue') - expect(alternative).to eq('blue') + @params = { "ab_test" => { "link_color" => "pink" } } + alternative = ab_test("link_color", "blue") + expect(alternative).to eq("blue") end it "should not store the split when a param forced alternative" do - @params = { 'ab_test' => { 'link_color' => 'blue' } } + @params = { "ab_test" => { "link_color" => "blue" } } expect(ab_user).not_to receive(:[]=) - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") end it "SPLIT_DISABLE query parameter should also force the alternative (uses control)" do - @params = {'SPLIT_DISABLE' => 'true'} - alternative = ab_test('link_color', 'blue', 'red') - expect(alternative).to eq('blue') - alternative = ab_test('link_color', {'blue' => 1}, 'red' => 5) - expect(alternative).to eq('blue') - alternative = ab_test('link_color', 'red', 'blue') - expect(alternative).to eq('red') - alternative = ab_test('link_color', {'red' => 5}, 'blue' => 1) - expect(alternative).to eq('red') + @params = { "SPLIT_DISABLE" => "true" } + alternative = ab_test("link_color", "blue", "red") + expect(alternative).to eq("blue") + alternative = ab_test("link_color", { "blue" => 1 }, "red" => 5) + expect(alternative).to eq("blue") + alternative = ab_test("link_color", "red", "blue") + expect(alternative).to eq("red") + alternative = ab_test("link_color", { "red" => 5 }, "blue" => 1) + expect(alternative).to eq("red") end it "should not store the split when Split generically disabled" do - @params = {'SPLIT_DISABLE' => 'true'} + @params = { "SPLIT_DISABLE" => "true" } expect(ab_user).not_to receive(:[]=) - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") end context "when store_override is set" do before { Split.configuration.store_override = true } it "should store the forced alternative" do - @params = { 'ab_test' => { 'link_color' => 'blue' } } - expect(ab_user).to receive(:[]=).with('link_color', 'blue') - ab_test('link_color', 'blue', 'red') + @params = { "ab_test" => { "link_color" => "blue" } } + expect(ab_user).to receive(:[]=).with("link_color", "blue") + ab_test("link_color", "blue", "red") end end @@ -164,35 +165,35 @@ before { Split.configuration.on_trial_choose = :some_method } it "should call the method" do expect(self).to receive(:some_method) - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") end end it "should allow passing a block" do - alt = ab_test('link_color', 'blue', 'red') - ret = ab_test('link_color', 'blue', 'red') { |alternative| "shared/#{alternative}" } + alt = ab_test("link_color", "blue", "red") + ret = ab_test("link_color", "blue", "red") { |alternative| "shared/#{alternative}" } expect(ret).to eq("shared/#{alt}") end it "should allow the share of visitors see an alternative to be specified" do - ab_test('link_color', {'blue' => 0.8}, {'red' => 20}) - expect(['red', 'blue']).to include(ab_user['link_color']) + ab_test("link_color", { "blue" => 0.8 }, { "red" => 20 }) + expect(["red", "blue"]).to include(ab_user["link_color"]) end it "should allow alternative weighting interface as a single hash" do - ab_test('link_color', {'blue' => 0.01}, 'red' => 0.2) - experiment = Split::ExperimentCatalog.find('link_color') - expect(experiment.alternatives.map(&:name)).to eq(['blue', 'red']) - expect(experiment.alternatives.collect{|a| a.weight}).to match_array([0.01, 0.2]) + ab_test("link_color", { "blue" => 0.01 }, "red" => 0.2) + experiment = Split::ExperimentCatalog.find("link_color") + expect(experiment.alternatives.map(&:name)).to eq(["blue", "red"]) + expect(experiment.alternatives.collect { |a| a.weight }).to match_array([0.01, 0.2]) end it "should only let a user participate in one experiment at a time" do - link_color = ab_test('link_color', 'blue', 'red') - ab_test('button_size', 'small', 'big') - expect(ab_user['link_color']).to eq(link_color) - big = Split::Alternative.new('big', 'button_size') + link_color = ab_test("link_color", "blue", "red") + ab_test("button_size", "small", "big") + expect(ab_user["link_color"]).to eq(link_color) + big = Split::Alternative.new("big", "button_size") expect(big.participant_count).to eq(0) - small = Split::Alternative.new('small', 'button_size') + small = Split::Alternative.new("small", "button_size") expect(small.participant_count).to eq(0) end @@ -200,49 +201,59 @@ Split.configure do |config| config.allow_multiple_experiments = true end - link_color = ab_test('link_color', 'blue', 'red') - button_size = ab_test('button_size', 'small', 'big') - expect(ab_user['link_color']).to eq(link_color) - expect(ab_user['button_size']).to eq(button_size) - button_size_alt = Split::Alternative.new(button_size, 'button_size') + link_color = ab_test("link_color", "blue", "red") + button_size = ab_test("button_size", "small", "big") + expect(ab_user["link_color"]).to eq(link_color) + expect(ab_user["button_size"]).to eq(button_size) + button_size_alt = Split::Alternative.new(button_size, "button_size") expect(button_size_alt.participant_count).to eq(1) end context "with allow_multiple_experiments = 'control'" do it "should let a user participate in many experiment with one non-'control' alternative" do Split.configure do |config| - config.allow_multiple_experiments = 'control' + config.allow_multiple_experiments = "control" end groups = 100.times.map do |n| - ab_test("test#{n}".to_sym, {'control' => (100 - n)}, {"test#{n}-alt" => n}) + ab_test("test#{n}".to_sym, { "control" => (100 - n) }, { "test#{n}-alt" => n }) end experiments = ab_user.active_experiments expect(experiments.size).to be > 1 - count_control = experiments.values.count { |g| g == 'control' } + count_control = experiments.values.count { |g| g == "control" } expect(count_control).to eq(experiments.size - 1) - count_alts = groups.count { |g| g != 'control' } + count_alts = groups.count { |g| g != "control" } expect(count_alts).to eq(1) end context "when user already has experiment" do - let(:mock_user){ Split::User.new(self, {'test_0' => 'test-alt'}) } + let(:mock_user) { Split::User.new(self, { "test_0" => "test-alt" }) } before do Split.configure do |config| - config.allow_multiple_experiments = 'control' + config.allow_multiple_experiments = "control" end - Split::ExperimentCatalog.find_or_initialize('test_0', 'control', 'test-alt').save - Split::ExperimentCatalog.find_or_initialize('test_1', 'control', 'test-alt').save + Split::ExperimentCatalog.find_or_initialize("test_0", "control", "test-alt").save + Split::ExperimentCatalog.find_or_initialize("test_1", "control", "test-alt").save end it "should restore previously selected alternative" do expect(ab_user.active_experiments.size).to eq 1 - expect(ab_test(:test_0, {'control' => 100}, {"test-alt" => 1})).to eq 'test-alt' - expect(ab_test(:test_0, {'control' => 1}, {"test-alt" => 100})).to eq 'test-alt' + expect(ab_test(:test_0, { "control" => 100 }, { "test-alt" => 1 })).to eq "test-alt" + expect(ab_test(:test_0, { "control" => 1 }, { "test-alt" => 100 })).to eq "test-alt" + end + + it "should select the correct alternatives after experiment resets" do + experiment = Split::ExperimentCatalog.find(:test_0) + experiment.reset + mock_user[experiment.key] = "test-alt" + + expect(ab_user.active_experiments.size).to eq 1 + expect(ab_test(:test_0, { "control" => 100 }, { "test-alt" => 1 })).to eq "test-alt" + expect(ab_test(:test_0, { "control" => 0 }, { "test-alt" => 100 })).to eq "test-alt" end it "should select the correct alternatives after experiment resets" do @@ -257,21 +268,19 @@ it "lets override existing choice" do pending "this requires user store reset on first call not depending on whelther it is current trial" - @params = { 'ab_test' => { 'test_1' => 'test-alt' } } + @params = { "ab_test" => { "test_1" => "test-alt" } } - expect(ab_test(:test_0, {'control' => 0}, {"test-alt" => 100})).to eq 'control' - expect(ab_test(:test_1, {'control' => 100}, {"test-alt" => 1})).to eq 'test-alt' + expect(ab_test(:test_0, { "control" => 0 }, { "test-alt" => 100 })).to eq "control" + expect(ab_test(:test_1, { "control" => 100 }, { "test-alt" => 1 })).to eq "test-alt" end - end - end it "should not over-write a finished key when an experiment is on a later version" do experiment.increment_version - ab_user = { experiment.key => 'blue', experiment.finished_key => true } + ab_user = { experiment.key => "blue", experiment.finished_key => true } finished_session = ab_user.dup - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") expect(ab_user).to eq(finished_session) end @@ -300,72 +309,72 @@ def ab_test_user_qualified? end end - describe 'metadata' do - context 'is defined' do + describe "metadata" do + context "is defined" do before do Split.configuration.experiments = { - :my_experiment => { - :alternatives => ["one", "two"], - :resettable => false, - :metadata => { 'one' => 'Meta1', 'two' => 'Meta2' } + my_experiment: { + alternatives: ["one", "two"], + resettable: false, + metadata: { "one" => "Meta1", "two" => "Meta2" } } } end - it 'should be passed to helper block' do - @params = { 'ab_test' => { 'my_experiment' => 'two' } } - expect(ab_test('my_experiment')).to eq 'two' - expect(ab_test('my_experiment') do |alternative, meta| + it "should be passed to helper block" do + @params = { "ab_test" => { "my_experiment" => "two" } } + expect(ab_test("my_experiment")).to eq "two" + expect(ab_test("my_experiment") do |alternative, meta| meta - end).to eq('Meta2') + end).to eq("Meta2") end - it 'should pass control metadata helper block if library disabled' do + it "should pass control metadata helper block if library disabled" do Split.configure do |config| config.enabled = false end - expect(ab_test('my_experiment')).to eq 'one' - expect(ab_test('my_experiment') do |_, meta| + expect(ab_test("my_experiment")).to eq "one" + expect(ab_test("my_experiment") do |_, meta| meta - end).to eq('Meta1') + end).to eq("Meta1") end end - context 'is not defined' do + context "is not defined" do before do Split.configuration.experiments = { - :my_experiment => { - :alternatives => ["one", "two"], - :resettable => false, - :metadata => nil + my_experiment: { + alternatives: ["one", "two"], + resettable: false, + metadata: nil } } end - it 'should be passed to helper block' do - expect(ab_test('my_experiment') do |alternative, meta| + it "should be passed to helper block" do + expect(ab_test("my_experiment") do |alternative, meta| meta end).to eq({}) end - it 'should pass control metadata helper block if library disabled' do + it "should pass control metadata helper block if library disabled" do Split.configure do |config| config.enabled = false end - expect(ab_test('my_experiment') do |_, meta| + expect(ab_test("my_experiment") do |_, meta| meta end).to eq({}) end end end - describe 'ab_finished' do - context 'for an experiment that the user participates in' do + describe "ab_finished" do + context "for an experiment that the user participates in" do before(:each) do - @experiment_name = 'link_color' - @alternatives = ['blue', 'red'] + @experiment_name = "link_color" + @alternatives = ["blue", "red"] @experiment = Split::ExperimentCatalog.find_or_create(@experiment_name, *@alternatives) @alternative_name = ab_test(@experiment_name, *@alternatives) @previous_completion_count = Split::Alternative.new(@alternative_name, @experiment_name).completed_count @@ -422,25 +431,25 @@ def ab_test_user_qualified? end it "should set experiment's finished key if reset is false" do - ab_finished(@experiment_name, {:reset => false}) + ab_finished(@experiment_name, { reset: false }) expect(ab_user[@experiment.key]).to eq(@alternative_name) expect(ab_user[@experiment.finished_key]).to eq(true) end - it 'should not increment the counter if reset is false and the experiment has been already finished' do - 2.times { ab_finished(@experiment_name, {:reset => false}) } + it "should not increment the counter if reset is false and the experiment has been already finished" do + 2.times { ab_finished(@experiment_name, { reset: false }) } new_completion_count = Split::Alternative.new(@alternative_name, @experiment_name).completed_count expect(new_completion_count).to eq(@previous_completion_count + 1) end - it 'should not increment the counter for an ended experiment' do - e = Split::ExperimentCatalog.find_or_create('button_size', 'small', 'big') - e.winner = 'small' - a = ab_test('button_size', 'small', 'big') - expect(a).to eq('small') - expect(lambda { - ab_finished('button_size') - }).not_to change { Split::Alternative.new(a, 'button_size').completed_count } + it "should not increment the counter for an ended experiment" do + e = Split::ExperimentCatalog.find_or_create("button_size", "small", "big") + e.winner = "small" + a = ab_test("button_size", "small", "big") + expect(a).to eq("small") + expect { + ab_finished("button_size") + }.not_to change { Split::Alternative.new(a, "button_size").completed_count } end it "should clear out the user's participation from their session" do @@ -451,7 +460,7 @@ def ab_test_user_qualified? it "should not clear out the users session if reset is false" do expect(ab_user[@experiment.key]).to eq(@alternative_name) - ab_finished(@experiment_name, {:reset => false}) + ab_finished(@experiment_name, { reset: false }) expect(ab_user[@experiment.key]).to eq(@alternative_name) expect(ab_user[@experiment.finished_key]).to eq(true) end @@ -486,46 +495,46 @@ def ab_test_user_qualified? end end - context 'for an experiment that the user is excluded from' do + context "for an experiment that the user is excluded from" do before do - alternative = ab_test('link_color', 'blue', 'red') - expect(Split::Alternative.new(alternative, 'link_color').participant_count).to eq(1) - alternative = ab_test('button_size', 'small', 'big') - expect(Split::Alternative.new(alternative, 'button_size').participant_count).to eq(0) + alternative = ab_test("link_color", "blue", "red") + expect(Split::Alternative.new(alternative, "link_color").participant_count).to eq(1) + alternative = ab_test("button_size", "small", "big") + expect(Split::Alternative.new(alternative, "button_size").participant_count).to eq(0) end - it 'should not increment the completed counter' do + it "should not increment the completed counter" do # So, user should be participating in the link_color experiment and # receive the control for button_size. As the user is not participating in # the button size experiment, finishing it should not increase the # completion count for that alternative. - expect(lambda { - ab_finished('button_size') - }).not_to change { Split::Alternative.new('small', 'button_size').completed_count } + expect { + ab_finished("button_size") + }.not_to change { Split::Alternative.new("small", "button_size").completed_count } end end - context 'for an experiment that the user does not participate in' do + context "for an experiment that the user does not participate in" do before do - Split::ExperimentCatalog.find_or_create(:not_started_experiment, 'control', 'alt') + Split::ExperimentCatalog.find_or_create(:not_started_experiment, "control", "alt") end - it 'should not raise an exception' do + it "should not raise an exception" do expect { ab_finished(:not_started_experiment) }.not_to raise_exception end - it 'should not change the user state when reset is false' do - expect { ab_finished(:not_started_experiment, reset: false) }.not_to change { ab_user.keys}.from([]) + it "should not change the user state when reset is false" do + expect { ab_finished(:not_started_experiment, reset: false) }.not_to change { ab_user.keys }.from([]) end - it 'should not change the user state when reset is true' do + it "should not change the user state when reset is true" do expect(self).not_to receive(:reset!) ab_finished(:not_started_experiment) end - it 'should not increment the completed counter' do + it "should not increment the completed counter" do ab_finished(:not_started_experiment) - expect(Split::Alternative.new('control', :not_started_experiment).completed_count).to eq(0) - expect(Split::Alternative.new('alt', :not_started_experiment).completed_count).to eq(0) + expect(Split::Alternative.new("control", :not_started_experiment).completed_count).to eq(0) + expect(Split::Alternative.new("alt", :not_started_experiment).completed_count).to eq(0) end end end @@ -533,9 +542,9 @@ def ab_test_user_qualified? context "finished with config" do it "passes reset option" do Split.configuration.experiments = { - :my_experiment => { - :alternatives => ["one", "two"], - :resettable => false, + my_experiment: { + alternatives: ["one", "two"], + resettable: false, } } alternative = ab_test(:my_experiment) @@ -551,11 +560,11 @@ def ab_test_user_qualified? before { Split.configuration.experiments = {} } before { expect(Split::Alternative).to receive(:new).at_least(1).times.and_call_original } - def should_finish_experiment(experiment_name, should_finish=true) + def should_finish_experiment(experiment_name, should_finish = true) alts = Split.configuration.experiments[experiment_name][:alternatives] experiment = Split::ExperimentCatalog.find_or_create(experiment_name, *alts) alt_name = ab_user[experiment.key] = alts.first - alt = double('alternative') + alt = double("alternative") expect(alt).to receive(:name).at_most(1).times.and_return(alt_name) expect(Split::Alternative).to receive(:new).at_most(1).times.with(alt_name, experiment_name.to_s).and_return(alt) if should_finish @@ -567,8 +576,8 @@ def should_finish_experiment(experiment_name, should_finish=true) it "completes the test" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ "control_opt", "other_opt" ], - :metric => :my_metric + alternatives: [ "control_opt", "other_opt" ], + metric: :my_metric } should_finish_experiment :my_experiment ab_finished :my_metric @@ -576,17 +585,17 @@ def should_finish_experiment(experiment_name, should_finish=true) it "completes all relevant tests" do Split.configuration.experiments = { - :exp_1 => { - :alternatives => [ "1-1", "1-2" ], - :metric => :my_metric + exp_1: { + alternatives: [ "1-1", "1-2" ], + metric: :my_metric }, - :exp_2 => { - :alternatives => [ "2-1", "2-2" ], - :metric => :another_metric + exp_2: { + alternatives: [ "2-1", "2-2" ], + metric: :another_metric }, - :exp_3 => { - :alternatives => [ "3-1", "3-2" ], - :metric => :my_metric + exp_3: { + alternatives: [ "3-1", "3-2" ], + metric: :my_metric }, } should_finish_experiment :exp_1 @@ -597,10 +606,10 @@ def should_finish_experiment(experiment_name, should_finish=true) it "passes reset option" do Split.configuration.experiments = { - :my_exp => { - :alternatives => ["one", "two"], - :metric => :my_metric, - :resettable => false, + my_exp: { + alternatives: ["one", "two"], + metric: :my_metric, + resettable: false, } } alternative_name = ab_test(:my_exp) @@ -613,199 +622,197 @@ def should_finish_experiment(experiment_name, should_finish=true) it "passes through options" do Split.configuration.experiments = { - :my_exp => { - :alternatives => ["one", "two"], - :metric => :my_metric, + my_exp: { + alternatives: ["one", "two"], + metric: :my_metric, } } alternative_name = ab_test(:my_exp) exp = Split::ExperimentCatalog.find :my_exp - ab_finished :my_metric, :reset => false + ab_finished :my_metric, reset: false expect(ab_user[exp.key]).to eq(alternative_name) expect(ab_user[exp.finished_key]).to be_truthy end end - describe 'conversions' do - it 'should return a conversion rate for an alternative' do - alternative_name = ab_test('link_color', 'blue', 'red') + describe "conversions" do + it "should return a conversion rate for an alternative" do + alternative_name = ab_test("link_color", "blue", "red") - previous_convertion_rate = Split::Alternative.new(alternative_name, 'link_color').conversion_rate + previous_convertion_rate = Split::Alternative.new(alternative_name, "link_color").conversion_rate expect(previous_convertion_rate).to eq(0.0) - ab_finished('link_color') + ab_finished("link_color") - new_convertion_rate = Split::Alternative.new(alternative_name, 'link_color').conversion_rate + new_convertion_rate = Split::Alternative.new(alternative_name, "link_color").conversion_rate expect(new_convertion_rate).to eq(1.0) end end - describe 'active experiments' do - it 'should show an active test' do - alternative = ab_test('def', '4', '5', '6') + describe "active experiments" do + it "should show an active test" do + alternative = ab_test("def", "4", "5", "6") expect(active_experiments.count).to eq 1 expect(active_experiments.first[0]).to eq "def" expect(active_experiments.first[1]).to eq alternative end - it 'should show a finished test' do - alternative = ab_test('def', '4', '5', '6') - ab_finished('def', {:reset => false}) + it "should show a finished test" do + alternative = ab_test("def", "4", "5", "6") + ab_finished("def", { reset: false }) expect(active_experiments.count).to eq 1 expect(active_experiments.first[0]).to eq "def" expect(active_experiments.first[1]).to eq alternative end - it 'should show an active test when an experiment is on a later version' do + it "should show an active test when an experiment is on a later version" do experiment.reset expect(experiment.version).to eq(1) - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") expect(active_experiments.count).to eq 1 expect(active_experiments.first[0]).to eq "link_color" end - it 'should show versioned tests properly' do + it "should show versioned tests properly" do 10.times { experiment.reset } - alternative = ab_test(experiment.name, 'blue', 'red') + alternative = ab_test(experiment.name, "blue", "red") ab_finished(experiment.name, reset: false) expect(experiment.version).to eq(10) expect(active_experiments.count).to eq 1 - expect(active_experiments).to eq({'link_color' => alternative }) + expect(active_experiments).to eq({ "link_color" => alternative }) end - it 'should show multiple tests' do + it "should show multiple tests" do Split.configure do |config| config.allow_multiple_experiments = true end - alternative = ab_test('def', '4', '5', '6') - another_alternative = ab_test('ghi', '7', '8', '9') + alternative = ab_test("def", "4", "5", "6") + another_alternative = ab_test("ghi", "7", "8", "9") expect(active_experiments.count).to eq 2 - expect(active_experiments['def']).to eq alternative - expect(active_experiments['ghi']).to eq another_alternative + expect(active_experiments["def"]).to eq alternative + expect(active_experiments["ghi"]).to eq another_alternative end - it 'should not show tests with winners' do + it "should not show tests with winners" do Split.configure do |config| config.allow_multiple_experiments = true end - e = Split::ExperimentCatalog.find_or_create('def', '4', '5', '6') - e.winner = '4' - ab_test('def', '4', '5', '6') - another_alternative = ab_test('ghi', '7', '8', '9') + e = Split::ExperimentCatalog.find_or_create("def", "4", "5", "6") + e.winner = "4" + ab_test("def", "4", "5", "6") + another_alternative = ab_test("ghi", "7", "8", "9") expect(active_experiments.count).to eq 1 expect(active_experiments.first[0]).to eq "ghi" expect(active_experiments.first[1]).to eq another_alternative end end - describe 'when user is a robot' do + describe "when user is a robot" do before(:each) do - @request = OpenStruct.new(:user_agent => 'Googlebot/2.1 (+http://www.google.com/bot.html)') + @request = OpenStruct.new(user_agent: "Googlebot/2.1 (+http://www.google.com/bot.html)") end - describe 'ab_test' do - it 'should return the control' do - alternative = ab_test('link_color', 'blue', 'red') + describe "ab_test" do + it "should return the control" do + alternative = ab_test("link_color", "blue", "red") expect(alternative).to eq experiment.control.name end - it 'should not create a experiment' do - ab_test('link_color', 'blue', 'red') - expect(Split::Experiment.new('link_color')).to be_a_new_record + it "should not create a experiment" do + ab_test("link_color", "blue", "red") + expect(Split::Experiment.new("link_color")).to be_a_new_record end it "should not increment the participation count" do + previous_red_count = Split::Alternative.new("red", "link_color").participant_count + previous_blue_count = Split::Alternative.new("blue", "link_color").participant_count - previous_red_count = Split::Alternative.new('red', 'link_color').participant_count - previous_blue_count = Split::Alternative.new('blue', 'link_color').participant_count - - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") - new_red_count = Split::Alternative.new('red', 'link_color').participant_count - new_blue_count = Split::Alternative.new('blue', 'link_color').participant_count + new_red_count = Split::Alternative.new("red", "link_color").participant_count + new_blue_count = Split::Alternative.new("blue", "link_color").participant_count expect((new_red_count + new_blue_count)).to eq(previous_red_count + previous_blue_count) end end - describe 'finished' do + describe "finished" do it "should not increment the completed count" do - alternative_name = ab_test('link_color', 'blue', 'red') + alternative_name = ab_test("link_color", "blue", "red") - previous_completion_count = Split::Alternative.new(alternative_name, 'link_color').completed_count + previous_completion_count = Split::Alternative.new(alternative_name, "link_color").completed_count - ab_finished('link_color') + ab_finished("link_color") - new_completion_count = Split::Alternative.new(alternative_name, 'link_color').completed_count + new_completion_count = Split::Alternative.new(alternative_name, "link_color").completed_count expect(new_completion_count).to eq(previous_completion_count) end end end - describe 'when providing custom ignore logic' do + describe "when providing custom ignore logic" do context "using a proc to configure custom logic" do - before(:each) do Split.configure do |c| - c.ignore_filter = proc{|request| true } # ignore everything + c.ignore_filter = proc { |request| true } # ignore everything end end it "ignores the ab_test" do - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") - red_count = Split::Alternative.new('red', 'link_color').participant_count - blue_count = Split::Alternative.new('blue', 'link_color').participant_count + red_count = Split::Alternative.new("red", "link_color").participant_count + blue_count = Split::Alternative.new("blue", "link_color").participant_count expect((red_count + blue_count)).to be(0) end end end shared_examples_for "a disabled test" do - describe 'ab_test' do - it 'should return the control' do - alternative = ab_test('link_color', 'blue', 'red') + describe "ab_test" do + it "should return the control" do + alternative = ab_test("link_color", "blue", "red") expect(alternative).to eq experiment.control.name end it "should not increment the participation count" do - previous_red_count = Split::Alternative.new('red', 'link_color').participant_count - previous_blue_count = Split::Alternative.new('blue', 'link_color').participant_count + previous_red_count = Split::Alternative.new("red", "link_color").participant_count + previous_blue_count = Split::Alternative.new("blue", "link_color").participant_count - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") - new_red_count = Split::Alternative.new('red', 'link_color').participant_count - new_blue_count = Split::Alternative.new('blue', 'link_color').participant_count + new_red_count = Split::Alternative.new("red", "link_color").participant_count + new_blue_count = Split::Alternative.new("blue", "link_color").participant_count expect((new_red_count + new_blue_count)).to eq(previous_red_count + previous_blue_count) end end - describe 'finished' do + describe "finished" do it "should not increment the completed count" do - alternative_name = ab_test('link_color', 'blue', 'red') + alternative_name = ab_test("link_color", "blue", "red") - previous_completion_count = Split::Alternative.new(alternative_name, 'link_color').completed_count + previous_completion_count = Split::Alternative.new(alternative_name, "link_color").completed_count - ab_finished('link_color') + ab_finished("link_color") - new_completion_count = Split::Alternative.new(alternative_name, 'link_color').completed_count + new_completion_count = Split::Alternative.new(alternative_name, "link_color").completed_count expect(new_completion_count).to eq(previous_completion_count) end end end - describe 'when ip address is ignored' do + describe "when ip address is ignored" do context "individually" do before(:each) do - @request = OpenStruct.new(:ip => '81.19.48.130') + @request = OpenStruct.new(ip: "81.19.48.130") Split.configure do |c| - c.ignore_ip_addresses << '81.19.48.130' + c.ignore_ip_addresses << "81.19.48.130" end end @@ -814,7 +821,7 @@ def should_finish_experiment(experiment_name, should_finish=true) context "for a range" do before(:each) do - @request = OpenStruct.new(:ip => '81.19.48.129') + @request = OpenStruct.new(ip: "81.19.48.129") Split.configure do |c| c.ignore_ip_addresses << /81\.19\.48\.[0-9]+/ end @@ -825,9 +832,9 @@ def should_finish_experiment(experiment_name, should_finish=true) context "using both a range and a specific value" do before(:each) do - @request = OpenStruct.new(:ip => '81.19.48.128') + @request = OpenStruct.new(ip: "81.19.48.128") Split.configure do |c| - c.ignore_ip_addresses << '81.19.48.130' + c.ignore_ip_addresses << "81.19.48.130" c.ignore_ip_addresses << /81\.19\.48\.[0-9]+/ end end @@ -837,119 +844,119 @@ def should_finish_experiment(experiment_name, should_finish=true) context "when ignored other address" do before do - @request = OpenStruct.new(:ip => '1.1.1.1') + @request = OpenStruct.new(ip: "1.1.1.1") Split.configure do |c| - c.ignore_ip_addresses << '81.19.48.130' + c.ignore_ip_addresses << "81.19.48.130" end end it "works as usual" do - alternative_name = ab_test('link_color', 'red', 'blue') - expect{ - ab_finished('link_color') - }.to change(Split::Alternative.new(alternative_name, 'link_color'), :completed_count).by(1) + alternative_name = ab_test("link_color", "red", "blue") + expect { + ab_finished("link_color") + }.to change(Split::Alternative.new(alternative_name, "link_color"), :completed_count).by(1) end end end - describe 'when user is previewing' do + describe "when user is previewing" do before(:each) do - @request = OpenStruct.new(headers: { 'x-purpose' => 'preview' }) + @request = OpenStruct.new(headers: { "x-purpose" => "preview" }) end it_behaves_like "a disabled test" end - describe 'versioned experiments' do + describe "versioned experiments" do it "should use version zero if no version is present" do - alternative_name = ab_test('link_color', 'blue', 'red') + alternative_name = ab_test("link_color", "blue", "red") expect(experiment.version).to eq(0) - expect(ab_user['link_color']).to eq(alternative_name) + expect(ab_user["link_color"]).to eq(alternative_name) end it "should save the version of the experiment to the session" do experiment.reset expect(experiment.version).to eq(1) - alternative_name = ab_test('link_color', 'blue', 'red') - expect(ab_user['link_color:1']).to eq(alternative_name) + alternative_name = ab_test("link_color", "blue", "red") + expect(ab_user["link_color:1"]).to eq(alternative_name) end it "should load the experiment even if the version is not 0" do experiment.reset expect(experiment.version).to eq(1) - alternative_name = ab_test('link_color', 'blue', 'red') - expect(ab_user['link_color:1']).to eq(alternative_name) - return_alternative_name = ab_test('link_color', 'blue', 'red') + alternative_name = ab_test("link_color", "blue", "red") + expect(ab_user["link_color:1"]).to eq(alternative_name) + return_alternative_name = ab_test("link_color", "blue", "red") expect(return_alternative_name).to eq(alternative_name) end it "should reset the session of a user on an older version of the experiment" do - alternative_name = ab_test('link_color', 'blue', 'red') - expect(ab_user['link_color']).to eq(alternative_name) - alternative = Split::Alternative.new(alternative_name, 'link_color') + alternative_name = ab_test("link_color", "blue", "red") + expect(ab_user["link_color"]).to eq(alternative_name) + alternative = Split::Alternative.new(alternative_name, "link_color") expect(alternative.participant_count).to eq(1) experiment.reset expect(experiment.version).to eq(1) - alternative = Split::Alternative.new(alternative_name, 'link_color') + alternative = Split::Alternative.new(alternative_name, "link_color") expect(alternative.participant_count).to eq(0) - new_alternative_name = ab_test('link_color', 'blue', 'red') - expect(ab_user['link_color:1']).to eq(new_alternative_name) - new_alternative = Split::Alternative.new(new_alternative_name, 'link_color') + new_alternative_name = ab_test("link_color", "blue", "red") + expect(ab_user["link_color:1"]).to eq(new_alternative_name) + new_alternative = Split::Alternative.new(new_alternative_name, "link_color") expect(new_alternative.participant_count).to eq(1) end it "should cleanup old versions of experiments from the session" do - alternative_name = ab_test('link_color', 'blue', 'red') - expect(ab_user['link_color']).to eq(alternative_name) - alternative = Split::Alternative.new(alternative_name, 'link_color') + alternative_name = ab_test("link_color", "blue", "red") + expect(ab_user["link_color"]).to eq(alternative_name) + alternative = Split::Alternative.new(alternative_name, "link_color") expect(alternative.participant_count).to eq(1) experiment.reset expect(experiment.version).to eq(1) - alternative = Split::Alternative.new(alternative_name, 'link_color') + alternative = Split::Alternative.new(alternative_name, "link_color") expect(alternative.participant_count).to eq(0) - new_alternative_name = ab_test('link_color', 'blue', 'red') - expect(ab_user['link_color:1']).to eq(new_alternative_name) + new_alternative_name = ab_test("link_color", "blue", "red") + expect(ab_user["link_color:1"]).to eq(new_alternative_name) end it "should only count completion of users on the current version" do - alternative_name = ab_test('link_color', 'blue', 'red') - expect(ab_user['link_color']).to eq(alternative_name) - alternative = Split::Alternative.new(alternative_name, 'link_color') + alternative_name = ab_test("link_color", "blue", "red") + expect(ab_user["link_color"]).to eq(alternative_name) + Split::Alternative.new(alternative_name, "link_color") experiment.reset expect(experiment.version).to eq(1) - ab_finished('link_color') - alternative = Split::Alternative.new(alternative_name, 'link_color') + ab_finished("link_color") + alternative = Split::Alternative.new(alternative_name, "link_color") expect(alternative.completed_count).to eq(0) end end - context 'when redis is not available' do + context "when redis is not available" do before(:each) do expect(Split).to receive(:redis).at_most(5).times.and_raise(Errno::ECONNREFUSED.new) end - context 'and db_failover config option is turned off' do + context "and db_failover config option is turned off" do before(:each) do Split.configure do |config| config.db_failover = false end end - describe 'ab_test' do - it 'should raise an exception' do - expect(lambda { ab_test('link_color', 'blue', 'red') }).to raise_error(Errno::ECONNREFUSED) + describe "ab_test" do + it "should raise an exception" do + expect { ab_test("link_color", "blue", "red") }.to raise_error(Errno::ECONNREFUSED) end end - describe 'finished' do - it 'should raise an exception' do - expect(lambda { ab_finished('link_color') }).to raise_error(Errno::ECONNREFUSED) + describe "finished" do + it "should raise an exception" do + expect { ab_finished("link_color") }.to raise_error(Errno::ECONNREFUSED) end end @@ -961,29 +968,29 @@ def should_finish_experiment(experiment_name, should_finish=true) end it "should not attempt to connect to redis" do - expect(lambda { ab_test('link_color', 'blue', 'red') }).not_to raise_error + expect { ab_test("link_color", "blue", "red") }.not_to raise_error end it "should return control variable" do - expect(ab_test('link_color', 'blue', 'red')).to eq('blue') - expect(lambda { ab_finished('link_color') }).not_to raise_error + expect(ab_test("link_color", "blue", "red")).to eq("blue") + expect { ab_finished("link_color") }.not_to raise_error end end end - context 'and db_failover config option is turned on' do + context "and db_failover config option is turned on" do before(:each) do Split.configure do |config| config.db_failover = true end end - describe 'ab_test' do - it 'should not raise an exception' do - expect(lambda { ab_test('link_color', 'blue', 'red') }).not_to raise_error + describe "ab_test" do + it "should not raise an exception" do + expect { ab_test("link_color", "blue", "red") }.not_to raise_error end - it 'should call db_failover_on_db_error proc with error as parameter' do + it "should call db_failover_on_db_error proc with error as parameter" do Split.configure do |config| config.db_failover_on_db_error = proc do |error| expect(error).to be_a(Errno::ECONNREFUSED) @@ -991,43 +998,43 @@ def should_finish_experiment(experiment_name, should_finish=true) end expect(Split.configuration.db_failover_on_db_error).to receive(:call).and_call_original - ab_test('link_color', 'blue', 'red') + ab_test("link_color", "blue", "red") end - it 'should always use first alternative' do - expect(ab_test('link_color', 'blue', 'red')).to eq('blue') - expect(ab_test('link_color', {'blue' => 0.01}, 'red' => 0.2)).to eq('blue') - expect(ab_test('link_color', {'blue' => 0.8}, {'red' => 20})).to eq('blue') - expect(ab_test('link_color', 'blue', 'red') do |alternative| + it "should always use first alternative" do + expect(ab_test("link_color", "blue", "red")).to eq("blue") + expect(ab_test("link_color", { "blue" => 0.01 }, "red" => 0.2)).to eq("blue") + expect(ab_test("link_color", { "blue" => 0.8 }, { "red" => 20 })).to eq("blue") + expect(ab_test("link_color", "blue", "red") do |alternative| "shared/#{alternative}" - end).to eq('shared/blue') + end).to eq("shared/blue") end - context 'and db_failover_allow_parameter_override config option is turned on' do + context "and db_failover_allow_parameter_override config option is turned on" do before(:each) do Split.configure do |config| config.db_failover_allow_parameter_override = true end end - context 'and given an override parameter' do - it 'should use given override instead of the first alternative' do - @params = { 'ab_test' => { 'link_color' => 'red' } } - expect(ab_test('link_color', 'blue', 'red')).to eq('red') - expect(ab_test('link_color', 'blue', 'red', 'green')).to eq('red') - expect(ab_test('link_color', {'blue' => 0.01}, 'red' => 0.2)).to eq('red') - expect(ab_test('link_color', {'blue' => 0.8}, {'red' => 20})).to eq('red') - expect(ab_test('link_color', 'blue', 'red') do |alternative| + context "and given an override parameter" do + it "should use given override instead of the first alternative" do + @params = { "ab_test" => { "link_color" => "red" } } + expect(ab_test("link_color", "blue", "red")).to eq("red") + expect(ab_test("link_color", "blue", "red", "green")).to eq("red") + expect(ab_test("link_color", { "blue" => 0.01 }, "red" => 0.2)).to eq("red") + expect(ab_test("link_color", { "blue" => 0.8 }, { "red" => 20 })).to eq("red") + expect(ab_test("link_color", "blue", "red") do |alternative| "shared/#{alternative}" - end).to eq('shared/red') + end).to eq("shared/red") end end end - context 'and preloaded config given' do + context "and preloaded config given" do before do Split.configuration.experiments[:link_color] = { - :alternatives => [ "blue", "red" ], + alternatives: [ "blue", "red" ], } end @@ -1037,12 +1044,12 @@ def should_finish_experiment(experiment_name, should_finish=true) end end - describe 'finished' do - it 'should not raise an exception' do - expect(lambda { ab_finished('link_color') }).not_to raise_error + describe "finished" do + it "should not raise an exception" do + expect { ab_finished("link_color") }.not_to raise_error end - it 'should call db_failover_on_db_error proc with error as parameter' do + it "should call db_failover_on_db_error proc with error as parameter" do Split.configure do |config| config.db_failover_on_db_error = proc do |error| expect(error).to be_a(Errno::ECONNREFUSED) @@ -1050,19 +1057,19 @@ def should_finish_experiment(experiment_name, should_finish=true) end expect(Split.configuration.db_failover_on_db_error).to receive(:call).and_call_original - ab_finished('link_color') + ab_finished("link_color") end end end end context "with preloaded config" do - before { Split.configuration.experiments = {}} + before { Split.configuration.experiments = {} } it "pulls options from config file" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ "control_opt", "other_opt" ], - :goals => ["goal1", "goal2"] + alternatives: [ "control_opt", "other_opt" ], + goals: ["goal1", "goal2"] } ab_test :my_experiment expect(Split::Experiment.new(:my_experiment).alternatives.map(&:name)).to eq([ "control_opt", "other_opt" ]) @@ -1071,8 +1078,8 @@ def should_finish_experiment(experiment_name, should_finish=true) it "can be called multiple times" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ "control_opt", "other_opt" ], - :goals => ["goal1", "goal2"] + alternatives: [ "control_opt", "other_opt" ], + goals: ["goal1", "goal2"] } 5.times { ab_test :my_experiment } experiment = Split::Experiment.new(:my_experiment) @@ -1083,8 +1090,8 @@ def should_finish_experiment(experiment_name, should_finish=true) it "accepts multiple goals" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ "control_opt", "other_opt" ], - :goals => [ "goal1", "goal2", "goal3" ] + alternatives: [ "control_opt", "other_opt" ], + goals: [ "goal1", "goal2", "goal3" ] } ab_test :my_experiment experiment = Split::Experiment.new(:my_experiment) @@ -1093,7 +1100,7 @@ def should_finish_experiment(experiment_name, should_finish=true) it "allow specifying goals to be optional" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ "control_opt", "other_opt" ] + alternatives: [ "control_opt", "other_opt" ] } experiment = Split::Experiment.new(:my_experiment) expect(experiment.goals).to eq([]) @@ -1101,7 +1108,7 @@ def should_finish_experiment(experiment_name, should_finish=true) it "accepts multiple alternatives" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ "control_opt", "second_opt", "third_opt" ], + alternatives: [ "control_opt", "second_opt", "third_opt" ], } ab_test :my_experiment experiment = Split::Experiment.new(:my_experiment) @@ -1110,68 +1117,68 @@ def should_finish_experiment(experiment_name, should_finish=true) it "accepts probability on alternatives" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ - { :name => "control_opt", :percent => 67 }, - { :name => "second_opt", :percent => 10 }, - { :name => "third_opt", :percent => 23 }, + alternatives: [ + { name: "control_opt", percent: 67 }, + { name: "second_opt", percent: 10 }, + { name: "third_opt", percent: 23 }, ], } ab_test :my_experiment experiment = Split::Experiment.new(:my_experiment) - expect(experiment.alternatives.collect{|a| [a.name, a.weight]}).to eq([['control_opt', 0.67], ['second_opt', 0.1], ['third_opt', 0.23]]) + expect(experiment.alternatives.collect { |a| [a.name, a.weight] }).to eq([["control_opt", 0.67], ["second_opt", 0.1], ["third_opt", 0.23]]) end it "accepts probability on some alternatives" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ - { :name => "control_opt", :percent => 34 }, + alternatives: [ + { name: "control_opt", percent: 34 }, "second_opt", - { :name => "third_opt", :percent => 23 }, + { name: "third_opt", percent: 23 }, "fourth_opt", ], } ab_test :my_experiment experiment = Split::Experiment.new(:my_experiment) - names_and_weights = experiment.alternatives.collect{|a| [a.name, a.weight]} - expect(names_and_weights).to eq([['control_opt', 0.34], ['second_opt', 0.215], ['third_opt', 0.23], ['fourth_opt', 0.215]]) - expect(names_and_weights.inject(0){|sum, nw| sum + nw[1]}).to eq(1.0) + names_and_weights = experiment.alternatives.collect { |a| [a.name, a.weight] } + expect(names_and_weights).to eq([["control_opt", 0.34], ["second_opt", 0.215], ["third_opt", 0.23], ["fourth_opt", 0.215]]) + expect(names_and_weights.inject(0) { |sum, nw| sum + nw[1] }).to eq(1.0) end it "allows name param without probability" do Split.configuration.experiments[:my_experiment] = { - :alternatives => [ - { :name => "control_opt" }, + alternatives: [ + { name: "control_opt" }, "second_opt", - { :name => "third_opt", :percent => 64 }, + { name: "third_opt", percent: 64 }, ], } ab_test :my_experiment experiment = Split::Experiment.new(:my_experiment) - names_and_weights = experiment.alternatives.collect{|a| [a.name, a.weight]} - expect(names_and_weights).to eq([['control_opt', 0.18], ['second_opt', 0.18], ['third_opt', 0.64]]) - expect(names_and_weights.inject(0){|sum, nw| sum + nw[1]}).to eq(1.0) + names_and_weights = experiment.alternatives.collect { |a| [a.name, a.weight] } + expect(names_and_weights).to eq([["control_opt", 0.18], ["second_opt", 0.18], ["third_opt", 0.64]]) + expect(names_and_weights.inject(0) { |sum, nw| sum + nw[1] }).to eq(1.0) end it "fails gracefully if config is missing experiment" do - Split.configuration.experiments = { :other_experiment => { :foo => "Bar" } } - expect(lambda { ab_test :my_experiment }).to raise_error(Split::ExperimentNotFound) + Split.configuration.experiments = { other_experiment: { foo: "Bar" } } + expect { ab_test :my_experiment }.to raise_error(Split::ExperimentNotFound) end it "fails gracefully if config is missing" do - expect(lambda { Split.configuration.experiments = nil }).to raise_error(Split::InvalidExperimentsFormatError) + expect { Split.configuration.experiments = nil }.to raise_error(Split::InvalidExperimentsFormatError) end it "fails gracefully if config is missing alternatives" do - Split.configuration.experiments[:my_experiment] = { :foo => "Bar" } - expect(lambda { ab_test :my_experiment }).to raise_error(NoMethodError) + Split.configuration.experiments[:my_experiment] = { foo: "Bar" } + expect { ab_test :my_experiment }.to raise_error(NoMethodError) end end - it 'should handle multiple experiments correctly' do - experiment2 = Split::ExperimentCatalog.find_or_create('link_color2', 'blue', 'red') - ab_test('link_color', 'blue', 'red') - ab_test('link_color2', 'blue', 'red') - ab_finished('link_color2') + it "should handle multiple experiments correctly" do + experiment2 = Split::ExperimentCatalog.find_or_create("link_color2", "blue", "red") + ab_test("link_color", "blue", "red") + ab_test("link_color2", "blue", "red") + ab_finished("link_color2") experiment2.alternatives.each do |alt| expect(alt.unfinished_count).to eq(0) @@ -1180,8 +1187,8 @@ def should_finish_experiment(experiment_name, should_finish=true) context "with goals" do before do - @experiment = {'link_color' => ["purchase", "refund"]} - @alternatives = ['blue', 'red'] + @experiment = { "link_color" => ["purchase", "refund"] } + @alternatives = ["blue", "red"] @experiment_name, @goals = normalize_metric(@experiment) @goal1 = @goals[0] @goal2 = @goals[1] @@ -1195,8 +1202,8 @@ def should_finish_experiment(experiment_name, should_finish=true) describe "ab_test" do it "should allow experiment goals interface as a single hash" do ab_test(@experiment, *@alternatives) - experiment = Split::ExperimentCatalog.find('link_color') - expect(experiment.goals).to eq(['purchase', "refund"]) + experiment = Split::ExperimentCatalog.find("link_color") + expect(experiment.goals).to eq(["purchase", "refund"]) end end @@ -1206,9 +1213,9 @@ def should_finish_experiment(experiment_name, should_finish=true) end it "should increment the counter for the specified-goal completed alternative" do - expect{ ab_finished({"link_color" => ["purchase"]}) } - .to change{ Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal2) }.by(0) - .and change{ Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal1) }.by(1) + expect { ab_finished({ "link_color" => ["purchase"] }) } + .to change { Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal2) }.by(0) + .and change { Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal1) }.by(1) end end end diff --git a/spec/metric_spec.rb b/spec/metric_spec.rb index 21bbc758..c1bf63e4 100644 --- a/spec/metric_spec.rb +++ b/spec/metric_spec.rb @@ -1,31 +1,31 @@ # frozen_string_literal: true -require 'spec_helper' -require 'split/metric' + +require "spec_helper" +require "split/metric" describe Split::Metric do - describe 'possible experiments' do + describe "possible experiments" do it "should load the experiment if there is one, but no metric" do - experiment = Split::ExperimentCatalog.find_or_create('color', 'red', 'blue') - expect(Split::Metric.possible_experiments('color')).to eq([experiment]) + experiment = Split::ExperimentCatalog.find_or_create("color", "red", "blue") + expect(Split::Metric.possible_experiments("color")).to eq([experiment]) end it "should load the experiments in a metric" do - experiment1 = Split::ExperimentCatalog.find_or_create('color', 'red', 'blue') - experiment2 = Split::ExperimentCatalog.find_or_create('size', 'big', 'small') + experiment1 = Split::ExperimentCatalog.find_or_create("color", "red", "blue") + experiment2 = Split::ExperimentCatalog.find_or_create("size", "big", "small") - metric = Split::Metric.new(:name => 'purchase', :experiments => [experiment1, experiment2]) + metric = Split::Metric.new(name: "purchase", experiments: [experiment1, experiment2]) metric.save - expect(Split::Metric.possible_experiments('purchase')).to include(experiment1, experiment2) + expect(Split::Metric.possible_experiments("purchase")).to include(experiment1, experiment2) end it "should load both the metric experiments and an experiment with the same name" do - experiment1 = Split::ExperimentCatalog.find_or_create('purchase', 'red', 'blue') - experiment2 = Split::ExperimentCatalog.find_or_create('size', 'big', 'small') + experiment1 = Split::ExperimentCatalog.find_or_create("purchase", "red", "blue") + experiment2 = Split::ExperimentCatalog.find_or_create("size", "big", "small") - metric = Split::Metric.new(:name => 'purchase', :experiments => [experiment2]) + metric = Split::Metric.new(name: "purchase", experiments: [experiment2]) metric.save - expect(Split::Metric.possible_experiments('purchase')).to include(experiment1, experiment2) + expect(Split::Metric.possible_experiments("purchase")).to include(experiment1, experiment2) end end - end diff --git a/spec/persistence/cookie_adapter_spec.rb b/spec/persistence/cookie_adapter_spec.rb index e9758a47..e53dd88f 100644 --- a/spec/persistence/cookie_adapter_spec.rb +++ b/spec/persistence/cookie_adapter_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true + require "spec_helper" -require 'rack/test' +require "rack/test" describe Split::Persistence::CookieAdapter do subject { described_class.new(context) } @@ -14,8 +15,8 @@ it "handles invalid JSON" do context.request.cookies[:split] = { - :value => '{"foo":2,', - :expires => Time.now + value: '{"foo":2,', + expires: Time.now } expect(subject["my_key"]).to be_nil subject["my_key"] = "my_value" @@ -56,7 +57,7 @@ end it "ensure other added cookies are not overriden" do - context.response.set_cookie 'dummy', 'wow' + context.response.set_cookie "dummy", "wow" subject["foo"] = "FOO" expect(context.response.headers["Set-Cookie"]).to include("dummy=wow") expect(context.response.headers["Set-Cookie"]).to include("split=") @@ -77,7 +78,7 @@ controller.send(:"request=", ActionDispatch::Request.new({})) end - response = ActionDispatch::Response.new(200, {}, '').tap do |res| + response = ActionDispatch::Response.new(200, {}, "").tap do |res| res.request = controller.request end @@ -100,7 +101,7 @@ expect(subject["foo"]).to eq("FOO") expect(subject["bar"]).to eq("BAR") cookie_jar = context.request.env["action_dispatch.cookies"] - expect(cookie_jar['split']).to eq("{\"foo\":\"FOO\",\"bar\":\"BAR\"}") + expect(cookie_jar["split"]).to eq('{"foo":"FOO","bar":"BAR"}') end end end diff --git a/spec/persistence/dual_adapter_spec.rb b/spec/persistence/dual_adapter_spec.rb index baccc221..a3621228 100644 --- a/spec/persistence/dual_adapter_spec.rb +++ b/spec/persistence/dual_adapter_spec.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -require 'spec_helper' +require "spec_helper" describe Split::Persistence::DualAdapter do - let(:context) { 'some context' } + let(:context) { "some context" } let(:logged_in_adapter_instance) { double } let(:logged_in_adapter) do @@ -14,8 +14,8 @@ Class.new.tap { |c| allow(c).to receive(:new) { logged_out_adapter_instance } } end - context 'when fallback_to_logged_out_adapter is false' do - context 'when logged in' do + context "when fallback_to_logged_out_adapter is false" do + context "when logged in" do subject do described_class.with_config( logged_in: lambda { |context| true }, @@ -25,32 +25,32 @@ ).new(context) end - it '#[]=' do - expect(logged_in_adapter_instance).to receive(:[]=).with('my_key', 'my_value') + it "#[]=" do + expect(logged_in_adapter_instance).to receive(:[]=).with("my_key", "my_value") expect_any_instance_of(logged_out_adapter).not_to receive(:[]=) - subject['my_key'] = 'my_value' + subject["my_key"] = "my_value" end - it '#[]' do - expect(logged_in_adapter_instance).to receive(:[]).with('my_key') { 'my_value' } + it "#[]" do + expect(logged_in_adapter_instance).to receive(:[]).with("my_key") { "my_value" } expect_any_instance_of(logged_out_adapter).not_to receive(:[]) - expect(subject['my_key']).to eq('my_value') + expect(subject["my_key"]).to eq("my_value") end - it '#delete' do - expect(logged_in_adapter_instance).to receive(:delete).with('my_key') { 'my_value' } + it "#delete" do + expect(logged_in_adapter_instance).to receive(:delete).with("my_key") { "my_value" } expect_any_instance_of(logged_out_adapter).not_to receive(:delete) - expect(subject.delete('my_key')).to eq('my_value') + expect(subject.delete("my_key")).to eq("my_value") end - it '#keys' do - expect(logged_in_adapter_instance).to receive(:keys) { ['my_value'] } + it "#keys" do + expect(logged_in_adapter_instance).to receive(:keys) { ["my_value"] } expect_any_instance_of(logged_out_adapter).not_to receive(:keys) - expect(subject.keys).to eq(['my_value']) + expect(subject.keys).to eq(["my_value"]) end end - context 'when logged out' do + context "when logged out" do subject do described_class.with_config( logged_in: lambda { |context| false }, @@ -60,34 +60,34 @@ ).new(context) end - it '#[]=' do + it "#[]=" do expect_any_instance_of(logged_in_adapter).not_to receive(:[]=) - expect(logged_out_adapter_instance).to receive(:[]=).with('my_key', 'my_value') - subject['my_key'] = 'my_value' + expect(logged_out_adapter_instance).to receive(:[]=).with("my_key", "my_value") + subject["my_key"] = "my_value" end - it '#[]' do + it "#[]" do expect_any_instance_of(logged_in_adapter).not_to receive(:[]) - expect(logged_out_adapter_instance).to receive(:[]).with('my_key') { 'my_value' } - expect(subject['my_key']).to eq('my_value') + expect(logged_out_adapter_instance).to receive(:[]).with("my_key") { "my_value" } + expect(subject["my_key"]).to eq("my_value") end - it '#delete' do + it "#delete" do expect_any_instance_of(logged_in_adapter).not_to receive(:delete) - expect(logged_out_adapter_instance).to receive(:delete).with('my_key') { 'my_value' } - expect(subject.delete('my_key')).to eq('my_value') + expect(logged_out_adapter_instance).to receive(:delete).with("my_key") { "my_value" } + expect(subject.delete("my_key")).to eq("my_value") end - it '#keys' do + it "#keys" do expect_any_instance_of(logged_in_adapter).not_to receive(:keys) - expect(logged_out_adapter_instance).to receive(:keys) { ['my_value', 'my_value2'] } - expect(subject.keys).to eq(['my_value', 'my_value2']) + expect(logged_out_adapter_instance).to receive(:keys) { ["my_value", "my_value2"] } + expect(subject.keys).to eq(["my_value", "my_value2"]) end end end - context 'when fallback_to_logged_out_adapter is true' do - context 'when logged in' do + context "when fallback_to_logged_out_adapter is true" do + context "when logged in" do subject do described_class.with_config( logged_in: lambda { |context| true }, @@ -97,33 +97,33 @@ ).new(context) end - it '#[]=' do - expect(logged_in_adapter_instance).to receive(:[]=).with('my_key', 'my_value') - expect(logged_out_adapter_instance).to receive(:[]=).with('my_key', 'my_value') - expect(logged_out_adapter_instance).to receive(:[]).with('my_key') { nil } - subject['my_key'] = 'my_value' + it "#[]=" do + expect(logged_in_adapter_instance).to receive(:[]=).with("my_key", "my_value") + expect(logged_out_adapter_instance).to receive(:[]=).with("my_key", "my_value") + expect(logged_out_adapter_instance).to receive(:[]).with("my_key") { nil } + subject["my_key"] = "my_value" end - it '#[]' do - expect(logged_in_adapter_instance).to receive(:[]).with('my_key') { 'my_value' } + it "#[]" do + expect(logged_in_adapter_instance).to receive(:[]).with("my_key") { "my_value" } expect_any_instance_of(logged_out_adapter).not_to receive(:[]) - expect(subject['my_key']).to eq('my_value') + expect(subject["my_key"]).to eq("my_value") end - it '#delete' do - expect(logged_in_adapter_instance).to receive(:delete).with('my_key') { 'my_value' } - expect(logged_out_adapter_instance).to receive(:delete).with('my_key') { 'my_value' } - expect(subject.delete('my_key')).to eq('my_value') + it "#delete" do + expect(logged_in_adapter_instance).to receive(:delete).with("my_key") { "my_value" } + expect(logged_out_adapter_instance).to receive(:delete).with("my_key") { "my_value" } + expect(subject.delete("my_key")).to eq("my_value") end - it '#keys' do - expect(logged_in_adapter_instance).to receive(:keys) { ['my_value'] } - expect(logged_out_adapter_instance).to receive(:keys) { ['my_value', 'my_value2'] } - expect(subject.keys).to eq(['my_value', 'my_value2']) + it "#keys" do + expect(logged_in_adapter_instance).to receive(:keys) { ["my_value"] } + expect(logged_out_adapter_instance).to receive(:keys) { ["my_value", "my_value2"] } + expect(subject.keys).to eq(["my_value", "my_value2"]) end end - context 'when logged out' do + context "when logged out" do subject do described_class.with_config( logged_in: lambda { |context| false }, @@ -133,39 +133,39 @@ ).new(context) end - it '#[]=' do + it "#[]=" do expect_any_instance_of(logged_in_adapter).not_to receive(:[]=) - expect(logged_out_adapter_instance).to receive(:[]=).with('my_key', 'my_value') - expect(logged_out_adapter_instance).to receive(:[]).with('my_key') { nil } - subject['my_key'] = 'my_value' + expect(logged_out_adapter_instance).to receive(:[]=).with("my_key", "my_value") + expect(logged_out_adapter_instance).to receive(:[]).with("my_key") { nil } + subject["my_key"] = "my_value" end - it '#[]' do + it "#[]" do expect_any_instance_of(logged_in_adapter).not_to receive(:[]) - expect(logged_out_adapter_instance).to receive(:[]).with('my_key') { 'my_value' } - expect(subject['my_key']).to eq('my_value') + expect(logged_out_adapter_instance).to receive(:[]).with("my_key") { "my_value" } + expect(subject["my_key"]).to eq("my_value") end - it '#delete' do - expect(logged_in_adapter_instance).to receive(:delete).with('my_key') { 'my_value' } - expect(logged_out_adapter_instance).to receive(:delete).with('my_key') { 'my_value' } - expect(subject.delete('my_key')).to eq('my_value') + it "#delete" do + expect(logged_in_adapter_instance).to receive(:delete).with("my_key") { "my_value" } + expect(logged_out_adapter_instance).to receive(:delete).with("my_key") { "my_value" } + expect(subject.delete("my_key")).to eq("my_value") end - it '#keys' do - expect(logged_in_adapter_instance).to receive(:keys) { ['my_value'] } - expect(logged_out_adapter_instance).to receive(:keys) { ['my_value', 'my_value2'] } - expect(subject.keys).to eq(['my_value', 'my_value2']) + it "#keys" do + expect(logged_in_adapter_instance).to receive(:keys) { ["my_value"] } + expect(logged_out_adapter_instance).to receive(:keys) { ["my_value", "my_value2"] } + expect(subject.keys).to eq(["my_value", "my_value2"]) end end end - describe 'when errors in config' do + describe "when errors in config" do before { described_class.config.clear } - let(:some_proc) { ->{} } + let(:some_proc) { -> { } } - it 'when no logged in adapter' do - expect{ + it "when no logged in adapter" do + expect { described_class.with_config( logged_in: some_proc, logged_out_adapter: logged_out_adapter @@ -173,8 +173,8 @@ }.to raise_error(StandardError, /:logged_in_adapter/) end - it 'when no logged out adapter' do - expect{ + it "when no logged out adapter" do + expect { described_class.with_config( logged_in: some_proc, logged_in_adapter: logged_in_adapter @@ -182,8 +182,8 @@ }.to raise_error(StandardError, /:logged_out_adapter/) end - it 'when no logged in detector' do - expect{ + it "when no logged in detector" do + expect { described_class.with_config( logged_in_adapter: logged_in_adapter, logged_out_adapter: logged_out_adapter diff --git a/spec/persistence/redis_adapter_spec.rb b/spec/persistence/redis_adapter_spec.rb index 1766c075..1a1eac11 100644 --- a/spec/persistence/redis_adapter_spec.rb +++ b/spec/persistence/redis_adapter_spec.rb @@ -1,67 +1,76 @@ # frozen_string_literal: true + require "spec_helper" describe Split::Persistence::RedisAdapter do - - let(:context) { double(:lookup => 'blah') } + let(:context) { double(lookup: "blah") } subject { Split::Persistence::RedisAdapter.new(context) } - describe '#redis_key' do + describe "#redis_key" do before { Split::Persistence::RedisAdapter.reset_config! } - context 'default' do - it 'should raise error with prompt to set lookup_by' do - expect{Split::Persistence::RedisAdapter.new(context)}.to raise_error(RuntimeError) + context "default" do + it "should raise error with prompt to set lookup_by" do + expect { Split::Persistence::RedisAdapter.new(context) }.to raise_error(RuntimeError) end end - context 'config with key' do + context "config with key" do before { Split::Persistence::RedisAdapter.reset_config! } - subject { Split::Persistence::RedisAdapter.new(context, 'manual') } + subject { Split::Persistence::RedisAdapter.new(context, "manual") } it 'should be "persistence:manual"' do - expect(subject.redis_key).to eq('persistence:manual') + expect(subject.redis_key).to eq("persistence:manual") end end context 'config with lookup_by = proc { "block" }' do - before { Split::Persistence::RedisAdapter.with_config(:lookup_by => proc{'block'}) } + before { Split::Persistence::RedisAdapter.with_config(lookup_by: proc { "block" }) } it 'should be "persistence:block"' do - expect(subject.redis_key).to eq('persistence:block') + expect(subject.redis_key).to eq("persistence:block") end end - context 'config with lookup_by = proc { |context| context.test }' do - before { Split::Persistence::RedisAdapter.with_config(:lookup_by => proc{'block'}) } - let(:context) { double(:test => 'block') } + context "config with lookup_by = proc { |context| context.test }" do + before { Split::Persistence::RedisAdapter.with_config(lookup_by: proc { "block" }) } + let(:context) { double(test: "block") } it 'should be "persistence:block"' do - expect(subject.redis_key).to eq('persistence:block') + expect(subject.redis_key).to eq("persistence:block") end end context 'config with lookup_by = "method_name"' do - before { Split::Persistence::RedisAdapter.with_config(:lookup_by => 'method_name') } - let(:context) { double(:method_name => 'val') } + before { Split::Persistence::RedisAdapter.with_config(lookup_by: "method_name") } + let(:context) { double(method_name: "val") } it 'should be "persistence:bar"' do - expect(subject.redis_key).to eq('persistence:val') + expect(subject.redis_key).to eq("persistence:val") end end - context 'config with namespace and lookup_by' do - before { Split::Persistence::RedisAdapter.with_config(:lookup_by => proc{'frag'}, :namespace => 'namer') } + context "config with namespace and lookup_by" do + before { Split::Persistence::RedisAdapter.with_config(lookup_by: proc { "frag" }, namespace: "namer") } it 'should be "namer"' do - expect(subject.redis_key).to eq('namer:frag') + expect(subject.redis_key).to eq("namer:frag") end end end - context 'functional tests' do - before { Split::Persistence::RedisAdapter.with_config(:lookup_by => 'lookup') } + describe "#find" do + before { Split::Persistence::RedisAdapter.with_config(lookup_by: proc { "frag" }, namespace: "a_namespace") } + + it "should create and user from a given key" do + adapter = Split::Persistence::RedisAdapter.find(2) + expect(adapter.redis_key).to eq("a_namespace:2") + end + end + + context "functional tests" do + before { Split::Persistence::RedisAdapter.with_config(lookup_by: "lookup") } describe "#[] and #[]=" do it "should set and return the value for given key" do @@ -85,6 +94,5 @@ expect(subject.keys).to match(["my_key", "my_second_key"]) end end - end end diff --git a/spec/persistence/session_adapter_spec.rb b/spec/persistence/session_adapter_spec.rb index 2d57d1ed..63074c36 100644 --- a/spec/persistence/session_adapter_spec.rb +++ b/spec/persistence/session_adapter_spec.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true + require "spec_helper" describe Split::Persistence::SessionAdapter do - - let(:context) { double(:session => {}) } + let(:context) { double(session: {}) } subject { Split::Persistence::SessionAdapter.new(context) } describe "#[] and #[]=" do @@ -28,5 +28,4 @@ expect(subject.keys).to match(["my_key", "my_second_key"]) end end - end diff --git a/spec/persistence_spec.rb b/spec/persistence_spec.rb index 346c293f..24a0dc2d 100644 --- a/spec/persistence_spec.rb +++ b/spec/persistence_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true + require "spec_helper" describe Split::Persistence do - subject { Split::Persistence } describe ".adapter" do @@ -30,5 +30,4 @@ end end end - end diff --git a/spec/redis_interface_spec.rb b/spec/redis_interface_spec.rb index 42c4e43c..578578f8 100644 --- a/spec/redis_interface_spec.rb +++ b/spec/redis_interface_spec.rb @@ -1,111 +1,44 @@ -require 'spec_helper' +# frozen_string_literal: true + +require "spec_helper" describe Split::RedisInterface do - let(:list_name) { 'list_name' } - let(:set_name) { 'set_name' } + let(:list_name) { "list_name" } + let(:set_name) { "set_name" } let(:interface) { described_class.new } - describe '#persist_list' do + describe "#persist_list" do subject(:persist_list) do interface.persist_list(list_name, %w(a b c d)) end specify do expect(persist_list).to eq %w(a b c d) - expect(Split.redis.lindex(list_name, 0)).to eq 'a' - expect(Split.redis.lindex(list_name, 1)).to eq 'b' - expect(Split.redis.lindex(list_name, 2)).to eq 'c' - expect(Split.redis.lindex(list_name, 3)).to eq 'd' + expect(Split.redis.lindex(list_name, 0)).to eq "a" + expect(Split.redis.lindex(list_name, 1)).to eq "b" + expect(Split.redis.lindex(list_name, 2)).to eq "c" + expect(Split.redis.lindex(list_name, 3)).to eq "d" expect(Split.redis.llen(list_name)).to eq 4 end - context 'list is overwritten but not deleted' do + context "list is overwritten but not deleted" do specify do expect(persist_list).to eq %w(a b c d) - interface.persist_list(list_name, ['z']) - expect(Split.redis.lindex(list_name, 0)).to eq 'z' + interface.persist_list(list_name, ["z"]) + expect(Split.redis.lindex(list_name, 0)).to eq "z" expect(Split.redis.llen(list_name)).to eq 1 end end end - describe '#add_to_list' do - subject(:add_to_list) do - interface.add_to_list(list_name, 'y') - interface.add_to_list(list_name, 'z') - end - - specify do - add_to_list - expect(Split.redis.lindex(list_name, 0)).to eq 'y' - expect(Split.redis.lindex(list_name, 1)).to eq 'z' - expect(Split.redis.llen(list_name)).to eq 2 - end - end - - describe '#set_list_index' do - subject(:set_list_index) do - interface.add_to_list(list_name, 'y') - interface.add_to_list(list_name, 'z') - interface.set_list_index(list_name, 0, 'a') - end - - specify do - set_list_index - expect(Split.redis.lindex(list_name, 0)).to eq 'a' - expect(Split.redis.lindex(list_name, 1)).to eq 'z' - expect(Split.redis.llen(list_name)).to eq 2 - end - end - - describe '#list_length' do - subject(:list_length) do - interface.add_to_list(list_name, 'y') - interface.add_to_list(list_name, 'z') - interface.list_length(list_name) - end - - specify do - expect(list_length).to eq 2 - end - end - - describe '#remove_last_item_from_list' do - subject(:remove_last_item_from_list) do - interface.add_to_list(list_name, 'y') - interface.add_to_list(list_name, 'z') - interface.remove_last_item_from_list(list_name) - end - - specify do - remove_last_item_from_list - expect(Split.redis.lindex(list_name, 0)).to eq 'y' - expect(Split.redis.llen(list_name)).to eq 1 - end - end - - describe '#make_list_length' do - subject(:make_list_length) do - interface.add_to_list(list_name, 'y') - interface.add_to_list(list_name, 'z') - interface.make_list_length(list_name, 1) - end - - specify do - make_list_length - expect(Split.redis.lindex(list_name, 0)).to eq 'y' - expect(Split.redis.llen(list_name)).to eq 1 - end - end - - describe '#add_to_set' do + describe "#add_to_set" do subject(:add_to_set) do - interface.add_to_set(set_name, 'something') + interface.add_to_set(set_name, "something") end specify do add_to_set - expect(Split.redis.sismember(set_name, 'something')).to be true + expect(Split.redis.sismember(set_name, "something")).to be true end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6951489f..3e9859c9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,37 +1,38 @@ # frozen_string_literal: true -ENV['RACK_ENV'] = "test" -require 'rubygems' -require 'bundler/setup' +ENV["RACK_ENV"] = "test" -require 'simplecov' -SimpleCov.start - -require 'split' -require 'ostruct' -require 'yaml' +require "rubygems" +require "bundler/setup" -Dir['./spec/support/*.rb'].each { |f| require f } +require "simplecov" +SimpleCov.start -require "fakeredis" +require "split" +require "ostruct" +require "yaml" -G_fakeredis = Redis.new +Dir["./spec/support/*.rb"].each { |f| require f } module GlobalSharedContext extend RSpec::SharedContext - let(:mock_user){ Split::User.new(double(session: {})) } + let(:mock_user) { Split::User.new(double(session: {})) } + before(:each) do Split.configuration = Split::Configuration.new - Split.redis = G_fakeredis - Split.redis.flushall + Split.redis = Redis.new + Split.redis.select(10) + Split.redis.flushdb + Split::Cache.clear @ab_user = mock_user - params = nil + @params = nil end end RSpec.configure do |config| - config.order = 'random' + config.order = "random" config.include GlobalSharedContext + config.raise_errors_for_deprecations! end def session @@ -42,11 +43,11 @@ def params @params ||= {} end -def request(ua = 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_6; de-de) AppleWebKit/533.20.25 (KHTML, like Gecko) Version/5.0.4 Safari/533.20.27') +def request(ua = "Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_6; de-de) AppleWebKit/533.20.25 (KHTML, like Gecko) Version/5.0.4 Safari/533.20.27") @request ||= begin r = OpenStruct.new r.user_agent = ua - r.ip = '192.168.1.1' + r.ip = "192.168.1.1" r end end diff --git a/spec/split_spec.rb b/spec/split_spec.rb index 51761637..a1361889 100644 --- a/spec/split_spec.rb +++ b/spec/split_spec.rb @@ -1,18 +1,18 @@ # frozen_string_literal: true -require 'spec_helper' -RSpec.describe Split do +require "spec_helper" +RSpec.describe Split do around(:each) do |ex| - old_env, old_redis = [ENV.delete('REDIS_URL'), Split.redis] + old_env, old_redis = [ENV.delete("REDIS_URL"), Split.redis] ex.run - ENV['REDIS_URL'] = old_env + ENV["REDIS_URL"] = old_env Split.redis = old_redis end - describe '#redis=' do - it 'accepts a url string' do - Split.redis = 'redis://localhost:6379' + describe "#redis=" do + it "accepts a url string" do + Split.redis = "redis://localhost:6379" expect(Split.redis).to be_a(Redis) client = Split.redis.connection @@ -20,8 +20,8 @@ expect(client[:port]).to eq(6379) end - it 'accepts an options hash' do - Split.redis = {host: 'localhost', port: 6379, db: 12} + it "accepts an options hash" do + Split.redis = { host: "localhost", port: 6379, db: 12 } expect(Split.redis).to be_a(Redis) client = Split.redis.connection @@ -30,13 +30,13 @@ expect(client[:db]).to eq(12) end - it 'accepts a valid Redis instance' do + it "accepts a valid Redis instance" do other_redis = Redis.new(url: "redis://localhost:6379") Split.redis = other_redis expect(Split.redis).to eq(other_redis) end - it 'raises an ArgumentError when server cannot be determined' do + it "raises an ArgumentError when server cannot be determined" do expect { Split.redis = Object.new }.to raise_error(ArgumentError) end end diff --git a/spec/support/cookies_mock.rb b/spec/support/cookies_mock.rb index 276ea694..c23fc8a0 100644 --- a/spec/support/cookies_mock.rb +++ b/spec/support/cookies_mock.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CookiesMock +class CookiesMock def initialize @cookies = {} end @@ -16,5 +16,4 @@ def [](key) def delete(key) @cookies.delete(key) end - end diff --git a/spec/trial_spec.rb b/spec/trial_spec.rb index c6819de5..67eba6e7 100644 --- a/spec/trial_spec.rb +++ b/spec/trial_spec.rb @@ -1,54 +1,55 @@ # frozen_string_literal: true -require 'spec_helper' -require 'split/trial' + +require "spec_helper" +require "split/trial" describe Split::Trial do let(:user) { mock_user } - let(:alternatives) { ['basket', 'cart'] } + let(:alternatives) { ["basket", "cart"] } let(:experiment) do - Split::Experiment.new('basket_text', :alternatives => alternatives).save + Split::Experiment.new("basket_text", alternatives: alternatives).save end it "should be initializeable" do - experiment = double('experiment') - alternative = double('alternative', :kind_of? => Split::Alternative) - trial = Split::Trial.new(:experiment => experiment, :alternative => alternative) + experiment = double("experiment") + alternative = double("alternative", kind_of?: Split::Alternative) + trial = Split::Trial.new(experiment: experiment, alternative: alternative) expect(trial.experiment).to eq(experiment) expect(trial.alternative).to eq(alternative) end describe "alternative" do it "should use the alternative if specified" do - alternative = double('alternative', :kind_of? => Split::Alternative) - trial = Split::Trial.new(:experiment => double('experiment'), - :alternative => alternative, :user => user) + alternative = double("alternative", kind_of?: Split::Alternative) + trial = Split::Trial.new(experiment: double("experiment"), + alternative: alternative, user: user) expect(trial).not_to receive(:choose) expect(trial.alternative).to eq(alternative) end it "should load the alternative when the alternative name is set" do - experiment = Split::Experiment.new('basket_text', :alternatives => ['basket', 'cart']) + experiment = Split::Experiment.new("basket_text", alternatives: ["basket", "cart"]) experiment.save - trial = Split::Trial.new(:experiment => experiment, :alternative => 'basket') - expect(trial.alternative.name).to eq('basket') + trial = Split::Trial.new(experiment: experiment, alternative: "basket") + expect(trial.alternative.name).to eq("basket") end end describe "metadata" do let(:metadata) { Hash[alternatives.map { |k| [k, "Metadata for #{k}"] }] } let(:experiment) do - Split::Experiment.new('basket_text', :alternatives => alternatives, :metadata => metadata).save + Split::Experiment.new("basket_text", alternatives: alternatives, metadata: metadata).save end - it 'has metadata on each trial' do - trial = Split::Trial.new(:experiment => experiment, :user => user, :metadata => metadata['cart'], - :override => 'cart') - expect(trial.metadata).to eq(metadata['cart']) + it "has metadata on each trial" do + trial = Split::Trial.new(experiment: experiment, user: user, metadata: metadata["cart"], + override: "cart") + expect(trial.metadata).to eq(metadata["cart"]) end - it 'has metadata on each trial from the experiment' do - trial = Split::Trial.new(:experiment => experiment, :user => user) + it "has metadata on each trial from the experiment" do + trial = Split::Trial.new(experiment: experiment, user: user) trial.choose! expect(trial.metadata).to eq(metadata[trial.alternative.name]) expect(trial.metadata).to match(/#{trial.alternative.name}/) @@ -56,24 +57,24 @@ end describe "#choose!" do - let(:context) { double(on_trial_callback: 'test callback') } + let(:context) { double(on_trial_callback: "test callback") } let(:trial) do - Split::Trial.new(:user => user, :experiment => experiment) + Split::Trial.new(user: user, experiment: experiment) end - shared_examples_for 'a trial with callbacks' do - it 'does not run if on_trial callback is not respondable' do + shared_examples_for "a trial with callbacks" do + it "does not run if on_trial callback is not respondable" do Split.configuration.on_trial = :foo allow(context).to receive(:respond_to?).with(:foo, true).and_return false expect(context).to_not receive(:foo) trial.choose! context end - it 'runs on_trial callback' do + it "runs on_trial callback" do Split.configuration.on_trial = :on_trial_callback expect(context).to receive(:on_trial_callback) trial.choose! context end - it 'does not run nil on_trial callback' do + it "does not run nil on_trial callback" do Split.configuration.on_trial = nil expect(context).not_to receive(:on_trial_callback) trial.choose! context @@ -88,12 +89,12 @@ def expect_alternative(trial, alternative_name) end context "when override is present" do - let(:override) { 'cart' } + let(:override) { "cart" } let(:trial) do - Split::Trial.new(:user => user, :experiment => experiment, :override => override) + Split::Trial.new(user: user, experiment: experiment, override: override) end - it_behaves_like 'a trial with callbacks' + it_behaves_like "a trial with callbacks" it "picks the override" do expect(experiment).to_not receive(:next_alternative) @@ -102,7 +103,7 @@ def expect_alternative(trial, alternative_name) context "when alternative doesn't exist" do let(:override) { nil } - it 'falls back on next_alternative' do + it "falls back on next_alternative" do expect(experiment).to receive(:next_alternative).and_call_original expect_alternative(trial, alternatives) end @@ -111,7 +112,7 @@ def expect_alternative(trial, alternative_name) context "when disabled option is true" do let(:trial) do - Split::Trial.new(:user => user, :experiment => experiment, :disabled => true) + Split::Trial.new(user: user, experiment: experiment, disabled: true) end it "picks the control", :aggregate_failures do @@ -120,7 +121,7 @@ def expect_alternative(trial, alternative_name) expect(context).not_to receive(:on_trial_callback) - expect_alternative(trial, 'basket') + expect_alternative(trial, "basket") Split.configuration.on_trial = nil end end @@ -132,7 +133,7 @@ def expect_alternative(trial, alternative_name) expect(experiment).to_not receive(:next_alternative) expect(context).not_to receive(:on_trial_callback) - expect_alternative(trial, 'basket') + expect_alternative(trial, "basket") Split.configuration.enabled = true Split.configuration.on_trial = nil @@ -141,45 +142,45 @@ def expect_alternative(trial, alternative_name) context "when experiment has winner" do let(:trial) do - Split::Trial.new(:user => user, :experiment => experiment) + Split::Trial.new(user: user, experiment: experiment) end - it_behaves_like 'a trial with callbacks' + it_behaves_like "a trial with callbacks" it "picks the winner" do - experiment.winner = 'cart' + experiment.winner = "cart" expect(experiment).to_not receive(:next_alternative) - expect_alternative(trial, 'cart') + expect_alternative(trial, "cart") end end context "when exclude is true" do let(:trial) do - Split::Trial.new(:user => user, :experiment => experiment, :exclude => true) + Split::Trial.new(user: user, experiment: experiment, exclude: true) end - it_behaves_like 'a trial with callbacks' + it_behaves_like "a trial with callbacks" it "picks the control" do expect(experiment).to_not receive(:next_alternative) - expect_alternative(trial, 'basket') + expect_alternative(trial, "basket") end end context "when user is already participating" do - it_behaves_like 'a trial with callbacks' + it_behaves_like "a trial with callbacks" it "picks the same alternative" do - user[experiment.key] = 'basket' + user[experiment.key] = "basket" expect(experiment).to_not receive(:next_alternative) - expect_alternative(trial, 'basket') + expect_alternative(trial, "basket") end context "when alternative is not found" do it "falls back on next_alternative" do - user[experiment.key] = 'notfound' + user[experiment.key] = "notfound" expect(experiment).to receive(:next_alternative).and_call_original expect_alternative(trial, alternatives) end @@ -327,7 +328,7 @@ def expect_alternative(trial, alternative_name) expect(experiment).to_not receive(:next_alternative) expect(context).not_to receive(:on_trial_callback) - expect_alternative(trial, 'basket') + expect_alternative(trial, "basket") Split.configuration.enabled = true Split.configuration.on_trial = nil @@ -343,7 +344,7 @@ def expect_alternative(trial, alternative_name) end describe "#complete!" do - let(:trial) { Split::Trial.new(:user => user, :experiment => experiment) } + let(:trial) { Split::Trial.new(user: user, experiment: experiment) } before do allow(Split.configuration).to receive(:experiments).and_return(experiment.name => { "window_of_time_for_conversion_in_minutes" => 60 }) @@ -392,9 +393,9 @@ def expect_alternative(trial, alternative_name) expect(trial.alternative.completed_count).to be(old_completed_count+1) end - context 'when there are no goals' do - let(:trial) { Split::Trial.new(:user => user, :experiment => experiment) } - it 'should complete the trial' do + context "when there are no goals" do + let(:trial) { Split::Trial.new(user: user, experiment: experiment) } + it "should complete the trial" do trial.choose! old_completed_count = trial.alternative.completed_count trial.complete! @@ -404,11 +405,11 @@ def expect_alternative(trial, alternative_name) context "when there are many goals" do let(:goals) { [ "goal1", "goal2" ] } - let(:trial) { Split::Trial.new(:user => user, :experiment => experiment, :goals => goals) } + let(:trial) { Split::Trial.new(user: user, experiment: experiment, goals: goals) } it "increments the completed count corresponding to the goals" do trial.choose! - old_completed_counts = goals.map{ |goal| [goal, trial.alternative.completed_count(goal)] }.to_h + old_completed_counts = goals.map { |goal| [goal, trial.alternative.completed_count(goal)] }.to_h trial.complete! goals.each { | goal | expect(trial.alternative.completed_count(goal)).to eq(old_completed_counts[goal] + 1) } end @@ -416,7 +417,7 @@ def expect_alternative(trial, alternative_name) context "when there is 1 goal of type string" do let(:goal) { "goal" } - let(:trial) { Split::Trial.new(:user => user, :experiment => experiment, :goals => goal) } + let(:trial) { Split::Trial.new(user: user, experiment: experiment, goals: goal) } it "increments the completed count corresponding to the goal" do trial.choose! old_completed_count = trial.alternative.completed_count(goal) @@ -427,7 +428,7 @@ def expect_alternative(trial, alternative_name) end describe "#within_conversion_time_frame?" do - let(:trial) { Split::Trial.new(:user => user, :experiment => experiment) } + let(:trial) { Split::Trial.new(user: user, experiment: experiment) } it "memoizes the result" do allow(Split.configuration).to receive(:experiments).and_return(experiment.name => { "window_of_time_for_conversion_in_minutes" => 60 }) @@ -484,7 +485,7 @@ def expect_alternative(trial, alternative_name) context "when override is present" do it "stores when store_override is true" do - trial = Split::Trial.new(:user => user, :experiment => experiment, :override => 'basket') + trial = Split::Trial.new(user: user, experiment: experiment, override: "basket") Split.configuration.store_override = true expect(user).to receive("[]=") @@ -493,7 +494,7 @@ def expect_alternative(trial, alternative_name) end it "does not store when store_override is false" do - trial = Split::Trial.new(:user => user, :experiment => experiment, :override => 'basket') + trial = Split::Trial.new(user: user, experiment: experiment, override: "basket") expect(user).to_not receive("[]=") trial.choose! @@ -502,7 +503,7 @@ def expect_alternative(trial, alternative_name) context "when disabled is present" do it "stores when store_override is true" do - trial = Split::Trial.new(:user => user, :experiment => experiment, :disabled => true) + trial = Split::Trial.new(user: user, experiment: experiment, disabled: true) Split.configuration.store_override = true expect(user).to receive("[]=") @@ -510,7 +511,7 @@ def expect_alternative(trial, alternative_name) end it "does not store when store_override is false" do - trial = Split::Trial.new(:user => user, :experiment => experiment, :disabled => true) + trial = Split::Trial.new(user: user, experiment: experiment, disabled: true) expect(user).to_not receive("[]=") trial.choose! @@ -519,20 +520,20 @@ def expect_alternative(trial, alternative_name) context "when exclude is present" do it "does not store" do - trial = Split::Trial.new(:user => user, :experiment => experiment, :exclude => true) + trial = Split::Trial.new(user: user, experiment: experiment, exclude: true) expect(user).to_not receive("[]=") trial.choose! end end - context 'when experiment has winner' do + context "when experiment has winner" do let(:trial) do - experiment.winner = 'cart' - Split::Trial.new(:user => user, :experiment => experiment) + experiment.winner = "cart" + Split::Trial.new(user: user, experiment: experiment) end - it 'does not store' do + it "does not store" do expect(user).to_not receive("[]=") trial.choose! end diff --git a/spec/user_spec.rb b/spec/user_spec.rb index f83d3eef..1c5dab0c 100644 --- a/spec/user_spec.rb +++ b/spec/user_spec.rb @@ -1,22 +1,47 @@ -require 'spec_helper' -require 'split/experiment_catalog' -require 'split/experiment' -require 'split/user' +# frozen_string_literal: true + +require "spec_helper" +require "split/experiment_catalog" +require "split/experiment" +require "split/user" describe Split::User do - let(:user_keys) { { 'link_color' => 'blue' } } - let(:context) { double(:session => { split: user_keys }) } - let(:experiment) { Split::Experiment.new('link_color') } + let(:user_keys) { { "link_color" => "blue" } } + let(:context) { double(session: { split: user_keys }) } + let(:experiment) { Split::Experiment.new("link_color") } before(:each) do @subject = described_class.new(context) end - it 'delegates methods correctly' do - expect(@subject['link_color']).to eq(@subject.user['link_color']) + it "delegates methods correctly" do + expect(@subject["link_color"]).to eq(@subject.user["link_color"]) end - describe '#cleanup_old_versions!' do + context "#cleanup_old_versions!" do + context "when there are multiple variations" do + let(:experiment_version) { "#{experiment.name}:1" } + let(:second_experiment_version) { "#{experiment.name}_another:1" } + let(:third_experiment_version) { "variation_of_#{experiment.name}:1" } + let(:user_keys) do + { + experiment_version => "blue", + second_experiment_version => "red", + third_experiment_version => "yellow" + } + end + + before(:each) { @subject.cleanup_old_versions!(experiment) } + + it "removes key if old experiment is found" do + expect(@subject.keys).not_to include(experiment_version) + end + + it "does not remove other keys" do + expect(@subject.keys).to include(second_experiment_version, third_experiment_version) + end + end + context 'current version does not have number' do describe 'when the user is in the experiment with old version number' do let(:user_keys) { { 'link_color:1' => 'blue', 'link_color:1:finished' => 'true' } } @@ -124,9 +149,9 @@ end end end - end + end - context '#cleanup_old_experiments!' do + context "#cleanup_old_experiments!" do let(:user_keys) do { 'link_color' => 'blue', @@ -135,39 +160,33 @@ } end - it 'removes keys if experiment is not found' do + it "removes key if experiment is not found" do @subject.cleanup_old_experiments! expect(@subject.keys).to be_empty end - it 'removes keys if experiment has a winner' do - allow(Split::ExperimentCatalog).to receive(:find).with('link_color').and_return(experiment) - allow(experiment).to receive(:has_winner?).and_return(true) + it "removes key if experiment has a winner" do + allow(Split::ExperimentCatalog).to receive(:find).with("link_color").and_return(experiment) allow(experiment).to receive(:start_time).and_return(Date.today) - + allow(experiment).to receive(:has_winner?).and_return(true) @subject.cleanup_old_experiments! - expect(@subject.keys).to be_empty end - it 'removes keys if experiment has not started yet' do - allow(Split::ExperimentCatalog).to receive(:find).with('link_color').and_return(experiment) + it "removes key if experiment has not started yet" do + allow(Split::ExperimentCatalog).to receive(:find).with("link_color").and_return(experiment) allow(experiment).to receive(:has_winner?).and_return(false) allow(experiment).to receive(:start_time).and_return(nil) - @subject.cleanup_old_experiments! - expect(@subject.keys).to be_empty end - it 'keeps keys if the experiment has no winner and has started' do + it "keeps keys if the experiment has no winner and has started" do allow(Split::ExperimentCatalog).to receive(:find).with('link_color').and_return(experiment) allow(experiment).to receive(:has_winner?).and_return(false) allow(experiment).to receive(:start_time).and_return(Date.today) - @subject.cleanup_old_experiments! - expect(@subject.keys).to include("link_color") expect(@subject.keys).to include("link_color:finished") expect(@subject.keys).to include("link_color:time_of_assignment") @@ -182,26 +201,38 @@ } end - it 'keeps keys when the experiment has no winner and has started' do + it "keeps keys when the experiment has no winner and has started" do allow(Split::ExperimentCatalog).to receive(:find).with('link_color').and_return(experiment) allow(experiment).to receive(:has_winner?).and_return(false) allow(experiment).to receive(:start_time).and_return(Date.today) - @subject.cleanup_old_experiments! - expect(@subject.keys).to include("link_color:1") expect(@subject.keys).to include("link_color:1:finished") expect(@subject.keys).to include("link_color:1:time_of_assignment") end end - context 'when already cleaned up' do + context "with finished key" do + let(:user_keys) { { "link_color" => "blue", "link_color:finished" => true } } + + it "does not remove finished key for experiment without a winner" do + allow(Split::ExperimentCatalog).to receive(:find).with("link_color").and_return(experiment) + allow(Split::ExperimentCatalog).to receive(:find).with("link_color:finished").and_return(nil) + allow(experiment).to receive(:start_time).and_return(Date.today) + allow(experiment).to receive(:has_winner?).and_return(false) + @subject.cleanup_old_experiments! + expect(@subject.keys).to include("link_color") + expect(@subject.keys).to include("link_color:finished") + end + end + + context "when already cleaned up" do before do @subject.cleanup_old_experiments! end - it 'does not clean up again' do - expect(@subject).to_not receive(:experiment_keys) + it "does not clean up again" do + expect(@subject).to_not receive(:keys_without_finished) @subject.cleanup_old_experiments! end end @@ -472,6 +503,22 @@ end end + context "allows user to be loaded from adapter" do + it "loads user from adapter (RedisAdapter)" do + user = Split::Persistence::RedisAdapter.new(nil, 112233) + user["foo"] = "bar" + + ab_user = Split::User.find(112233, :redis) + + expect(ab_user["foo"]).to eql("bar") + end + + it "returns nil if adapter does not implement a finder method" do + ab_user = Split::User.find(112233, :dual_adapter) + expect(ab_user).to be_nil + end + end + context "instantiated with custom adapter" do let(:custom_adapter) { double(:persistence_adapter) } @@ -483,5 +530,4 @@ expect(@subject.user).to eq(custom_adapter) end end - end diff --git a/split.gemspec b/split.gemspec index f16e7df8..4c56739c 100644 --- a/split.gemspec +++ b/split.gemspec @@ -1,5 +1,6 @@ # -*- encoding: utf-8 -*- # frozen_string_literal: true + $:.push File.expand_path("../lib", __FILE__) require "split/version" @@ -8,7 +9,7 @@ Gem::Specification.new do |s| s.version = Split::VERSION s.platform = Gem::Platform::RUBY s.authors = ["Andrew Nesbitt"] - s.licenses = ['MIT'] + s.licenses = ["MIT"] s.email = ["andrewnez@gmail.com"] s.homepage = "https://github.com/splitrb/split" s.summary = "Rack based split testing framework" @@ -22,23 +23,22 @@ Gem::Specification.new do |s| "mailing_list_uri" => "https://groups.google.com/d/forum/split-ruby" } - s.required_ruby_version = '>= 2.2.2' - s.required_rubygems_version = '>= 2.0.0' + s.required_ruby_version = ">= 2.5.0" + s.required_rubygems_version = ">= 2.0.0" s.files = `git ls-files`.split("\n") s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n") s.require_paths = ["lib"] - s.add_dependency 'redis', '>= 2.1' - s.add_dependency 'sinatra', '>= 1.2.6' - s.add_dependency 'rubystats', '>= 0.3.0' + s.add_dependency "redis", ">= 4.2" + s.add_dependency "sinatra", ">= 1.2.6" + s.add_dependency "rubystats", ">= 0.3.0" - s.add_development_dependency 'bundler', '>= 1.17' - s.add_development_dependency 'simplecov', '~> 0.15' - s.add_development_dependency 'rack-test', '~> 1.1' - s.add_development_dependency 'rake', '~> 13' - s.add_development_dependency 'rspec', '~> 3.7' - s.add_development_dependency 'pry', '~> 0.10' - s.add_development_dependency 'fakeredis', '~> 0.7' - s.add_development_dependency 'rails', '>= 5.0' + s.add_development_dependency "bundler", ">= 1.17" + s.add_development_dependency "simplecov", "~> 0.15" + s.add_development_dependency "rack-test", "~> 1.1" + s.add_development_dependency "rake", "~> 13" + s.add_development_dependency "rspec", "~> 3.7" + s.add_development_dependency "pry", "~> 0.10" + s.add_development_dependency "rails", ">= 5.0" end