From a531078dcaf68da2a2d65beac04df1a1c846b966 Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Thu, 16 Jul 2026 13:56:42 -0400 Subject: [PATCH] Bug 2054479 - Require targeting and bucketing for Firefox Labs When queried for the list of all available Firefox Labs, the Nimbus Client was not filtering the recipes by whether they passed targeting and bucketing. This is now the case. There is now a new function `can_enroll` that returns an enum, `CanEnrollResult`, that distinguishes all the different cases that prevent enrollment. The majority of this function was refactored out of `evaluate_enrollment`, which is now a much simpler function. Additionally, the `targeting(expr, helper)` function has been removed. This function took an expression and a `NimbusTargetingHelper` and returned an `EnrollmentStatus` if and only if the targeting evaluation did not succeed or resulted in an error. This API was too awkward to work into `can_enroll`. The majority of this change is test fallout from removing this function, though the tests make more sense now that they are testing the results of JEXL evaluation and not comparing `EnrollmentStatus`es. In order to remove a bunch of unnecessary clones, the `NimbusTargetingHelper` methods now take a `&str` instead of a `String`. The UDL has been updated to use `[ByRef]` so that the FFI contract is unchanged. --- CHANGELOG.md | 1 + components/nimbus/src/evaluator.rs | 209 +++++---- components/nimbus/src/nimbus.udl | 6 +- components/nimbus/src/schema.rs | 4 +- components/nimbus/src/stateful/dbcache.rs | 16 +- .../nimbus/src/stateful/nimbus_client.rs | 14 +- components/nimbus/src/targeting.rs | 10 +- .../src/tests/stateful/test_behavior.rs | 46 +- .../src/tests/stateful/test_evaluator.rs | 442 +++++++----------- .../nimbus/src/tests/stateful/test_nimbus.rs | 89 ++-- .../src/tests/stateful/test_targeting.rs | 12 +- components/nimbus/src/tests/test_evaluator.rs | 221 ++++----- .../nimbus/tests/test_message_helpers.rs | 28 +- components/nimbus/tests/test_restart.rs | 18 +- 14 files changed, 503 insertions(+), 613 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b51545a608..79e3985c6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ ### Nimbus - Reasons are now reported in unenrollment events. ([#7480](https://github.com/mozilla/application-services/pull/7480)) +- The available Firefox Labs are now filtered by targeting and bucketing. ([#7481](https://github.com/mozilla/application-services/pull/7481)) # v153.0 (_2026-06-15_) diff --git a/components/nimbus/src/evaluator.rs b/components/nimbus/src/evaluator.rs index 553830ef3c..1404f07722 100644 --- a/components/nimbus/src/evaluator.rs +++ b/components/nimbus/src/evaluator.rs @@ -50,79 +50,123 @@ pub fn split_locale(locale: String) -> (Option, Option) { /// Determine the enrolment status for an experiment. /// -/// # Arguments: -/// - `available_randomization_units` The app provided available randomization units -/// - `targeting_attributes` The attributes to use when evaluating targeting -/// - `exp` The `Experiment` to evaluate. -/// -/// # Returns: -/// An `ExperimentEnrollment` - you need to inspect the EnrollmentStatus to -/// determine if the user is actually enrolled. -/// -/// # Errors: -/// -/// The function can return errors in one of the following cases (but not limited to): +/// # Errors /// -/// - If the bucket sampling failed (i.e we could not find if the user should or should not be enrolled in the experiment based on the bucketing) -/// - If an error occurs while determining the branch the user should be enrolled in any of the experiments +/// The function can return an error when branch selection fails due to an +/// invalid bucketing configuration. pub fn evaluate_enrollment( available_randomization_units: &AvailableRandomizationUnits, - exp: &Experiment, - th: &NimbusTargetingHelper, + experiment: &Experiment, + targeting_helper: &NimbusTargetingHelper, ) -> Result { - if let ExperimentAvailable::Unavailable { reason } = is_experiment_available(th, exp, true) { - return Ok(ExperimentEnrollment { - slug: exp.slug.clone(), - status: EnrollmentStatus::NotEnrolled { reason }, - }); - } + let status = match can_enroll(available_randomization_units, targeting_helper, experiment) { + CanEnrollResult::Unavailable { reason } => EnrollmentStatus::NotEnrolled { reason }, + CanEnrollResult::NotTargeted => EnrollmentStatus::NotEnrolled { + reason: NotEnrolledReason::NotTargeted, + }, + CanEnrollResult::NotSelected => EnrollmentStatus::NotEnrolled { + reason: NotEnrolledReason::NotSelected, + }, + CanEnrollResult::TargetingError { reason } => EnrollmentStatus::Error { reason }, + CanEnrollResult::NoRandomizationUnit => { + info!( + "Could not find a suitable randomization unit for {}. Skipping experiment.", + experiment.slug, + ); + EnrollmentStatus::Error { + reason: "No randomization unit".into(), + } + } + + CanEnrollResult::Enrollable { randomization_id } => EnrollmentStatus::new_enrolled( + EnrolledReason::Qualified, + &choose_branch(&experiment.slug, &experiment.branches, randomization_id)?.slug, + ), + }; + + Ok(ExperimentEnrollment { + slug: experiment.slug.clone(), + status, + }) +} + +/// Whether or not an experiment can be enrolled. +pub enum CanEnrollResult<'aru> { + /// The experiment is enrollable. + Enrollable { + /// The randomization ID that should be used for branch selection. + randomization_id: &'aru str, + }, + + /// The experiment is not available for a reason outlined in [`NotEnrolledReason`] + Unavailable { + /// The reason the enrollment is not available. + reason: NotEnrolledReason, + }, + + /// The experiment is not enrollable due to a targeting error. + TargetingError { + /// The stringified error. + reason: String, + }, + + /// The experiment is not enrollable because targeting expression evaluated + /// to false. + NotTargeted, - // Get targeting out of the way - "if let chains" are experimental, - // otherwise we could improve this. - if let Some(expr) = &exp.targeting - && let Some(status) = targeting(expr, th) + /// The experiment is not enrollable because randomization ID did not fall + /// into a selected bucket. + NotSelected, + + /// The experiment is not enrollable because it requires a randomization + /// unit that is not available. + NoRandomizationUnit, +} + +/// Determine whether or not it is possible to enroll in the given experiment. +pub fn can_enroll<'aru>( + available_randomization_units: &'aru AvailableRandomizationUnits, + targeting_helper: &NimbusTargetingHelper, + experiment: &Experiment, +) -> CanEnrollResult<'aru> { + if let ExperimentAvailable::Unavailable { reason } = + is_experiment_available(targeting_helper, experiment, true) { - return Ok(ExperimentEnrollment { - slug: exp.slug.clone(), - status, - }); + return CanEnrollResult::Unavailable { reason }; } - Ok(ExperimentEnrollment { - slug: exp.slug.clone(), - status: { - let bucket_config = exp.bucket_config.clone(); - match available_randomization_units.get_value(&bucket_config.randomization_unit) { - Some(id) => { - if sampling::bucket_sample( - vec![id.to_owned(), bucket_config.namespace], - bucket_config.start, - bucket_config.count, - bucket_config.total, - )? { - EnrollmentStatus::new_enrolled( - EnrolledReason::Qualified, - &choose_branch(&exp.slug, &exp.branches, id)?.clone().slug, - ) - } else { - EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotSelected, - } - } - } - None => { - // XXX: When we link in glean, it would be nice if we could emit - // a failure telemetry event here. - info!( - "Could not find a suitable randomization unit for {}. Skipping experiment.", - &exp.slug - ); - EnrollmentStatus::Error { - reason: "No randomization unit".into(), - } - } + + if let Some(targeting_expression) = &experiment.targeting { + match targeting_helper.eval_jexl(targeting_expression) { + Err(e) => { + return CanEnrollResult::TargetingError { + reason: e.to_string(), + }; } - }, - }) + Ok(false) => return CanEnrollResult::NotTargeted, + Ok(true) => {} + }; + } + + let Some(randomization_id) = + available_randomization_units.get_value(&experiment.bucket_config.randomization_unit) + else { + return CanEnrollResult::NoRandomizationUnit; + }; + + let Ok(is_sampled) = sampling::bucket_sample( + [randomization_id, &experiment.bucket_config.namespace], + experiment.bucket_config.start, + experiment.bucket_config.count, + experiment.bucket_config.total, + ) else { + return CanEnrollResult::NoRandomizationUnit; + }; + + if is_sampled { + CanEnrollResult::Enrollable { randomization_id } + } else { + CanEnrollResult::NotSelected + } } /// Whether or not an experiment is available. @@ -222,41 +266,6 @@ pub(crate) fn choose_branch<'a>( branches.get(index).ok_or(NimbusError::OutOfBoundsError) } -/// Checks if the client is targeted by an experiment -/// This api evaluates the JEXL statement retrieved from the server -/// against the application context provided by the client -/// -/// # Arguments -/// - `expression_statement`: The JEXL statement provided by the server -/// - `targeting_attributes`: The client attributes to target against -/// -/// If this app can not be targeted, returns an EnrollmentStatus to indicate -/// why. Returns None if we should continue to evaluate the enrollment status. -/// -/// In practice, if this returns an EnrollmentStatus, it will be either -/// EnrollmentStatus::NotEnrolled, or EnrollmentStatus::Error in the following -/// cases (But not limited to): -/// - The `expression_statement` is not a valid JEXL statement -/// - The `expression_statement` expects fields that do not exist in the AppContext definition -/// - The result of evaluating the statement against the context is not a boolean -/// - jexl-rs returned an error -pub(crate) fn targeting( - expression_statement: &str, - targeting_helper: &NimbusTargetingHelper, -) -> Option { - match targeting_helper.eval_jexl(expression_statement.to_string()) { - Ok(res) => match res { - true => None, - false => Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted, - }), - }, - Err(e) => Some(EnrollmentStatus::Error { - reason: e.to_string(), - }), - } -} - #[cfg(test)] mod unit_tests { use super::*; diff --git a/components/nimbus/src/nimbus.udl b/components/nimbus/src/nimbus.udl index 62b8919088..7eb73f04bc 100644 --- a/components/nimbus/src/nimbus.udl +++ b/components/nimbus/src/nimbus.udl @@ -154,7 +154,7 @@ interface GeckoPrefHandler { void set_gecko_prefs_state(sequence new_prefs_state); void set_gecko_prefs_original_values(sequence original_gecko_prefs); - + }; dictionary GeckoPref { @@ -469,12 +469,12 @@ interface NimbusTargetingHelper { /// Execute the given jexl expression and evaluate against the existing targeting parameters and context passed to /// the helper at construction. [Throws=NimbusError] - boolean eval_jexl(string expression); + boolean eval_jexl([ByRef] string expression); /// Evaluate a JEXL expression and return debug results as JSON. /// For CLI testing and debugging. [Throws=NimbusError] - string eval_jexl_debug(string expression); + string eval_jexl_debug([ByRef] string expression); }; interface NimbusStringHelper { diff --git a/components/nimbus/src/schema.rs b/components/nimbus/src/schema.rs index cd80626540..43f9be77cc 100644 --- a/components/nimbus/src/schema.rs +++ b/components/nimbus/src/schema.rs @@ -347,7 +347,7 @@ pub enum RandomizationUnit { UserId, } -#[derive(Default)] +#[derive(Clone, Default)] pub struct AvailableRandomizationUnits { pub user_id: Option, pub nimbus_id: Option, @@ -377,7 +377,7 @@ impl AvailableRandomizationUnits { } } - pub fn get_value<'a>(&'a self, wanted: &'a RandomizationUnit) -> Option<&'a str> { + pub fn get_value(&self, wanted: &RandomizationUnit) -> Option<&str> { match wanted { RandomizationUnit::NimbusId => self.nimbus_id.as_deref(), RandomizationUnit::UserId => self.user_id.as_deref(), diff --git a/components/nimbus/src/stateful/dbcache.rs b/components/nimbus/src/stateful/dbcache.rs index 2d91068ae4..e68d685cd4 100644 --- a/components/nimbus/src/stateful/dbcache.rs +++ b/components/nimbus/src/stateful/dbcache.rs @@ -9,13 +9,13 @@ use crate::enrollment::{ EnrolledFeature, EnrolledFeatureConfig, ExperimentEnrollment, map_features_by_feature_id, }; use crate::error::{NimbusError, Result, warn}; -use crate::evaluator::{ExperimentAvailable, is_experiment_available}; +use crate::evaluator::{CanEnrollResult, can_enroll}; use crate::stateful::enrollment::get_enrollments; use crate::stateful::firefox_labs::FirefoxLabsMetadata; use crate::stateful::gecko_prefs::GeckoPrefStore; use crate::stateful::persistence::{Database, StoreId, Writer}; use crate::targeting::NimbusTargetingHelper; -use crate::{EnrolledExperiment, Experiment}; +use crate::{AvailableRandomizationUnits, EnrolledExperiment, Experiment}; // This module manages an in-memory cache of the database, so that some // functions exposed by nimbus can return results without blocking on any @@ -219,6 +219,7 @@ impl DatabaseCache { pub fn get_available_firefox_labs_metadata( &self, + available_randomization_units: &AvailableRandomizationUnits, targeting_helper: &NimbusTargetingHelper, coenrolling_feature_ids: &[String], ) -> Result> { @@ -237,13 +238,12 @@ impl DatabaseCache { } let enrolled = data.experiments_by_slug.contains_key(&experiment.slug); + let enrollable = matches!( + can_enroll(available_randomization_units, targeting_helper, experiment,), + CanEnrollResult::Enrollable { .. } + ); - // We call is_experiment_available with is_release=true - // because being able to enroll in experiments for different channels is a bug. - // - // See-also https://bugzilla.mozilla.org/show_bug.cgi?id=1909348 - if is_experiment_available(targeting_helper, experiment, true) - == ExperimentAvailable::Available + if enrollable && (enrolled || (features_available( experiment, diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs index 05ce8f1201..f8df9442df 100644 --- a/components/nimbus/src/stateful/nimbus_client.rs +++ b/components/nimbus/src/stateful/nimbus_client.rs @@ -1003,14 +1003,20 @@ impl NimbusClient { } pub fn get_available_firefox_labs(&self) -> Result> { - let targeting_attributes = self.get_targeting_attributes(); + let mut state = self.mutable_state.lock().unwrap(); + state.update_time_to_now(Utc::now()); + let targeting_helper = NimbusTargetingHelper::with_targeting_attributes( - &targeting_attributes, + &state.targeting_attributes, self.event_store.clone(), self.gecko_prefs.clone(), ); - self.database_cache - .get_available_firefox_labs_metadata(&targeting_helper, &self.coenrolling_feature_ids) + + self.database_cache.get_available_firefox_labs_metadata( + &state.available_randomization_units, + &targeting_helper, + &self.coenrolling_feature_ids, + ) } pub fn enroll_in_firefox_lab(&self, slug: &str) -> Result { diff --git a/components/nimbus/src/targeting.rs b/components/nimbus/src/targeting.rs index 107c038e3d..3e676124f3 100644 --- a/components/nimbus/src/targeting.rs +++ b/components/nimbus/src/targeting.rs @@ -49,20 +49,20 @@ impl NimbusTargetingHelper { } } - pub fn eval_jexl(&self, expr: String) -> Result { + pub fn eval_jexl(&self, expr: &str) -> Result { cfg_if::cfg_if! { if #[cfg(feature = "stateful")] { - jexl_eval(&expr, &self.context, self.event_store.clone(), self.gecko_pref_store.clone()) + jexl_eval(expr, &self.context, self.event_store.clone(), self.gecko_pref_store.clone()) } else { - jexl_eval(&expr, &self.context) + jexl_eval(expr, &self.context) } } } #[cfg(feature = "stateful")] - pub fn eval_jexl_debug(&self, expression: String) -> Result { + pub fn eval_jexl_debug(&self, expression: &str) -> Result { let eval_result = jexl_eval_raw( - &expression, + expression, &self.context, self.event_store.clone(), self.gecko_pref_store.clone(), diff --git a/components/nimbus/src/tests/stateful/test_behavior.rs b/components/nimbus/src/tests/stateful/test_behavior.rs index b11b405c32..6abd1573d4 100644 --- a/components/nimbus/src/tests/stateful/test_behavior.rs +++ b/components/nimbus/src/tests/stateful/test_behavior.rs @@ -1459,72 +1459,76 @@ mod event_store_tests { let th = NimbusTargetingHelper::from(store); assert!( - th.eval_jexl(format!("'{event_id}'|eventSum('Minutes') >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes') >= 0")) .is_err() ); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 1) == 1"))?); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 1, 1) == 0"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes', 1) == 1"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes', 1, 1) == 0"))?); // This is one minute bucket 24h ago. We error out at zero. - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 1, 24 * 60) == 0"))?); + assert!(th.eval_jexl(&format!( + "'{event_id}'|eventSum('Minutes', 1, 24 * 60) == 0" + ))?); // This is the last 24 hours of one minute buckets. This is the same as the first 60. - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 24 * 60) == 1"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes', 24 * 60) == 1"))?); assert!( - th.eval_jexl(format!("'{event_id}'|eventSum('Years') >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventSum('Years') >= 0")) .is_err() ); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Years', 1) == 1"))?); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Years', 1, 1) == 0"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Years', 1) == 1"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Years', 1, 1) == 0"))?); assert!( - th.eval_jexl(format!("'{event_id}'|eventCountNonZero('Minutes') >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventCountNonZero('Minutes') >= 0")) .is_err() ); - assert!(th.eval_jexl(format!("'{event_id}'|eventCountNonZero('Minutes', 1) == 1"))?); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( + "'{event_id}'|eventCountNonZero('Minutes', 1) == 1" + ))?); + assert!(th.eval_jexl(&format!( "'{event_id}'|eventCountNonZero('Minutes', 1, 1) == 0" ))?); assert!( - th.eval_jexl(format!( + th.eval_jexl(&format!( "'{event_id}'|eventAveragePerInterval('Minutes') >= 0" )) .is_err() ); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerInterval('Minutes', 1) == 1" ))?); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerInterval('Minutes', 1, 1) == 0" ))?); assert!( - th.eval_jexl(format!( + th.eval_jexl(&format!( "'{event_id}'|eventAveragePerNonZeroInterval('Minutes') >= 0" )) .is_err() ); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerNonZeroInterval('Minutes', 1) >= 0" ))?); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerNonZeroInterval('Minutes', 1, 1) >= 0" ))?); // When was this event last seen? It was seen zero minutes ago. - assert!(th.eval_jexl(format!("'{event_id}'|eventLastSeen('Minutes') == 0"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventLastSeen('Minutes') == 0"))?); // Before this last minute, when was this event last seen? It was at least 60 minutes ago, if ever - assert!(th.eval_jexl(format!("'{event_id}'|eventLastSeen('Minutes', 1) > 60"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventLastSeen('Minutes', 1) > 60"))?); // LastSeen doesn't support a fourth argument. assert!( - th.eval_jexl(format!("'{event_id}'|eventLastSeen('Minutes', 1, 1) >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventLastSeen('Minutes', 1, 1) >= 0")) .is_err() ); // Q: Before 24 hours ago, when did we last see this event? // A: it was greater than 24h, but likely never. - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventLastSeen('Minutes', 24 * 60) > 24 * 60" ))?); diff --git a/components/nimbus/src/tests/stateful/test_evaluator.rs b/components/nimbus/src/tests/stateful/test_evaluator.rs index 187935ffaf..eb702a83d7 100644 --- a/components/nimbus/src/tests/stateful/test_evaluator.rs +++ b/components/nimbus/src/tests/stateful/test_evaluator.rs @@ -8,14 +8,13 @@ use std::sync::Arc; use chrono::Utc; use serde_json::json; -use crate::enrollment::NotEnrolledReason; -use crate::evaluator::targeting; +use crate::error::{NimbusError, Result}; use crate::stateful::behavior::{ EventStore, Interval, IntervalConfig, IntervalData, MultiIntervalCounter, SingleIntervalCounter, }; use crate::stateful::targeting::RecordedContext; use crate::tests::helpers::TestRecordedContext; -use crate::{AppContext, EnrollmentStatus, TargetingAttributes}; +use crate::{AppContext, NimbusTargetingHelper, TargetingAttributes}; #[test] fn test_event_sum_transform() { @@ -29,16 +28,14 @@ fn test_event_sum_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting("'app.foregrounded'|eventSum('Days', 3, 0) > 2", &th), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventSum('Days', 3, 0) > 2") + .unwrap(), ); - assert_eq!( - targeting("'app.foregrounded'|eventSum('Days', 3, 0) > 1", &th,), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventSum('Days', 3, 0) > 1") + .unwrap(), ); } @@ -54,22 +51,14 @@ fn test_event_count_non_zero_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting( - "'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 2", - &th, - ), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 2") + .unwrap(), ); - assert_eq!( - targeting( - "'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 1", - &th, - ), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 1") + .unwrap() ); } @@ -85,22 +74,14 @@ fn test_event_average_per_interval_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 2", - &th, - ), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 2") + .unwrap(), ); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 1.14", - &th, - ), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 1.14") + .unwrap() ); } @@ -116,159 +97,91 @@ fn test_event_average_per_non_zero_interval_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 1", - &th, - ), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 1") + .unwrap(), ); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 2.25", - &th, - ), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 2.25") + .unwrap() ); } #[test] fn test_event_transform_sum_cnz_avg_avgnz_parameters() { - let th = Default::default(); - - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Days') > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform Sum requires 2-3 parameters" - .to_string() - }) - ); - assert_eq!( - targeting( - "1|eventSum('Days', 3, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum(1, 3, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Day', 3, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Days', 'test', 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform Sum requires a positive number as the second parameter" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Days', 3, 'test') > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform Sum requires a positive number as the third parameter" - .to_string() - }) - ); + let th = NimbusTargetingHelper::default(); + + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Days') > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform Sum requires 2-3 parameters" + )); + assert!(matches!( + th.eval_jexl("1|eventSum('Days', 3, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum(1, 3, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Day', 3, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Days', 'test', 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform Sum requires a positive number as the second parameter" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Days', 3, 'test') > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform Sum requires a positive number as the third parameter" + )); } #[test] fn test_event_transform_last_seen_parameters() { - let th = Default::default(); - - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen() > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen('Days', 0, 10) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" - .to_string() - }) - ); - assert_eq!( - targeting( - "1|eventLastSeen('Days', 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen(1, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen('Day', 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen('Days', 'test') > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform LastSeen requires a positive number as the second parameter" - .to_string() - }) - ); - - assert_eq!( - targeting("'app_cycle.foreground1'|eventLastSeen('Days', 2) > 1", &th), - None + let th = NimbusTargetingHelper::default(); + + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen() > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen('Days', 0, 10) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" + )); + assert!(matches!( + th.eval_jexl("1|eventLastSeen('Days', 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen(1, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen('Day', 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen('Days', 'test') > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform LastSeen requires a positive number as the second parameter" + )); + + assert!( + th.eval_jexl("'app_cycle.foreground1'|eventLastSeen('Days', 2) > 1") + .unwrap() ); } @@ -277,7 +190,7 @@ fn test_targeting_active_experiments_equivalency() { // Here's our valid jexl statement let expression_statement = "'test' in active_experiments"; // A matching context that includes the appropriate specific context - let mut targeting_attributes: TargetingAttributes = AppContext { + let mut targeting_attributes = TargetingAttributes::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -293,37 +206,36 @@ fn test_targeting_active_experiments_equivalency() { debug_tag: None, custom_targeting_attributes: None, ..Default::default() - } - .into(); + }); + let mut set = HashSet::::new(); set.insert("test".into()); targeting_attributes.active_experiments = set; // The targeting should pass! - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into()), - None + assert!( + NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap() ); // We set active_experiment treatment to something not expected and try again let mut set = HashSet::::new(); set.insert("test1".into()); targeting_attributes.active_experiments = set; - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); // We set active_experiments to None and try again let set = HashSet::::new(); targeting_attributes.active_experiments = set; - assert_eq!( - targeting(expression_statement, &targeting_attributes.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); } @@ -332,7 +244,7 @@ fn test_targeting_active_experiments_exists() { // Here's our valid jexl statement let expression_statement = "'test' in active_experiments"; // A matching context that includes the appropriate specific context - let mut targeting_attributes: TargetingAttributes = AppContext { + let mut targeting_attributes = TargetingAttributes::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -348,26 +260,26 @@ fn test_targeting_active_experiments_exists() { debug_tag: None, custom_targeting_attributes: None, ..Default::default() - } - .into(); + }); + let mut set = HashSet::::new(); set.insert("test".into()); targeting_attributes.active_experiments = set; // The targeting should pass! - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into()), - None + assert!( + NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap() ); // We set active_experiment treatment to something not expected and try again let set = HashSet::::new(); targeting_attributes.active_experiments = set; - assert_eq!( - targeting(expression_statement, &targeting_attributes.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); } @@ -376,7 +288,7 @@ fn test_targeting_is_already_enrolled() { // Here's our valid jexl statement let expression_statement = "is_already_enrolled"; // A matching context that includes the appropriate specific context - let ac = AppContext { + let mut targeting_attributes = TargetingAttributes::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -384,110 +296,94 @@ fn test_targeting_is_already_enrolled() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - let mut targeting_attributes = TargetingAttributes::from(ac); + }); targeting_attributes.is_already_enrolled = true; // The targeting should pass! - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into(),), - None + assert!( + NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); // We make the is_already_enrolled false and try again targeting_attributes.is_already_enrolled = false; - assert_eq!( - targeting(expression_statement, &targeting_attributes.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); } #[test] fn test_bucket_sample() { - let cases = [ - ("1.1", "1000", "1000", None), - ( - "0", - "1.1", - "1000", - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted, - }), - ), - ( - "0", - "0", - "1000.1", - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted, - }), - ), + fn eval_bucketsample_expr(start: &str, count: &str, total: &str) -> Result { + let expr = format!("0|bucketSample({start}, {count}, {total})"); + NimbusTargetingHelper::from(AppContext { + app_name: "nimbus_test".into(), + app_id: "nimbus-test".into(), + channel: "test".into(), + ..Default::default() + }) + .eval_jexl(&expr) + } + + let ok_cases = [ + ("1.1", "1000", "1000", true), + ("0", "1.1", "1000", false), + ("0", "0", "1000.1", false), + ]; + + for (start, count, total, expected) in ok_cases { + assert_eq!( + eval_bucketsample_expr(start, count, total).unwrap(), + expected + ); + } + + let err_cases = [ ( "4294967296", "1", "4294967297", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: start is out of range".into(), - }), + "Custom error: start is out of range", ), ( "0", "4294967296", "4294967296", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: count is out of range".into(), - }), + "Custom error: count is out of range", ), ( "0", "0", "4294967296", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: total is out of range".into(), - }), + "Custom error: total is out of range", ), ( r#""hello""#, "0", "1000", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: start is not a number".into(), - }), + "Custom error: start is not a number", ), ( "0", r#""hello""#, "1000", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: count is not a number".into(), - }), + "Custom error: count is not a number", ), ( "0", "1000", r#""hello""#, - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: total is not a number".into(), - }), + "Custom error: total is not a number", ), ]; - for (start, count, total, expected) in cases { - let expr = format!("0|bucketSample({start}, {count}, {total})"); - println!("{}", expr); - let targeting_attributes: TargetingAttributes = AppContext { - app_name: "nimbus_test".into(), - app_id: "nimbus-test".into(), - channel: "test".into(), - ..Default::default() - } - .into(); - - let result = targeting(&expr, &targeting_attributes.clone().into()); - - assert_eq!(result, expected); + for (start, count, total, expected) in err_cases { + assert!(matches!( + eval_bucketsample_expr(start, count, total).unwrap_err(), + NimbusError::EvaluationError(e) if e == expected)); } } diff --git a/components/nimbus/src/tests/stateful/test_nimbus.rs b/components/nimbus/src/tests/stateful/test_nimbus.rs index e865b33a6f..5a36fd33e2 100644 --- a/components/nimbus/src/tests/stateful/test_nimbus.rs +++ b/components/nimbus/src/tests/stateful/test_nimbus.rs @@ -1031,7 +1031,7 @@ fn test_active_enrollment_in_targeting() -> Result<()> { assert_eq!(active_experiments.len(), 1); let targeting_helper = client.create_targeting_helper(None)?; - assert!(targeting_helper.eval_jexl("'test-1' in active_experiments".to_string())?); + assert!(targeting_helper.eval_jexl("'test-1' in active_experiments")?); // Apply experiment that targets the above experiment is in enrollments let exp = get_targeted_experiment("test-2", "'test-1' in enrollments"); @@ -1042,12 +1042,12 @@ fn test_active_enrollment_in_targeting() -> Result<()> { assert_eq!(active_experiments.len(), 1); let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl("'test-1' in active_experiments".to_string())?); - assert!(targeting_helper.eval_jexl("'test-2' in active_experiments".to_string())?); - assert!(targeting_helper.eval_jexl("'test-1' in enrollments".to_string())?); - assert!(targeting_helper.eval_jexl("'test-2' in enrollments".to_string())?); - assert!(targeting_helper.eval_jexl("enrollments_map['test-1'] == 'treatment'".to_string())?); - assert!(targeting_helper.eval_jexl("enrollments_map['test-2'] == 'control'".to_string())?); + assert!(!targeting_helper.eval_jexl("'test-1' in active_experiments")?); + assert!(targeting_helper.eval_jexl("'test-2' in active_experiments")?); + assert!(targeting_helper.eval_jexl("'test-1' in enrollments")?); + assert!(targeting_helper.eval_jexl("'test-2' in enrollments")?); + assert!(targeting_helper.eval_jexl("enrollments_map['test-1'] == 'treatment'")?); + assert!(targeting_helper.eval_jexl("enrollments_map['test-2'] == 'control'")?); Ok(()) } @@ -1104,16 +1104,16 @@ fn test_previous_enrollments_in_targeting() -> Result<()> { assert_eq!(active_experiments.len(), 5); let targeting_helper = client.create_targeting_helper(None)?; - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_3))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_4))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_5))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_2))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_3))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_4))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_5))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_3))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_4))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_5))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_3))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_4))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_5))?); // Apply empty first experiment, disqualifying second experiment, and decreased bucket rollout let exp_2 = get_targeted_experiment_with_feature(slug_2, "false", "feature-2"); @@ -1184,16 +1184,16 @@ fn test_previous_enrollments_in_targeting() -> Result<()> { )); let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_3))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_4))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_5))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_2))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_3))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_4))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_5))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_3))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_4))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_5))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_3))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_4))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_5))?); Ok(()) } @@ -1234,20 +1234,20 @@ fn test_opt_out_multiple_experiments_same_feature_does_not_re_enroll() -> Result client.apply_pending_experiments()?; let targeting_helper = client.create_targeting_helper(None)?; - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); client.opt_out(slug_1.into())?; let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); client.opt_out(slug_2.into())?; let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); Ok(()) } @@ -2597,6 +2597,17 @@ fn test_firefox_labs_enroll_unenroll() -> Result<()> { .patch(json!({ "firefoxLabsDescription": null })), get_firefox_lab_with_feature("lab-different-channel", "lab-feature-7") .patch(json!({ "channel": "mystery" })), + get_firefox_lab_with_feature("false-targeting", "lab-feature-8") + .patch(json!({ "targeting": "false" })), + get_firefox_lab_with_feature("false-bucketing", "lab-feature-9").patch(json!({ + "bucketConfig": { + "randomizationUnit": "nimbus_id", + "start": 0, + "count": 0, + "total": 10000, + "namespace": "firefox-labs-test", + } + })), ])?; assert_eq!( @@ -2639,6 +2650,18 @@ fn test_firefox_labs_enroll_unenroll() -> Result<()> { branch: Some("control".into()), ..Default::default() }, + EnrollmentStatusExtraDef { + slug: Some("false-bucketing".into()), + status: Some("NotEnrolled".into()), + reason: Some("FirefoxLabs".into()), + ..Default::default() + }, + EnrollmentStatusExtraDef { + slug: Some("false-targeting".into()), + status: Some("NotEnrolled".into()), + reason: Some("FirefoxLabs".into()), + ..Default::default() + }, EnrollmentStatusExtraDef { slug: Some("lab".into()), status: Some("NotEnrolled".into()), diff --git a/components/nimbus/src/tests/stateful/test_targeting.rs b/components/nimbus/src/tests/stateful/test_targeting.rs index 14db35aef6..08a3db2ef7 100644 --- a/components/nimbus/src/tests/stateful/test_targeting.rs +++ b/components/nimbus/src/tests/stateful/test_targeting.rs @@ -83,9 +83,7 @@ fn test_eval_jexl_debug_success() { "locale": "en-US", })); - let result = helper - .eval_jexl_debug("locale == 'en-US'".to_string()) - .unwrap(); + let result = helper.eval_jexl_debug("locale == 'en-US'").unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); assert_eq!(parsed["success"], true); @@ -98,7 +96,7 @@ fn test_eval_jexl_debug_error() { let helper = create_helper(json!({})); - let result = helper.eval_jexl_debug("invalid {{".to_string()).unwrap(); + let result = helper.eval_jexl_debug("invalid {{").unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); assert_eq!(parsed["success"], false); @@ -111,7 +109,7 @@ fn test_eval_jexl_debug_json_structure() { let helper = create_helper(json!({"test": true})); - let result = helper.eval_jexl_debug("test".to_string()).unwrap(); + let result = helper.eval_jexl_debug("test").unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); @@ -128,7 +126,7 @@ fn test_eval_jexl_debug_returns_pretty_json() { let helper = create_helper(json!({"locale": "en-US"})); - let result = helper.eval_jexl_debug("locale".to_string()).unwrap(); + let result = helper.eval_jexl_debug("locale").unwrap(); // Pretty JSON should have newlines assert!(result.contains('\n')); @@ -149,7 +147,7 @@ fn test_eval_jexl_debug_with_version_compare() { let helper = create_helper(serde_json::to_value(&targeting_attributes).unwrap()); let result = helper - .eval_jexl_debug("app_version|versionCompare('114.0') > 0".to_string()) + .eval_jexl_debug("app_version|versionCompare('114.0') > 0") .unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); diff --git a/components/nimbus/src/tests/test_evaluator.rs b/components/nimbus/src/tests/test_evaluator.rs index 58fb18b8a6..1bbb6d5548 100644 --- a/components/nimbus/src/tests/test_evaluator.rs +++ b/components/nimbus/src/tests/test_evaluator.rs @@ -7,10 +7,10 @@ use serde_json::{Map, Value, json}; use crate::enrollment::{EnrolledReason, EnrollmentStatus, NotEnrolledReason}; -use crate::evaluator::{ExperimentAvailable, choose_branch, is_experiment_available, targeting}; +use crate::evaluator::{ExperimentAvailable, choose_branch, is_experiment_available}; use crate::{ - AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, RandomizationUnit, - Result, TargetingAttributes, evaluate_enrollment, + AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, NimbusError, + NimbusTargetingHelper, RandomizationUnit, Result, TargetingAttributes, evaluate_enrollment, }; pub fn ta_with_locale(locale: String) -> TargetingAttributes { @@ -32,29 +32,19 @@ pub fn ta_with_locale(locale: String) -> TargetingAttributes { } #[test] -fn test_locale_substring() -> Result<()> { +fn test_locale_substring() { let expression_statement = "'en' in locale || 'de' in locale"; - let ta = ta_with_locale("de-US".to_string()); + let targeting_helper: NimbusTargetingHelper = ta_with_locale("de-US".to_string()).into(); - assert_eq!(targeting(expression_statement, &ta.into()), None); - Ok(()) + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); } #[test] -fn test_locale_substring_fails() -> Result<()> { +fn test_locale_substring_fails() { let expression_statement = "'en' in locale || 'de' in locale"; - let ta = ta_with_locale("cz-US".to_string()); - let enrollment_status = targeting(expression_statement, &ta.into()).unwrap(); - if let EnrollmentStatus::NotEnrolled { reason } = enrollment_status { - if let NotEnrolledReason::NotTargeted = reason { - // OK - } else { - panic!("Expected to fail on NotTargeted reason, got: {:?}", reason) - } - } else { - panic! {"Expected to fail targeting with NotEnrolled, got: {:?}", enrollment_status} - } - Ok(()) + let targeting_helper: NimbusTargetingHelper = ta_with_locale("cz-US".to_string()).into(); + + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()); } #[test] @@ -79,35 +69,25 @@ fn test_language_region_from_locale() { #[test] fn test_geo_targeting_one_locale() -> Result<()> { let expression_statement = "language in ['ro']"; - let ta = ta_with_locale("ro".to_string()); + let targeting_helper: NimbusTargetingHelper = ta_with_locale("ro".to_string()).into(); - assert_eq!(targeting(expression_statement, &ta.into()), None); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); Ok(()) } #[test] -fn test_geo_targeting_multiple_locales() -> Result<()> { +fn test_geo_targeting_multiple_locales() { let expression_statement = "language in ['en', 'ro']"; - let ta = ta_with_locale("ro".to_string()); - assert_eq!(targeting(expression_statement, &ta.into()), None); - Ok(()) + let targeting_helper: NimbusTargetingHelper = ta_with_locale("ro".to_string()).into(); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); } #[test] -fn test_geo_targeting_fails_properly() -> Result<()> { +fn test_geo_targeting_fails_properly() { let expression_statement = "language in ['en', 'ro']"; - let ta = ta_with_locale("ar".to_string()); - let enrollment_status = targeting(expression_statement, &ta.into()).unwrap(); - if let EnrollmentStatus::NotEnrolled { reason } = enrollment_status { - if let NotEnrolledReason::NotTargeted = reason { - // OK - } else { - panic!("Expected to fail on NotTargeted reason, got: {:?}", reason) - } - } else { - panic! {"Expected to fail targeting with NotEnrolled, got: {:?}", enrollment_status} - } - Ok(()) + let targeting_helper: NimbusTargetingHelper = ta_with_locale("ar".to_string()).into(); + + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()); } #[cfg(feature = "stateful")] @@ -115,11 +95,11 @@ fn test_geo_targeting_fails_properly() -> Result<()> { fn test_minimum_version_targeting_passes() -> Result<()> { // Here's our valid jexl statement let expression_statement = "app_version|versionCompare('96.!') >= 0"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("97pre.1.0-beta.1".into()), ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); Ok(()) } @@ -128,76 +108,52 @@ fn test_minimum_version_targeting_passes() -> Result<()> { fn test_minimum_version_targeting_fails() -> Result<()> { // Here's our valid jexl statement let expression_statement = "app_version|versionCompare('96+.0') >= 0"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("96.1".into()), ..Default::default() - }; - assert_eq!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - ); + }); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap(),); Ok(()) } #[cfg(feature = "stateful")] #[test] -fn test_targeting_specific_version() -> Result<()> { +fn test_targeting_specific_version() { // Here's our valid jexl statement that targets **only** 96 versions let expression_statement = "(app_version|versionCompare('96.!') >= 0) && (app_version|versionCompare('97.!') < 0)"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("96.1".into()), ..Default::default() - }; + }); // OK 96.1 is a 96 version - assert_eq!(targeting(expression_statement, &ctx.into()), None); - let ctx = AppContext { + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("97.1".into()), ..Default::default() - }; + }); // Not targeted, version is 97 - assert_eq!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - ); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap(),); - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("95.1".into()), ..Default::default() - }; + }); // Not targeted, version is 95 - assert_eq!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - ); - - Ok(()) + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap(),); } #[test] -fn test_targeting_invalid_transform() -> Result<()> { +fn test_targeting_invalid_transform() { let expression_statement = "app_version|invalid_transform('96+.0')"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("96.1".into()), ..Default::default() - }; - let err = targeting(expression_statement, &ctx.into()); - if let Some(e) = err { - if let EnrollmentStatus::Error { reason: _ } = e { - // OK - } else { - panic!("Should have returned an error since the transform doesn't exist") - } - } else { - panic!("Should not have been targeted") - } - Ok(()) + }); + assert!(matches!( + targeting_helper.eval_jexl(expression_statement).unwrap_err(), + NimbusError::EvaluationError(e) if e == "Unknown transform: invalid_transform", + )); } #[cfg(feature = "stateful")] @@ -208,7 +164,7 @@ fn test_targeting() { "app_id == '1010' && (app_version|versionCompare('4.0') >= 0 || app_build == \"1234\")"; // A matching context testing the logical AND + OR of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -216,11 +172,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A matching context testing the logical OR of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -228,11 +184,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A matching context testing the other branch of the logical OR - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -240,11 +196,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A non-matching context testing the logical AND of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "not_nimbus_test".to_string(), app_id: "org.example.app".to_string(), channel: "test".to_string(), @@ -252,16 +208,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert!(matches!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - )); + }); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()); // A non-matching context testing the logical OR of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "not_nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -269,13 +220,8 @@ fn test_targeting() { app_build: Some("12345".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert!(matches!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - )); + }); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()) } #[test] @@ -287,7 +233,7 @@ fn test_targeting_custom_targeting_attributes() { custom_targeting_attributes.insert("is_first_run".into(), json!(true)); custom_targeting_attributes.insert("ios_version".into(), json!("8.8")); // A matching context that includes the appropriate specific context - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -295,11 +241,11 @@ fn test_targeting_custom_targeting_attributes() { app_build: Some("1234".to_string()), custom_targeting_attributes: Some(custom_targeting_attributes), ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A matching context without the specific context - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -307,11 +253,11 @@ fn test_targeting_custom_targeting_attributes() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; + }); // We haven't defined `is_first_run` here, so this should error out, i.e. return an error. assert!(matches!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::Error { .. }) + targeting_helper.eval_jexl(expression_statement).unwrap_err(), + NimbusError::EvaluationError(e) if e == "Identifier 'is_first_run' is undefined", )); } @@ -319,23 +265,26 @@ fn test_targeting_custom_targeting_attributes() { fn test_invalid_expression() { // This expression doesn't return a bool let expression_statement = "2.0"; + let targeting_helper = NimbusTargetingHelper::default(); - assert_eq!( - targeting(expression_statement, &Default::default()), - Some(EnrollmentStatus::Error { - reason: "Invalid Expression - didn't evaluate to a bool".to_string() - }) - ) + assert!(matches!( + targeting_helper + .eval_jexl(expression_statement) + .unwrap_err(), + NimbusError::InvalidExpression + )); } #[test] fn test_evaluation_error() { // This is an invalid JEXL statement let expression_statement = "This is not a valid JEXL expression"; + let targeting_helper = NimbusTargetingHelper::default(); assert!(matches!( - targeting(expression_statement, &Default::default()), - Some(EnrollmentStatus::Error { reason }) if reason.starts_with("EvaluationError:"))) + targeting_helper.eval_jexl(expression_statement).inspect_err(|e| eprintln!("{:?}", e)).unwrap_err(), + NimbusError::EvaluationError(e) if e.starts_with("Parsing error: Unrecognized token `is`"), + )); } #[test] @@ -574,12 +523,12 @@ fn test_wrong_randomization_units() { // Application context for matching the above experiment. If any of the `app_name`, `app_id`, // or `channel` doesn't match the experiment, then the client won't be enrolled. - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "NimbusTest".to_string(), app_id: "org.example.app".to_string(), channel: "nightly".to_string(), ..Default::default() - }; + }); // We won't be enrolled in the experiment because we don't have the right randomization units since the // experiment is requesting the `UserId` and the `Default::default()` here will just have the @@ -587,7 +536,7 @@ fn test_wrong_randomization_units() { let enrollment = evaluate_enrollment( &AvailableRandomizationUnits::with_nimbus_id(&uuid::Uuid::new_v4()), &experiment, - &ctx.clone().into(), + &targeting_helper, ) .unwrap(); // The status should be `Error` @@ -595,8 +544,12 @@ fn test_wrong_randomization_units() { // Fits because of the user_id. let available_randomization_units = AvailableRandomizationUnits::with_user_id("bobo"); - let enrollment = - evaluate_enrollment(&available_randomization_units, &experiment, &ctx.into()).unwrap(); + let enrollment = evaluate_enrollment( + &available_randomization_units, + &experiment, + &targeting_helper, + ) + .unwrap(); assert!(matches!( enrollment.status, EnrollmentStatus::Enrolled { @@ -726,16 +679,16 @@ fn test_enrollment_bucketing() { // Tested against the desktop implementation let id = uuid::Uuid::parse_str("299eed1e-be6d-457d-9e53-da7b1a03f10d").unwrap(); // Application context for matching exp3 - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_id: "org.example.app".to_string(), channel: "nightly".to_string(), ..Default::default() - }; + }); let enrollment = evaluate_enrollment( &available_randomization_units.apply_nimbus_id(&id), &experiment, - &ctx.into(), + &targeting_helper, ) .unwrap(); assert!(matches!( diff --git a/components/nimbus/tests/test_message_helpers.rs b/components/nimbus/tests/test_message_helpers.rs index 9151e36762..9d136adb97 100644 --- a/components/nimbus/tests/test_message_helpers.rs +++ b/components/nimbus/tests/test_message_helpers.rs @@ -22,18 +22,18 @@ fn test_jexl_expression() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; // We get a boolean back from a string! - assert!(helper.eval_jexl("app_name == 'fenix'".to_string())?); + assert!(helper.eval_jexl("app_name == 'fenix'")?); // We get true and false back from two similar JEXL expressions! // I think we can convince ourselves that JEXL is being evaluated against the // AppContext. - assert!(!helper.eval_jexl("app_name == 'xinef'".to_string())?); + assert!(!helper.eval_jexl("app_name == 'xinef'")?); // The expression contains a variable not declared (snek_case Good, camelCase Bad) - assert!(helper.eval_jexl("appName == 'fenix'".to_string()).is_err()); + assert!(helper.eval_jexl("appName == 'fenix'").is_err()); // This validates that helpers created from the create_targeting_helper have the event_store present in jexl operations - assert!(helper.eval_jexl("'test'|eventSum('Days', 1, 0) == 1".to_string())?); + assert!(helper.eval_jexl("'test'|eventSum('Days', 1, 0) == 1")?); let helper = nimbus.create_targeting_helper( json!( @@ -46,12 +46,12 @@ fn test_jexl_expression() -> Result<()> { // Check the versionCompare function, just to prove to ourselves that it's the same JEXL evaluator. assert!(helper.eval_jexl( - "(version|versionCompare('95.!') >= 0) && (version|versionCompare('96.!') < 0)".to_string(), + "(version|versionCompare('95.!') >= 0) && (version|versionCompare('96.!') < 0)", )?); // Check the versionCompare function, just to prove to ourselves that it's the same JEXL evaluator. assert!(!helper.eval_jexl( - "(version|versionCompare('96.!') >= 0) && (version|versionCompare('97.!') < 0)".to_string(), + "(version|versionCompare('96.!') >= 0) && (version|versionCompare('97.!') < 0)", )?); Ok(()) @@ -64,11 +64,11 @@ fn test_derived_targeting_attributes_available() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; - assert!(helper.eval_jexl("locale == 'en-GB'".to_string())?); + assert!(helper.eval_jexl("locale == 'en-GB'")?); - assert!(helper.eval_jexl("language == 'en'".to_string())?); + assert!(helper.eval_jexl("language == 'en'")?); - assert!(helper.eval_jexl("region == 'GB'".to_string())?); + assert!(helper.eval_jexl("region == 'GB'")?); Ok(()) } @@ -82,7 +82,7 @@ fn test_derived_targeting_attributes_none() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; - assert!(!helper.eval_jexl("(locale||'NONE') == 'en'".to_string())?); + assert!(!helper.eval_jexl("(locale||'NONE') == 'en'")?); // assert!(helper.eval_jexl( // "language == null".to_string() @@ -101,17 +101,17 @@ fn test_jexl_expression_with_targeting_attributes() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; - assert!(helper.eval_jexl("days_since_install == 0".to_string())?); + assert!(helper.eval_jexl("days_since_install == 0")?); - assert!(helper.eval_jexl("days_since_update == 0".to_string())?); + assert!(helper.eval_jexl("days_since_update == 0")?); nimbus.set_install_time(Utc::now() - Duration::days(10)); nimbus.set_update_time(Utc::now() - Duration::days(5)); let helper = nimbus.create_targeting_helper(None)?; - assert!(helper.eval_jexl("days_since_install == 10".to_string())?); + assert!(helper.eval_jexl("days_since_install == 10")?); - assert!(helper.eval_jexl("days_since_update == 5".to_string())?); + assert!(helper.eval_jexl("days_since_update == 5")?); Ok(()) } diff --git a/components/nimbus/tests/test_restart.rs b/components/nimbus/tests/test_restart.rs index c9e36f86e0..cdb588feee 100644 --- a/components/nimbus/tests/test_restart.rs +++ b/components/nimbus/tests/test_restart.rs @@ -211,9 +211,9 @@ fn test_targeting_attributes_active_experiments() -> Result<()> { assert_eq!(ta.active_experiments, expected); let eval = client.create_targeting_helper(None)?; - assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments".to_string())?); - assert!(eval.eval_jexl("'experiment_target_false' in active_experiments".to_string())?); - assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments".to_string())?); + assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments")?); + assert!(eval.eval_jexl("'experiment_target_false' in active_experiments")?); + assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments")?); drop(client); @@ -224,9 +224,9 @@ fn test_targeting_attributes_active_experiments() -> Result<()> { assert_eq!(ta.active_experiments, expected); let eval = client.create_targeting_helper(None)?; - assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments".to_string())?); - assert!(eval.eval_jexl("'experiment_target_false' in active_experiments".to_string())?); - assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments".to_string())?); + assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments")?); + assert!(eval.eval_jexl("'experiment_target_false' in active_experiments")?); + assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments")?); drop(client); @@ -237,9 +237,9 @@ fn test_targeting_attributes_active_experiments() -> Result<()> { assert_eq!(ta.active_experiments, expected); let eval = client.create_targeting_helper(None)?; - assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments".to_string())?); - assert!(eval.eval_jexl("'experiment_target_false' in active_experiments".to_string())?); - assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments".to_string())?); + assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments")?); + assert!(eval.eval_jexl("'experiment_target_false' in active_experiments")?); + assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments")?); Ok(()) }