Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

add published_date field to experiment schema #5936

Merged
merged 1 commit into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions components/nimbus/src/enrollment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,7 @@ impl<'a> EnrollmentsEvolver<'a> {

// Step 3. Evolve the remaining enrollments with the previous and
// next data.
let next_experiments = sort_experiments_by_published_date(next_experiments);
for next_experiment in next_experiments {
let slug = &next_experiment.slug;

Expand Down Expand Up @@ -946,6 +947,12 @@ where
.collect()
}

pub(crate) fn sort_experiments_by_published_date(experiments: &[Experiment]) -> Vec<&Experiment> {
let mut experiments: Vec<_> = experiments.iter().collect();
experiments.sort_by(|a, b| a.published_date.cmp(&b.published_date));
experiments
}

/// Take a list of enrollments and a map of experiments, and generate mapping of `feature_id` to
/// `EnrolledFeatureConfig` structs.
fn map_features(
Expand Down
14 changes: 14 additions & 0 deletions components/nimbus/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub struct Experiment {
pub reference_branch: Option<String>,
#[serde(default)]
pub is_rollout: bool,
pub published_date: Option<chrono::DateTime<chrono::Utc>>,
// N.B. records in RemoteSettings will have `id` and `filter_expression` fields,
// but we ignore them because they're for internal use by RemoteSettings.
}
Expand Down Expand Up @@ -76,6 +77,19 @@ impl Experiment {

feature_ids.into_iter().collect()
}

#[cfg(test)]
pub(crate) fn patch(&self, patch: Value) -> Self {
let mut experiment = serde_json::to_value(self).unwrap();
if let (Some(e), Some(w)) = (experiment.as_object(), patch.as_object()) {
let mut e = e.clone();
for (key, value) in w {
e.insert(key.clone(), value.clone());
}
experiment = serde_json::to_value(e).unwrap();
}
serde_json::from_value(experiment).unwrap()
}
}

impl ExperimentMetadata for Experiment {
Expand Down
52 changes: 52 additions & 0 deletions components/nimbus/src/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,3 +552,55 @@ pub fn get_targeted_experiment(slug: &str, targeting: &str) -> serde_json::Value
"last_modified": 1_602_197_324_372i64,
})
}

pub(crate) fn get_experiment_with_published_date(
slug: &str,
published_date: Option<String>,
) -> Experiment {
serde_json::from_value(json!({
"schemaVersion": "1.0.0",
"slug": slug,
"endDate": null,
"branches":[
{
"slug": "control",
"ratio": 1,
"feature": {
"featureId": "about_welcome",
"enabled": true,
}
},
{
"slug": "treatment",
"ratio": 1,
"feature": {
"featureId": "about_welcome",
"enabled": false,
}
},
],
"featureIds": ["about_welcome"],
"channel": "nightly",
"probeSets":[],
"startDate":null,
"appName":"fenix",
"appId":"org.mozilla.fenix",
"bucketConfig":{
// Also enroll everyone.
"count":10_000,
"start":0,
"total":10_000,
"namespace":"secure-silver",
"randomizationUnit":"nimbus_id"
},
"userFacingName":"2nd test experiment",
"referenceBranch":"control",
"isEnrollmentPaused":false,
"proposedEnrollment":7,
"userFacingDescription":"2nd test experiment.",
"id":"secure-silver",
"last_modified":1_602_197_324_372i64,
"publishedDate": published_date
}))
.unwrap()
}
57 changes: 56 additions & 1 deletion components/nimbus/src/tests/test_enrollment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

// Testing enrollment.rs

use crate::tests::helpers::get_bucketed_rollout;
use crate::tests::helpers::{get_bucketed_rollout, get_experiment_with_published_date};
use crate::{
defaults::Defaults,
enrollment::*,
Expand Down Expand Up @@ -3136,3 +3136,58 @@ fn test_populate_feature_maps() -> Result<()> {

Ok(())
}

#[test]
fn test_sort_experiments_by_published_date() -> Result<()> {
let slug_1 = "slug-1";
let slug_2 = "slug-2";
let slug_3 = "slug-3";
let slug_4 = "slug-4";
let experiments = vec![
get_experiment_with_published_date(slug_1, None),
get_experiment_with_published_date(slug_2, Some("2023-11-21T18:00:00Z".into())),
get_experiment_with_published_date(slug_3, None),
get_experiment_with_published_date(slug_4, Some("2023-11-21T15:00:00Z".into())),
];
let result = sort_experiments_by_published_date(&experiments);

assert_eq!(slug_1, result[0].slug);
assert_eq!(slug_3, result[1].slug);
assert_eq!(slug_4, result[2].slug);
assert_eq!(slug_2, result[3].slug);
Ok(())
}

#[test]
fn test_evolve_enrollments_ordering() -> Result<()> {
let _ = env_logger::try_init();
let (_, app_ctx, aru) = local_ctx();
let th = app_ctx.into();
let ids = HashSet::new();
let evolver = EnrollmentsEvolver::new(&aru, &th, &ids);

let exp1 = get_single_feature_experiment("slug-1", "colliding-feature", json!({"x": 1 }))
.patch(json!({"publishedDate": "2023-11-21T18:00:00Z"}));
let exp2 = get_single_feature_experiment("slug-2", "colliding-feature", json!({"x": 2 }))
.patch(json!({"publishedDate": "2023-11-21T15:00:00Z"}));

let all_experiments = [exp1, exp2];
let no_experiments: [Experiment; 0] = [];

let (enrollments, _) =
evolver.evolve_enrollment_recipes(true, &no_experiments, &all_experiments, &[])?;

let observed = map_features_by_feature_id(&enrollments, &all_experiments, &ids);
let expected = HashMap::from([(
"colliding-feature".to_string(),
EnrolledFeatureConfig::new(
"colliding-feature",
json!({"x": 2 }),
"slug-2",
Some("control"),
),
)]);
assert_eq!(observed, expected);

Ok(())
}