From 4be0c221c19e2c8d097f44ca5f7d40c65c63d561 Mon Sep 17 00:00:00 2001 From: Jared Lockhart <119884+jaredlockhart@users.noreply.github.com> Date: Tue, 21 Jun 2022 17:01:14 -0400 Subject: [PATCH] fix #7440 feat(nimbus): migrate sticky targeting configs Because * Now that the sticky targeting clause is moved right into the NimbusExperiment targeting method * There's no need for the sticky clause to be included in individual targeting configs This commit * Removes the sticky clauses from all desktop and mobile targeting configs * Removes the now redundant 'pip_never_used_sticky' targeting config and migrates experiments that used it to 'pip_never_used' * Migrates all existing experiments that use a previously sticky targeting config to have is_sticky=True --- .../migrations/0214_migrate_sticky.py | 55 ++++ .../experiments/tests/test_migrations.py | 200 +++++++++---- .../experiments/tests/test_models.py | 9 +- app/experimenter/targeting/constants.py | 56 ++-- .../integration/nimbus/app_contexts.json | 265 ++++++++---------- .../nimbus/test_mobile_integration.py | 48 ++-- 6 files changed, 367 insertions(+), 266 deletions(-) create mode 100644 app/experimenter/experiments/migrations/0214_migrate_sticky.py diff --git a/app/experimenter/experiments/migrations/0214_migrate_sticky.py b/app/experimenter/experiments/migrations/0214_migrate_sticky.py new file mode 100644 index 0000000000..f8f8488348 --- /dev/null +++ b/app/experimenter/experiments/migrations/0214_migrate_sticky.py @@ -0,0 +1,55 @@ +# Generated by Django 3.2.13 on 2022-06-21 20:01 + +from django.db import migrations + + +def migrate_sticky_experiments(apps, schema_editor): + NimbusExperiment = apps.get_model("experiments", "NimbusExperiment") + + NimbusExperiment.objects.filter( + targeting_config_slug__in=[ + "first_run", + "first_run_chrome", + "first_run_win1903", + "not_tcp_study_first_run", + "windows_userchoice_first_run", + "infrequent_user_uris", + "infrequent_user_need_pin", + "infrequent_user_need_default", + "infrequent_user_need_default_has_pin", + "infrequent_user_has_default_need_pin", + "infrequent_windows_user_need_pin", + "infrequent_win_user_uris", + "infrequent_user_5_bookmarks", + "casual_user_uris", + "casual_user_need_pin", + "casual_user_need_default", + "casual_user_need_default_has_pin", + "casual_user_has_default_need_pin", + "regular_user_uris", + "regular_user_need_pin", + "regular_user_need_default", + "regular_user_need_default_has_pin", + "regular_user_has_default_need_pin", + "regular_user_uses_fxa", + "core_user_uris", + "pip_never_used_sticky", + "mobile_new_users", + "mobile_recently_updated_users", + ] + ).update(is_sticky=True) + + NimbusExperiment.objects.filter(targeting_config_slug="pip_never_used_sticky").update( + targeting_config_slug="pip_never_used" + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ("experiments", "0213_alter_nimbusexperiment_languages_field_for_mobile_client"), + ] + + operations = [ + migrations.RunPython(migrate_sticky_experiments), + ] diff --git a/app/experimenter/experiments/tests/test_migrations.py b/app/experimenter/experiments/tests/test_migrations.py index 0c5da671aa..3c0d26dd70 100644 --- a/app/experimenter/experiments/tests/test_migrations.py +++ b/app/experimenter/experiments/tests/test_migrations.py @@ -1,66 +1,129 @@ from django_test_migrations.contrib.unittest_case import MigratorTestCase -from experimenter.base.models import Language, Locale from experimenter.experiments.constants import NimbusConstants from experimenter.experiments.models import NimbusExperiment as Experiment +DESKTOP_STICKY_TARGETING_SLUGS = [ + "first_run", + "first_run_chrome", + "first_run_win1903", + "not_tcp_study_first_run", + "windows_userchoice_first_run", + "infrequent_user_uris", + "infrequent_user_need_pin", + "infrequent_user_need_default", + "infrequent_user_need_default_has_pin", + "infrequent_user_has_default_need_pin", + "infrequent_windows_user_need_pin", + "infrequent_win_user_uris", + "infrequent_user_5_bookmarks", + "casual_user_uris", + "casual_user_need_pin", + "casual_user_need_default", + "casual_user_need_default_has_pin", + "casual_user_has_default_need_pin", + "regular_user_uris", + "regular_user_need_pin", + "regular_user_need_default", + "regular_user_need_default_has_pin", + "regular_user_has_default_need_pin", + "regular_user_uses_fxa", + "core_user_uris", +] + +MOBILE_STICKY_TARGETING_SLUGS = [ + "mobile_new_users", + "mobile_recently_updated_users", +] + +NON_STICKY_TARGETING_SLUGS = [ + "core_user_has_default_need_pin", + "core_user_need_default", + "core_user_need_default_has_pin", + "core_user_need_pin", + "fx95_desktop_users", + "homepage_google_dot_com", + "mac_only", + "no_enterprise_or_last_30d_vpn_use", + "no_enterprise_or_past_vpn", + "no_enterprise_users", + "no_targeting", + "not_tcp_study", + "pocket_common", + "rally_core_addon_user", + "urlbar_firefox_suggest", + "windows_userchoice", +] + class TestMigration(MigratorTestCase): - migrate_from = ("experiments", "0212_nimbusexperiment_is_sticky") - migrate_to = ( + migrate_from = ( "experiments", "0213_alter_nimbusexperiment_languages_field_for_mobile_client", ) + migrate_to = ( + "experiments", + "0214_migrate_sticky", + ) def prepare(self): """Prepare some data before the migration.""" User = self.old_state.apps.get_model("auth", "User") - Locale = self.old_state.apps.get_model("base", "Locale") - Language = self.old_state.apps.get_model("base", "Language") NimbusExperiment = self.old_state.apps.get_model( "experiments", "NimbusExperiment" ) user = User.objects.create(email="test@example.com") - locale_en = Locale.objects.create(code="en", name="English") - Language.objects.create(code="en", name="English") - locale_ts = Locale.objects.create(code="ts", name="does not exist") - locale_ca = Locale.objects.create(code="en-CA", name="English Canada") - desktop_experiment = NimbusExperiment.objects.create( - owner=user, - name="desktop experiment", - slug="desktop_experiment", - application=Experiment.Application.DESKTOP, - status=NimbusConstants.Status.DRAFT, - ) - desktop_experiment.locales.add(locale_en.id) - mobile_experiments_with_locales = NimbusExperiment.objects.create( - owner=user, - name="mobile experiment with locales", - slug="mobile_experiment_with_locales", - application=Experiment.Application.FOCUS_ANDROID, - status=NimbusConstants.Status.DRAFT, - ) - mobile_experiments_with_locales.locales.add(locale_en.id, locale_ts.id, locale_ca) + # Desktop experiments that will keep the same targeting slug + # but have is_sticky set to True + for desktop_sticky_targeting in DESKTOP_STICKY_TARGETING_SLUGS: + NimbusExperiment.objects.create( + owner=user, + name=desktop_sticky_targeting, + slug=desktop_sticky_targeting, + application=Experiment.Application.DESKTOP, + status=NimbusConstants.Status.DRAFT, + targeting_config_slug=desktop_sticky_targeting, + is_sticky=False, + ) + # Targeting slug 'pip_never_used_sticky' will get replaced with 'pip_never_used' + # and is_sticky set to True NimbusExperiment.objects.create( owner=user, - name="mobile experiment without locales", - slug="mobile_experiment_without_locales", - application=Experiment.Application.FOCUS_IOS, + name="pip_never_used_sticky", + slug="pip_never_used_sticky", + application=Experiment.Application.DESKTOP, status=NimbusConstants.Status.DRAFT, + targeting_config_slug="pip_never_used_sticky", + is_sticky=False, ) - mobile_experiments_with_locales_but_not_in_draft = ( + # Mobile experiments that will keep the same targeting slug + # but have is_sticky set to True + for mobile_sticky_targeting in MOBILE_STICKY_TARGETING_SLUGS: NimbusExperiment.objects.create( owner=user, - name="mobile experiment not in draft with locales", - slug="mobile_experiment_not_in_draft_with_locales", - application=Experiment.Application.IOS, - status=NimbusConstants.Status.COMPLETE, + name=mobile_sticky_targeting, + slug=mobile_sticky_targeting, + application=Experiment.Application.FOCUS_ANDROID, + status=NimbusConstants.Status.DRAFT, + targeting_config_slug=mobile_sticky_targeting, + is_sticky=False, + ) + + # Desktop experiments that will keep the same targeting slug + # and keep is_sticky set to False + for non_sticky_targeting in NON_STICKY_TARGETING_SLUGS: + NimbusExperiment.objects.create( + owner=user, + name=non_sticky_targeting, + slug=non_sticky_targeting, + application=Experiment.Application.DESKTOP, + status=NimbusConstants.Status.DRAFT, + targeting_config_slug=non_sticky_targeting, + is_sticky=False, ) - ) - mobile_experiments_with_locales_but_not_in_draft.locales.add(locale_en.id) def test_migration(self): """Run the test itself.""" @@ -68,38 +131,59 @@ def test_migration(self): "experiments", "NimbusExperiment" ) - # get desktop experiment - desktop_experiment = NimbusExperiment.objects.get( - application=Experiment.Application.DESKTOP + # Desktop experiments that keep the same sticky targeting slug + # now have is_sticky set to True + self.assertEqual( + NimbusExperiment.objects.filter( + slug__in=DESKTOP_STICKY_TARGETING_SLUGS, is_sticky=True + ).count(), + len(DESKTOP_STICKY_TARGETING_SLUGS), ) - self.assertEqual( - desktop_experiment.locales.all()[0].code, Locale.objects.get(code="en").code + NimbusExperiment.objects.filter( + slug__in=DESKTOP_STICKY_TARGETING_SLUGS, is_sticky=False + ).count(), + 0, ) - # get mobile experiment included locale field - mobile_experiment_with_locales = NimbusExperiment.objects.get( - application=Experiment.Application.FOCUS_ANDROID + # 'pip_never_used_sticky' becomes 'pip_never_used' with is_sticky set to True + self.assertFalse( + NimbusExperiment.objects.filter( + targeting_config_slug="pip_never_used_sticky" + ).exists() ) - self.assertEqual(mobile_experiment_with_locales.locales.count(), 0) - self.assertEqual( - mobile_experiment_with_locales.languages.all()[0].code, - Language.objects.get(code="en").code, + self.assertTrue( + NimbusExperiment.objects.filter( + targeting_config_slug="pip_never_used", is_sticky=True + ).exists() ) - # get mobile experiment without locale field - mobile_experiment_with_locales = NimbusExperiment.objects.get( - application=Experiment.Application.FOCUS_IOS + # Mobile experiments that keep the same sticky targeting slug + # now have is_sticky set to True + self.assertEqual( + NimbusExperiment.objects.filter( + slug__in=MOBILE_STICKY_TARGETING_SLUGS, is_sticky=True + ).count(), + len(MOBILE_STICKY_TARGETING_SLUGS), ) - self.assertEqual(mobile_experiment_with_locales.locales.count(), 0) - self.assertEqual(mobile_experiment_with_locales.languages.count(), 0) - - # get mobile experiment without locale field which is not in draft - mobile_experiments_with_locales_but_not_in_draft = NimbusExperiment.objects.get( - application=Experiment.Application.IOS + self.assertEqual( + NimbusExperiment.objects.filter( + slug__in=MOBILE_STICKY_TARGETING_SLUGS, is_sticky=False + ).count(), + 0, ) + # Desktop experiments that keep the same non-sticky targeting slug + # still have is_sticky set to False + self.assertEqual( + NimbusExperiment.objects.filter( + slug__in=NON_STICKY_TARGETING_SLUGS, is_sticky=True + ).count(), + 0, + ) self.assertEqual( - mobile_experiments_with_locales_but_not_in_draft.locales.all()[0].code, - Locale.objects.get(code="en").code, + NimbusExperiment.objects.filter( + slug__in=NON_STICKY_TARGETING_SLUGS, is_sticky=False + ).count(), + len(NON_STICKY_TARGETING_SLUGS), ) diff --git a/app/experimenter/experiments/tests/test_models.py b/app/experimenter/experiments/tests/test_models.py index 446a319559..96883a7c86 100644 --- a/app/experimenter/experiments/tests/test_models.py +++ b/app/experimenter/experiments/tests/test_models.py @@ -565,10 +565,7 @@ def test_targeting_with_languages_mobile(self): ) self.assertEqual( experiment.targeting, - ( - "(is_already_enrolled || days_since_install < 7) " - "&& (language in ['en', 'es', 'fr'])" - ), + "(days_since_install < 7) && (language in ['en', 'es', 'fr'])", ) JEXLParser().parse(experiment.targeting) @@ -632,7 +629,7 @@ def test_targeting_with_sticky_mobile(self): "(is_already_enrolled) " "|| " "(" - "(is_already_enrolled || days_since_install < 7) " + "(days_since_install < 7) " "&& (app_version|versionCompare('100.!') >= 0) " "&& (language in ['en']) " "&& (region in ['CA'])" @@ -668,7 +665,7 @@ def test_targeting_with_locales_languages_mobile(self): self.assertEqual( experiment.targeting, ( - "(is_already_enrolled || days_since_install < 7) " + "(days_since_install < 7) " "&& ('de' in locale || 'en' in locale || 'es' in locale " "|| 'ro' in locale) " "&& (language in ['en', 'es', 'fr'])" diff --git a/app/experimenter/targeting/constants.py b/app/experimenter/targeting/constants.py index 1513e224d3..b86016e60b 100644 --- a/app/experimenter/targeting/constants.py +++ b/app/experimenter/targeting/constants.py @@ -20,7 +20,6 @@ def __post_init__(self): self.targeting_configs.append(self) -STICKY = "experiment.slug in activeExperiments" HAS_PIN = "!doesAppNeedPin" NEED_DEFAULT = "!isDefaultBrowser" PROFILE28DAYS = "(currentDate|date - profileAgeCreated|date) / 86400000 >= 28" @@ -39,10 +38,9 @@ def __post_init__(self): name="First start-up users", slug="first_run", description=("First start-up users (e.g. for about:welcome)"), - targeting=("(({is_first_startup} && {not_see_aw}) || {sticky})").format( + targeting="({is_first_startup} && {not_see_aw})".format( is_first_startup="isFirstStartup", not_see_aw="!('trailhead.firstrun.didSeeAboutWelcome'|preferenceValue)", - sticky=STICKY, ), desktop_telemetry=("payload.info.profile_subsession_counter = 1"), application_choice_names=(Application.DESKTOP.name,), @@ -55,7 +53,7 @@ def __post_init__(self): "First start-up users (e.g. for about:welcome) who download Firefox " "from Chrome" ), - targeting=("{first_run} && attributionData.ua == 'chrome'").format( + targeting="{first_run} && attributionData.ua == 'chrome'".format( first_run=FIRST_RUN.targeting ), desktop_telemetry=( @@ -68,7 +66,7 @@ def __post_init__(self): name="First start-up users on Windows 10 1903 (build 18362) or newer", slug="first_run_win1903", description="First start-up users (e.g. for about:welcome) on Windows 1903+", - targeting=("{first_run} && os.windowsBuildNumber >= 18362").format( + targeting="{first_run} && os.windowsBuildNumber >= 18362".format( first_run=FIRST_RUN.targeting ), desktop_telemetry=( @@ -81,9 +79,11 @@ def __post_init__(self): name="Exclude users in the TCP revenue study", slug="not_tcp_study", description="Exclude users with certain search codes set", - targeting="!'browser.search.param.google_channel_us'|preferenceValue('')|regExpMatch" - "('^[ntc]us5$') && !'browser.search.param.google_channel_row'|preferenceValue('')|" - "regExpMatch('^[ntc]row5$')", + targeting=( + "!'browser.search.param.google_channel_us'|preferenceValue('')|regExpMatch" + "('^[ntc]us5$') && !'browser.search.param.google_channel_row'|preferenceValue('')" + "|regExpMatch('^[ntc]row5$')" + ), desktop_telemetry="", application_choice_names=(Application.DESKTOP.name,), ) @@ -144,7 +144,7 @@ def __post_init__(self): name="New Users on Mobile", slug="mobile_new_users", description=("New users on mobile who installed the app less than a week ago"), - targeting=("is_already_enrolled || days_since_install < 7"), + targeting="days_since_install < 7", desktop_telemetry="", application_choice_names=( Application.FENIX.name, @@ -163,9 +163,7 @@ def __post_init__(self): "Users who updated their app within the last week. " "This excludes users who are new users" ), - targeting=( - "is_already_enrolled || (days_since_update < 7 && days_since_install >= 7)" - ), + targeting="days_since_update < 7 && days_since_install >= 7", desktop_telemetry="", application_choice_names=( Application.FENIX.name, @@ -248,8 +246,11 @@ def __post_init__(self): name="Infrequent user (uris)", slug="infrequent_user_uris", description="Between 1 and 6 days of activity in the past 28 days", - targeting=f"{STICKY} || {PROFILE28DAYS} && " - "userMonthlyActivity|length >= 1 && userMonthlyActivity|length <= 6", + targeting=( + f"{PROFILE28DAYS} " + "&& userMonthlyActivity|length >= 1 " + "&& userMonthlyActivity|length <= 6" + ), desktop_telemetry="", application_choice_names=(Application.DESKTOP.name,), ) @@ -324,8 +325,11 @@ def __post_init__(self): name="Casual user (uris)", slug="casual_user_uris", description="Between 7 and 13 days of activity in the past 28 days", - targeting=f"{STICKY} || {PROFILE28DAYS} && " - "userMonthlyActivity|length >= 7 && userMonthlyActivity|length <= 13", + targeting=( + f"{PROFILE28DAYS} " + "&& userMonthlyActivity|length >= 7 " + "&& userMonthlyActivity|length <= 13" + ), desktop_telemetry="", application_choice_names=(Application.DESKTOP.name,), ) @@ -370,8 +374,11 @@ def __post_init__(self): name="Regular user (uris)", slug="regular_user_uris", description="Between 14 and 20 days of activity in the past 28 days", - targeting=f"{STICKY} || {PROFILE28DAYS} && " - "userMonthlyActivity|length >= 14 && userMonthlyActivity|length <= 20", + targeting=( + f"{PROFILE28DAYS} " + "&& userMonthlyActivity|length >= 14 " + "&& userMonthlyActivity|length <= 20" + ), desktop_telemetry="", application_choice_names=(Application.DESKTOP.name,), ) @@ -425,7 +432,7 @@ def __post_init__(self): name="Core user (uris)", slug="core_user_uris", description="At least 21 days of activity in the past 28 days", - targeting=f"{STICKY} || {PROFILE28DAYS} && " "userMonthlyActivity|length >= 21", + targeting=f"{PROFILE28DAYS} && userMonthlyActivity|length >= 21", desktop_telemetry="", application_choice_names=(Application.DESKTOP.name,), ) @@ -510,17 +517,6 @@ def __post_init__(self): application_choice_names=(Application.DESKTOP.name,), ) -PIP_NEVER_USED_STICKY = NimbusTargetingConfig( - name="PiP Never Used (Sticky)", - slug="pip_never_used_sticky", - description="Users that have never used Picture in Picture, with sticky enrollment", - targeting="(({pip}) || ({sticky}))".format( - pip=PIP_NEVER_USED.targeting, - sticky=STICKY, - ), - desktop_telemetry="", - application_choice_names=(Application.DESKTOP.name,), -) RALLY_CORE_ADDON_USER = NimbusTargetingConfig( name="Mozilla Rally Core Add-on User", diff --git a/app/tests/integration/nimbus/app_contexts.json b/app/tests/integration/nimbus/app_contexts.json index c2cd1098b4..87ea0e3072 100644 --- a/app/tests/integration/nimbus/app_contexts.json +++ b/app/tests/integration/nimbus/app_contexts.json @@ -1,149 +1,124 @@ { - "query_result": { - "id": 15734423, - "query_hash": "85e731a13b18b6a67cb24eb9ab953842", - "query": "-- A query to generate Nimbus application contexts in JSON format for the\n-- purpose of testing client targeting.\n--\n-- The existing app contexts used by the Nimbus Rust SDK are shaped like this:\n--\n-- pub struct AppContext {\n-- pub app_name: String,\n-- pub app_id: String,\n-- pub channel: String,\n-- pub app_version: Option,\n-- pub app_build: Option,\n-- pub architecture: Option,\n-- pub device_manufacturer: Option,\n-- pub device_model: Option,\n-- pub locale: Option,\n-- pub os: Option,\n-- pub os_version: Option,\n-- pub android_sdk_version: Option,\n-- pub debug_tag: Option,\n-- pub installation_date: Option,\n-- pub home_directory: Option,\n-- #[serde(flatten)]\n-- pub custom_targeting_attributes: Option>,\n-- }\n\nWITH randomish_sample AS\n(\n SELECT \n \"fenix\" AS app_name,\n \"org.mozilla.firefox\" AS app_id,\n client_info.app_channel AS channel,\n client_info.app_display_version AS app_version,\n client_info.app_build AS app_build,\n client_info.architecture AS architecture,\n client_info.device_manufacturer AS device_manufacturer,\n client_info.device_model AS device_model,\n client_info.locale AS locale,\n client_info.os AS os,\n client_info.os_version AS os_version,\n client_info.android_sdk_version AS android_sdk_version,\n NULL AS debug_tag,\n UNIX_SECONDS(PARSE_TIMESTAMP('%F%Ez', client_info.first_run_date)) AS installation_date,\n NULL AS home_directory,\n NULL AS custom_targeting_attributes,\n ROW_NUMBER() OVER(PARTITION BY client_info.locale) AS rowno,\n FROM \n org_mozilla_firefox.baseline\n WHERE \n DATE(submission_timestamp) >= DATE_SUB(CURRENT_DATE(), INTERVAL 7 DAY)\n -- grabbing @ 100 samples here to ensure we get enough to LIMIT to 20 later\n AND RAND() < 100/(SELECT COUNT(*) FROM org_mozilla_firefox.baseline WHERE DATE(submission_timestamp) >= DATE_SUB(CURRENT_DATE(), INTERVAL 7 DAY)) \n), app_contexts AS (\n -- Stripping out the rowno, which is used to ensure we don't have duplicate locales\n -- so that we can match the AppContext object schema\n SELECT\n app_name,\n app_id,\n channel,\n app_version,\n app_build,\n architecture,\n device_manufacturer,\n device_model,\n locale,\n os,\n os_version,\n android_sdk_version,\n debug_tag,\n installation_date,\n home_directory,\n custom_targeting_attributes\n FROM\n randomish_sample\n WHERE\n rowno <= 1\n)\n\nSELECT \n TO_JSON_STRING(t) AS app_context\nFROM \n app_contexts AS t\nLIMIT\n 20", - "data": { - "columns": [ - { - "name": "app_context", - "friendly_name": "app_context", - "type": "string" + "query_result": { + "id": 15734423, + "query_hash": "85e731a13b18b6a67cb24eb9ab953842", + "query": "-- A query to generate Nimbus application contexts in JSON format for the\n-- purpose of testing client targeting.\n--\n-- The existing app contexts used by the Nimbus Rust SDK are shaped like this:\n--\n-- pub struct AppContext {\n-- pub app_name: String,\n-- pub app_id: String,\n-- pub channel: String,\n-- pub app_version: Option,\n-- pub app_build: Option,\n-- pub architecture: Option,\n-- pub device_manufacturer: Option,\n-- pub device_model: Option,\n-- pub locale: Option,\n-- pub os: Option,\n-- pub os_version: Option,\n-- pub android_sdk_version: Option,\n-- pub debug_tag: Option,\n-- pub installation_date: Option,\n-- pub home_directory: Option,\n-- #[serde(flatten)]\n-- pub custom_targeting_attributes: Option>,\n-- }\n\nWITH randomish_sample AS\n(\n SELECT \n \"fenix\" AS app_name,\n \"org.mozilla.firefox\" AS app_id,\n client_info.app_channel AS channel,\n client_info.app_display_version AS app_version,\n client_info.app_build AS app_build,\n client_info.architecture AS architecture,\n client_info.device_manufacturer AS device_manufacturer,\n client_info.device_model AS device_model,\n client_info.locale AS locale,\n client_info.os AS os,\n client_info.os_version AS os_version,\n client_info.android_sdk_version AS android_sdk_version,\n NULL AS debug_tag,\n UNIX_SECONDS(PARSE_TIMESTAMP('%F%Ez', client_info.first_run_date)) AS installation_date,\n NULL AS home_directory,\n NULL AS custom_targeting_attributes,\n ROW_NUMBER() OVER(PARTITION BY client_info.locale) AS rowno,\n FROM \n org_mozilla_firefox.baseline\n WHERE \n DATE(submission_timestamp) >= DATE_SUB(CURRENT_DATE(), INTERVAL 7 DAY)\n -- grabbing @ 100 samples here to ensure we get enough to LIMIT to 20 later\n AND RAND() < 100/(SELECT COUNT(*) FROM org_mozilla_firefox.baseline WHERE DATE(submission_timestamp) >= DATE_SUB(CURRENT_DATE(), INTERVAL 7 DAY)) \n), app_contexts AS (\n -- Stripping out the rowno, which is used to ensure we don't have duplicate locales\n -- so that we can match the AppContext object schema\n SELECT\n app_name,\n app_id,\n channel,\n app_version,\n app_build,\n architecture,\n device_manufacturer,\n device_model,\n locale,\n os,\n os_version,\n android_sdk_version,\n debug_tag,\n installation_date,\n home_directory,\n custom_targeting_attributes\n FROM\n randomish_sample\n WHERE\n rowno <= 1\n)\n\nSELECT \n TO_JSON_STRING(t) AS app_context\nFROM \n app_contexts AS t\nLIMIT\n 20", + "data": { + "columns": [ + { + "name": "app_context", + "friendly_name": "app_context", + "type": "string" + } + ], + "rows": [ + { + "app_context": { + "app_name": "fenix", + "app_id": "org.mozilla.firefox", + "channel": "release", + "app_version": "95.2.0", + "app_build": "2015851759", + "architecture": "armeabi-v7a", + "device_manufacturer": "HUAWEI", + "device_model": "MED-LX9", + "locale": "es-MX", + "os": "Android", + "os_version": "10", + "android_sdk_version": "29", + "debug_tag": null, + "installation_date": 1641967200, + "home_directory": null, + "custom_targeting_attributes": {} + } + }, + { + "app_context": { + "app_name": "fenix", + "app_id": "org.mozilla.firefox", + "channel": "release", + "app_version": "100.1.2", + "app_build": "2015879543", + "architecture": "arm64-v8a", + "device_manufacturer": "asus", + "device_model": "ASUS_Z012DA", + "locale": "zh-TW", + "os": "Android", + "os_version": "8.0.0", + "android_sdk_version": "26", + "debug_tag": null, + "installation_date": 1597939200, + "home_directory": null, + "custom_targeting_attributes": {} + } + }, + { + "app_context": { + "app_name": "fenix", + "app_id": "org.mozilla.firefox", + "channel": "release", + "app_version": "100.1.2", + "app_build": "2015879543", + "architecture": "armeabi-v7a", + "device_manufacturer": "HUAWEI", + "device_model": "AMN-LX1", + "locale": "fr-FR", + "os": "Android", + "os_version": "9", + "android_sdk_version": "28", + "debug_tag": null, + "installation_date": 1650405600, + "home_directory": null, + "custom_targeting_attributes": {} + } + }, + { + "app_context": { + "app_name": "fenix", + "app_id": "org.mozilla.firefox", + "channel": "release", + "app_version": "100.2.0", + "app_build": "2015880367", + "architecture": "arm64-v8a", + "device_manufacturer": "samsung", + "device_model": "SM-A326B", + "locale": "en-GB", + "os": "Android", + "os_version": "12", + "android_sdk_version": "31", + "debug_tag": null, + "installation_date": 1651602600, + "home_directory": null, + "custom_targeting_attributes": {} } - ], - "rows": [ - { - "app_context": { - "app_name": "fenix", - "app_id": "org.mozilla.firefox", - "channel": "release", - "app_version": "95.2.0", - "app_build": "2015851759", - "architecture": "armeabi-v7a", - "device_manufacturer": "HUAWEI", - "device_model": "MED-LX9", - "locale": "es-MX", - "os": "Android", - "os_version": "10", - "android_sdk_version": "29", - "debug_tag": null, - "installation_date": 1641967200, - "home_directory": null, - "custom_targeting_attributes": - { - "is_already_enrolled": "true", - "days_since_update": "1", - "days_since_install": "1" - } - } - }, - { - "app_context": { - "app_name": "fenix", - "app_id": "org.mozilla.firefox", - "channel": "release", - "app_version": "100.1.2", - "app_build": "2015879543", - "architecture": "arm64-v8a", - "device_manufacturer": "asus", - "device_model": "ASUS_Z012DA", - "locale": "zh-TW", - "os": "Android", - "os_version": "8.0.0", - "android_sdk_version": "26", - "debug_tag": null, - "installation_date": 1597939200, - "home_directory": null, - "custom_targeting_attributes": - { - "is_already_enrolled": "true", - "days_since_update": "1", - "days_since_install": "1" - } - } - }, - { - "app_context": { - "app_name": "fenix", - "app_id": "org.mozilla.firefox", - "channel": "release", - "app_version": "100.1.2", - "app_build": "2015879543", - "architecture": "armeabi-v7a", - "device_manufacturer": "HUAWEI", - "device_model": "AMN-LX1", - "locale": "fr-FR", - "os": "Android", - "os_version": "9", - "android_sdk_version": "28", - "debug_tag": null, - "installation_date": 1650405600, - "home_directory": null, - "custom_targeting_attributes": - { - "is_already_enrolled": "true", - "days_since_update": "1", - "days_since_install": "1" - } - } - }, - { - "app_context": { - "app_name": "fenix", - "app_id": "org.mozilla.firefox", - "channel": "release", - "app_version": "100.2.0", - "app_build": "2015880367", - "architecture": "arm64-v8a", - "device_manufacturer": "samsung", - "device_model": "SM-A326B", - "locale": "en-GB", - "os": "Android", - "os_version": "12", - "android_sdk_version": "31", - "debug_tag": null, - "installation_date": 1651602600, - "home_directory": null, - "custom_targeting_attributes": - { - "is_already_enrolled": "true", - "days_since_update": "1", - "days_since_install": "1" - } - } - }, - { - "app_context": { - "app_name": "fenix", - "app_id": "org.mozilla.firefox", - "channel": "release", - "app_version": "100.1.2", - "app_build": "2015879543", - "architecture": "arm64-v8a", - "device_manufacturer": "samsung", - "device_model": "SM-G998U", - "locale": "en-US ", - "os": "Android", - "os_version": "12", - "android_sdk_version": "31", - "debug_tag": null, - "installation_date": 1612674000, - "home_directory": null, - "custom_targeting_attributes": - { - "is_already_enrolled": "true", - "days_since_update": "1", - "days_since_install": "1" - } - } + }, + { + "app_context": { + "app_name": "fenix", + "app_id": "org.mozilla.firefox", + "channel": "release", + "app_version": "100.1.2", + "app_build": "2015879543", + "architecture": "arm64-v8a", + "device_manufacturer": "samsung", + "device_model": "SM-G998U", + "locale": "en-US ", + "os": "Android", + "os_version": "12", + "android_sdk_version": "31", + "debug_tag": null, + "installation_date": 1612674000, + "home_directory": null, + "custom_targeting_attributes": {} } - ], - "metadata": { - "data_scanned": 123246204411 } - }, - "data_source_id": 63, - "runtime": 7.98217, - "retrieved_at": "2022-05-24T22:03:07.953Z" - } - } \ No newline at end of file + ], + "metadata": { + "data_scanned": 123246204411 + } + }, + "data_source_id": 63, + "runtime": 7.98217, + "retrieved_at": "2022-05-24T22:03:07.953Z" + } +} diff --git a/app/tests/integration/nimbus/test_mobile_integration.py b/app/tests/integration/nimbus/test_mobile_integration.py index 8c639b29c0..8cd0e61c46 100644 --- a/app/tests/integration/nimbus/test_mobile_integration.py +++ b/app/tests/integration/nimbus/test_mobile_integration.py @@ -9,7 +9,7 @@ nimbus = pytest.importorskip("nimbus_rust") -def locale_databse_id_loader(locales=None): +def locale_database_id_loader(locales=None): locale_list = [] path = Path().resolve() path = str(path) @@ -26,7 +26,7 @@ def locale_databse_id_loader(locales=None): def client_info_list(): with open("nimbus/app_contexts.json") as file: - return json.load(file)["query_result"]["data"]["rows"] + return [r["app_context"] for r in json.load(file)["query_result"]["data"]["rows"]] @pytest.fixture(params=helpers.load_targeting_configs(app="MOBILE")) @@ -69,25 +69,10 @@ def _client_helper(app_context): return _client_helper -@pytest.fixture(name="jexl_evaluator") -def fixture_jexl_evaluator(): - def _eval_jexl(sdk_client, expression, context): - targetting_helper = sdk_client.create_targeting_helper(additional_context=context) - try: - value = targetting_helper.eval_jexl(expression) - except nimbus.NimbusError.EvaluationError: - return None - else: - return value - - return _eval_jexl - - @pytest.mark.run_targeting @pytest.mark.parametrize("targeting", helpers.load_targeting_configs("MOBILE")) @pytest.mark.parametrize("context", client_info_list()) def test_check_mobile_targeting( - jexl_evaluator, sdk_client, load_app_context, context, @@ -96,19 +81,28 @@ def test_check_mobile_targeting( create_mobile_experiment, targeting, ): - experiment_name = f"{slugify(experiment_name)}" - context = context["app_context"] + # The context fixtures can only contain strings or null context["locale"] = context["locale"][:2] # strip region + # This context dictionary supports non string values + # and must be encoded as JSON before being passed to the evaluator + custom_targeting_attributes = json.dumps( + {"is_already_enrolled": True, "days_since_update": 1, "days_since_install": 1} + ) + client = sdk_client(load_app_context(context)) + targeting_helper = client.create_targeting_helper( + additional_context=custom_targeting_attributes + ) + + experiment_slug = str(slugify(experiment_name)) create_mobile_experiment( - experiment_name, + experiment_slug, context["app_name"], - locale_databse_id_loader([context["locale"]]), + locale_database_id_loader([context["locale"]]), targeting, ) - data = helpers.load_experiment_data(experiment_name) + data = helpers.load_experiment_data(experiment_slug) expression = data["data"]["experimentBySlug"]["jexlTargetingExpression"] - assert jexl_evaluator( - sdk_client(load_app_context(context)), - expression, - json.dumps(context["custom_targeting_attributes"]), - ) + + # The evaluator will throw if it detects a syntax error, a comparison type mismatch, + # or an undefined variable + targeting_helper.eval_jexl(expression)