Skip to content

Commit

Permalink
Add new test cases for attributes (and fix failing tests) (#31)
Browse files Browse the repository at this point in the history
  • Loading branch information
jdorn committed Oct 30, 2023
1 parent d6042bd commit f412c1b
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 18 deletions.
6 changes: 3 additions & 3 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ Metrics/BlockLength:
Max: 40

Metrics/CyclomaticComplexity:
Max: 25
Max: 40

Metrics/PerceivedComplexity:
Max: 25
Max: 40

Metrics/ParameterLists:
Max: 10
Expand All @@ -29,7 +29,7 @@ Metrics/ClassLength:
Max: 250

Metrics/MethodLength:
Max: 60
Max: 100

Layout/LineLength:
Max: 200
Expand Down
2 changes: 1 addition & 1 deletion badge.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
61 changes: 50 additions & 11 deletions lib/growthbook/conditions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,18 @@ def self.elem_match(condition, attribute_value)
false
end

def self.compare(val1, val2)
if val1.is_a?(Numeric) || val2.is_a?(Numeric)
val1 = val1.is_a?(Numeric) ? val1 : val1.to_f
val2 = val2.is_a?(Numeric) ? val2 : val2.to_f
end

return 1 if val1 > val2
return -1 if val1 < val2

0
end

def self.eval_operator_condition(operator, attribute_value, condition_value)
case operator
when '$veq'
Expand All @@ -119,17 +131,41 @@ def self.eval_operator_condition(operator, attribute_value, condition_value)
when '$vlte'
padded_version_string(attribute_value) <= padded_version_string(condition_value)
when '$eq'
attribute_value == condition_value
begin
compare(attribute_value, condition_value).zero?
rescue StandardError
false
end
when '$ne'
attribute_value != condition_value
begin
compare(attribute_value, condition_value) != 0
rescue StandardError
false
end
when '$lt'
attribute_value < condition_value
begin
compare(attribute_value, condition_value).negative?
rescue StandardError
false
end
when '$lte'
attribute_value <= condition_value
begin
compare(attribute_value, condition_value) <= 0
rescue StandardError
false
end
when '$gt'
attribute_value > condition_value
begin
compare(attribute_value, condition_value).positive?
rescue StandardError
false
end
when '$gte'
attribute_value >= condition_value
begin
compare(attribute_value, condition_value) >= 0
rescue StandardError
false
end
when '$regex'
silence_warnings do
re = Regexp.new(condition_value)
Expand All @@ -138,8 +174,12 @@ def self.eval_operator_condition(operator, attribute_value, condition_value)
false
end
when '$in'
return false unless condition_value.is_a?(Array)

in?(attribute_value, condition_value)
when '$nin'
return false unless condition_value.is_a?(Array)

!in?(attribute_value, condition_value)
when '$elemMatch'
elem_match(condition_value, attribute_value)
Expand Down Expand Up @@ -178,21 +218,20 @@ def self.padded_version_string(input)
# Remove build info and leading `v` if any
# Split version into parts (both core version numbers and pre-release tags)
# "v1.2.3-rc.1+build123" -> ["1","2","3","rc","1"]
parts = input.gsub(/(^v|\+.*$)/, "").split(/[-.]/)
parts = input.gsub(/(^v|\+.*$)/, '').split(/[-.]/)

# If it's SemVer without a pre-release, add `~` to the end
# ["1","0","0"] -> ["1","0","0","~"]
# "~" is the largest ASCII character, so this will make "1.0.0" greater than "1.0.0-beta" for example
parts << "~" if(parts.length == 3)
parts << '~' if parts.length == 3

# Left pad each numeric part with spaces so string comparisons will work ("9">"10", but " 9"<"10")
parts.map do |part|
part.match(/^[0-9]+$/) ? part.rjust(5, " ") : part
end.join("-")
/^[0-9]+$/.match?(part) ? part.rjust(5, ' ') : part
end.join('-')
end

def self.in?(actual, expected)
return false unless expected.is_a?(Array)
return expected.include?(actual) unless actual.is_a?(Array)

(actual & expected).any?
Expand Down
4 changes: 2 additions & 2 deletions lib/growthbook/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def track_experiment(experiment, result)
def included_in_rollout?(seed:, hash_attribute:, hash_version:, range:, coverage:)
return true if range.nil? && coverage.nil?

hash_value = get_attribute(hash_attribute)
hash_value = get_attribute(hash_attribute).to_s

return false if hash_value.empty?

Expand All @@ -344,7 +344,7 @@ def included_in_rollout?(seed:, hash_attribute:, hash_version:, range:, coverage

def filtered_out?(filters)
filters.any? do |filter|
hash_value = get_attribute(filter['attribute'] || 'id')
hash_value = get_attribute(filter['attribute'] || 'id').to_s

if hash_value.empty?
false
Expand Down
2 changes: 2 additions & 0 deletions sig/lib/growthbook/conditions.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ module Growthbook

def self.in?: (untyped attribute_value, untyped condition_value) -> bool

def self.compare: (untyped val1, untyped val2) -> Integer

# Sets $VERBOSE for the duration of the block and back to its original
# value afterwards. Used for testing invalid regexes.
def self.silence_warnings: () { () -> untyped } -> untyped
Expand Down
68 changes: 67 additions & 1 deletion spec/cases.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"specVersion": "0.5.1",
"specVersion": "0.5.2",
"evalCondition": [
[
"$not - pass",
Expand Down Expand Up @@ -617,6 +617,46 @@
},
false
],
[
"missing attribute with comparison operators",
{
"age": {
"$gt": -10,
"$lt": 10,
"$gte": -9,
"$lte": 9,
"$ne": 10
}
},
{},
true
],
[
"comparing numbers and strings",
{
"n": {
"$gt": 5,
"$lt": 10
}
},
{
"n": "8"
},
true
],
[
"comparing numbers and strings - v2",
{
"n": {
"$gt": "5",
"$lt": "10"
}
},
{
"n": 8
},
true
],
[
"empty $or - pass",
{
Expand Down Expand Up @@ -2296,6 +2336,32 @@
"source": "force"
}
],
[
"force rule - coverage with integer hash attribute",
{
"attributes": {
"id": 3
},
"features": {
"feature": {
"defaultValue": 2,
"rules": [
{
"force": 1,
"coverage": 0.5
}
]
}
}
},
"feature",
{
"value": 1,
"on": true,
"off": false,
"source": "force"
}
],
[
"force rules - coverage excluded",
{
Expand Down

0 comments on commit f412c1b

Please sign in to comment.