From 5126e1f68fd117e9093e7709dcf29ee9679fdcfd Mon Sep 17 00:00:00 2001 From: Benjamin Forehand Date: Tue, 13 Sep 2022 17:06:06 -0500 Subject: [PATCH 1/6] fix #7740 type(test): Extend targeting integration tests Because * We currently check each targeting expression validates in the browser, but it is a very basic expression This Commit * Extends that test with additional fiends such as channel, locale, region and versions. Also closes #6791 --- app/tests/integration/nimbus/conftest.py | 56 ++++++++++++++++-- .../nimbus/models/base_dataclass.py | 2 +- .../nimbus/test_firefox_targeting.py | 59 +++++++++++++++---- 3 files changed, 99 insertions(+), 18 deletions(-) diff --git a/app/tests/integration/nimbus/conftest.py b/app/tests/integration/nimbus/conftest.py index ec897a5409..ce78797587 100644 --- a/app/tests/integration/nimbus/conftest.py +++ b/app/tests/integration/nimbus/conftest.py @@ -24,7 +24,7 @@ from requests.packages.urllib3.util.retry import Retry APPLICATION_FEATURE_IDS = { - BaseExperimentApplications.DESKTOP: "1", + BaseExperimentApplications.FIREFOX_DESKTOP: "1", BaseExperimentApplications.FENIX: "2", BaseExperimentApplications.IOS: "3", BaseExperimentApplications.FOCUS_ANDROID: "4", @@ -32,7 +32,7 @@ } APPLICATION_KINTO_REVIEW_PATH = { - BaseExperimentApplications.DESKTOP: ( + BaseExperimentApplications.FIREFOX_DESKTOP: ( "#/buckets/main-workspace/collections/nimbus-desktop-experiments/simple-review" ), BaseExperimentApplications.FENIX: ( @@ -50,7 +50,7 @@ } APPLICATION_KINTO_COLLECTION = { - "DESKTOP": KINTO_COLLECTION_DESKTOP, + "FIREFOX_DESKTOP": KINTO_COLLECTION_DESKTOP, "FENIX": KINTO_COLLECTION_MOBILE, "IOS": KINTO_COLLECTION_MOBILE, "FOCUS_ANDROID": KINTO_COLLECTION_MOBILE, @@ -274,7 +274,6 @@ def _create_desktop_experiment(slug, app, targeting, **data): "variables": { "input": { "id": experiment_id, - "name": f"test_check_telemetry_enrollment-{experiment_id}", "hypothesis": "Test hypothesis", "application": app.upper(), "changelogMessage": "test updated", @@ -288,6 +287,13 @@ def _create_desktop_experiment(slug, app, targeting, **data): "treatmentBranches": data.get("treatement_branch"), "populationPercent": data.get("population_percent"), "totalEnrolledClients": data.get("total_enrolled_clients"), + "channel": data.get("channel"), + "firefoxMinVersion": data.get("firefox_min_version"), + "firefoxMaxVersion": data.get("firefox_max_version"), + "locales": data.get("locales"), + "countries": data.get("countries"), + "proposedEnrollment": data.get("proposed_enrollment"), + "proposedDuration": data.get("proposed_duration"), } }, "query": "mutation updateExperiment($input: ExperimentInput!) \ @@ -320,6 +326,48 @@ def _language_database_id_loader(languages=None): return _language_database_id_loader +@pytest.fixture(name="countries_database_id_loader") +def fixture_countries_database_id_loader(): + """Return database id's for languages""" + + def _countries_database_id_loader(countries=None): + country_list = [] + path = Path().resolve() + path = str(path) + path = path.strip("/tests/integration/nimbus") + path = os.path.join("/", path, "experimenter/base/fixtures/countries.json") + with open(path) as file: + data = json.loads(file.read()) + for country in countries: + for item in data: + if country in item["fields"]["code"][:2]: + country_list.append(item["pk"]) + return country_list + + return _countries_database_id_loader + + +@pytest.fixture(name="locales_database_id_loader") +def fixture_locales_database_id_loader(): + """Return database id's for languages""" + + def _locales_database_id_loader(locales=None): + locale_list = [] + path = Path().resolve() + path = str(path) + path = path.strip("/tests/integration/nimbus") + path = os.path.join("/", path, "experimenter/base/fixtures/locales.json") + with open(path) as file: + data = json.loads(file.read()) + for locale in locales: + for item in data: + if locale in item["fields"]["code"]: + locale_list.append(item["pk"]) + return locale_list + + return _locales_database_id_loader + + @pytest.fixture def trigger_experiment_loader(selenium): def _trigger_experiment_loader(): diff --git a/app/tests/integration/nimbus/models/base_dataclass.py b/app/tests/integration/nimbus/models/base_dataclass.py index 009fa8202a..90ba58824e 100644 --- a/app/tests/integration/nimbus/models/base_dataclass.py +++ b/app/tests/integration/nimbus/models/base_dataclass.py @@ -4,7 +4,7 @@ class BaseExperimentApplications(Enum): - DESKTOP = "DESKTOP" + FIREFOX_DESKTOP = "DESKTOP" FENIX = "FENIX" IOS = "IOS" FOCUS_ANDROID = "FOCUS_ANDROID" diff --git a/app/tests/integration/nimbus/test_firefox_targeting.py b/app/tests/integration/nimbus/test_firefox_targeting.py index 488ba6dbb0..3f869ec680 100644 --- a/app/tests/integration/nimbus/test_firefox_targeting.py +++ b/app/tests/integration/nimbus/test_firefox_targeting.py @@ -1,10 +1,13 @@ import json import time +from urllib.parse import urljoin import pytest import requests from nimbus.models.base_dataclass import BaseExperimentApplications from nimbus.pages.browser import Browser +from nimbus.pages.experimenter.summary import SummaryPage +from nimbus.utils import helpers LOAD_DATA_RETRIES = 10 LOAD_DATA_RETRY_DELAY = 1.0 @@ -76,22 +79,52 @@ def targeting_config_slug(request): @pytest.mark.run_targeting def test_check_targeting( selenium, + slugify, default_data, - create_experiment, + experiment_name, targeting_config_slug, + create_desktop_experiment, + countries_database_id_loader, + locales_database_id_loader, ): - # TODO #6791 - # If the targeting config slug includes the word desktop it will cause this test - # to run against applications other than desktop, which will then fail. - # This check will prevent the test from executing fully but we should dig - # into preventing this case altogether when we have time. - if default_data.application != BaseExperimentApplications.DESKTOP: - return - - default_data.audience.targeting = targeting_config_slug - experiment = create_experiment(selenium) - - experiment_data = load_experiment_data(experiment.experiment_slug) + targeting = helpers.load_targeting_configs()[1] + experiment_slug = str(slugify(experiment_name)) + create_desktop_experiment( + experiment_slug, + "desktop", + targeting_config_slug, + public_description="Some sort of words", + risk_revenue=False, + risk_partner_related=False, + risk_brand=False, + feature_config=1, + reference_branch={ + "description": "reference branch", + "name": "Branch 1", + "ratio": 50, + "featureEnabled": True, + "featureValue": "{}", + }, + treatement_branch=[ + { + "description": "treatment branch", + "name": "Branch 2", + "ratio": 50, + "featureEnabled": False, + "featureValue": "", + } + ], + population_percent="75", + total_enrolled_clients=35, + channel="NIGHTLY", + firefox_min_version="FIREFOX_100", + firefox_max_version="FIREFOX_120", + countries=countries_database_id_loader(["CA"]), + proposed_enrollment="14", + proposed_duration="30", + locales=locales_database_id_loader(["en-CA"]), + ) + experiment_data = load_experiment_data(experiment_slug) targeting = experiment_data["data"]["experimentBySlug"]["jexlTargetingExpression"] recipe = experiment_data["data"]["experimentBySlug"]["recipeJson"] From a6026a8aeee1f61fc76d97b6fd2d398f7c4dd314 Mon Sep 17 00:00:00 2001 From: Benjamin Forehand Date: Tue, 13 Sep 2022 17:08:48 -0500 Subject: [PATCH 2/6] Add missing file. --- app/tests/integration/nimbus/parallel_pytest_args.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/tests/integration/nimbus/parallel_pytest_args.txt b/app/tests/integration/nimbus/parallel_pytest_args.txt index 4b1a2c2dc1..d4a1125e1f 100644 --- a/app/tests/integration/nimbus/parallel_pytest_args.txt +++ b/app/tests/integration/nimbus/parallel_pytest_args.txt @@ -1,7 +1,7 @@ --k DESKTOP -m run_per_app +-k FIREFOX_DESKTOP -m run_per_app -k FENIX -m run_per_app -k IOS -m run_per_app -k FOCUS_ANDROID -m run_per_app -k FOCUS_IOS -m run_per_app --k DESKTOP -m run_once --dist=loadgroup -n=2 --k DESKTOP -m run_targeting -n 2 +-k FIREFOX_DESKTOP -m run_once --dist=loadgroup -n=2 +-k FIREFOX_DESKTOP -m run_targeting -n 2 From 3672601b2e6e3de6ef81bc08cae594a18e47049b Mon Sep 17 00:00:00 2001 From: Benjamin Forehand Date: Tue, 13 Sep 2022 17:17:57 -0500 Subject: [PATCH 3/6] Attempt to fix failing tests. --- app/tests/integration/nimbus/models/base_dataclass.py | 2 +- app/tests/integration/nimbus/test_firefox_targeting.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/app/tests/integration/nimbus/models/base_dataclass.py b/app/tests/integration/nimbus/models/base_dataclass.py index 90ba58824e..6fba0be383 100644 --- a/app/tests/integration/nimbus/models/base_dataclass.py +++ b/app/tests/integration/nimbus/models/base_dataclass.py @@ -4,7 +4,7 @@ class BaseExperimentApplications(Enum): - FIREFOX_DESKTOP = "DESKTOP" + FIREFOX_DESKTOP = "FIREFOX_DESKTOP" FENIX = "FENIX" IOS = "IOS" FOCUS_ANDROID = "FOCUS_ANDROID" diff --git a/app/tests/integration/nimbus/test_firefox_targeting.py b/app/tests/integration/nimbus/test_firefox_targeting.py index 3f869ec680..929b3f1189 100644 --- a/app/tests/integration/nimbus/test_firefox_targeting.py +++ b/app/tests/integration/nimbus/test_firefox_targeting.py @@ -1,12 +1,9 @@ import json import time -from urllib.parse import urljoin import pytest import requests -from nimbus.models.base_dataclass import BaseExperimentApplications from nimbus.pages.browser import Browser -from nimbus.pages.experimenter.summary import SummaryPage from nimbus.utils import helpers LOAD_DATA_RETRIES = 10 From ee197a50aae03d2c66e274d5cf593f99952c60ce Mon Sep 17 00:00:00 2001 From: Benjamin Forehand Date: Tue, 13 Sep 2022 17:24:20 -0500 Subject: [PATCH 4/6] Revert changes to kinto. --- app/tests/integration/nimbus/conftest.py | 2 +- app/tests/integration/nimbus/models/base_dataclass.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/tests/integration/nimbus/conftest.py b/app/tests/integration/nimbus/conftest.py index ce78797587..c6a2caa2e4 100644 --- a/app/tests/integration/nimbus/conftest.py +++ b/app/tests/integration/nimbus/conftest.py @@ -50,7 +50,7 @@ } APPLICATION_KINTO_COLLECTION = { - "FIREFOX_DESKTOP": KINTO_COLLECTION_DESKTOP, + "DESKTOP": KINTO_COLLECTION_DESKTOP, "FENIX": KINTO_COLLECTION_MOBILE, "IOS": KINTO_COLLECTION_MOBILE, "FOCUS_ANDROID": KINTO_COLLECTION_MOBILE, diff --git a/app/tests/integration/nimbus/models/base_dataclass.py b/app/tests/integration/nimbus/models/base_dataclass.py index 6fba0be383..90ba58824e 100644 --- a/app/tests/integration/nimbus/models/base_dataclass.py +++ b/app/tests/integration/nimbus/models/base_dataclass.py @@ -4,7 +4,7 @@ class BaseExperimentApplications(Enum): - FIREFOX_DESKTOP = "FIREFOX_DESKTOP" + FIREFOX_DESKTOP = "DESKTOP" FENIX = "FENIX" IOS = "IOS" FOCUS_ANDROID = "FOCUS_ANDROID" From 9e30377dbfcb88f8400c5fda6b673fa7c2923698 Mon Sep 17 00:00:00 2001 From: Benjamin Forehand Date: Wed, 14 Sep 2022 09:50:04 -0500 Subject: [PATCH 5/6] Changed graphql experiment creation method. --- app/tests/integration/nimbus/conftest.py | 30 ++-------- .../nimbus/test_firefox_targeting.py | 47 +++++++++------- .../integration/nimbus/test_telemetry.py | 56 ++++++++++++++----- 3 files changed, 74 insertions(+), 59 deletions(-) diff --git a/app/tests/integration/nimbus/conftest.py b/app/tests/integration/nimbus/conftest.py index c6a2caa2e4..6b80ec785a 100644 --- a/app/tests/integration/nimbus/conftest.py +++ b/app/tests/integration/nimbus/conftest.py @@ -243,7 +243,7 @@ def _create_basic_experiment(name, app, targeting, languages=[]): @pytest.fixture def create_desktop_experiment(create_basic_experiment): - def _create_desktop_experiment(slug, app, targeting, **data): + def _create_desktop_experiment(slug, app, targeting, data): # create a basic experiment via graphql so we can get an ID create_basic_experiment( slug, @@ -269,33 +269,11 @@ def _create_desktop_experiment(slug, app, targeting, **data): ) experiment_id = response.json()["data"]["experimentBySlug"]["id"] + data.update({"id": experiment_id}) + query = { "operationName": "updateExperiment", - "variables": { - "input": { - "id": experiment_id, - "hypothesis": "Test hypothesis", - "application": app.upper(), - "changelogMessage": "test updated", - "targetingConfigSlug": targeting, - "publicDescription": data.get("public_description", "Fancy Words"), - "riskRevenue": data.get("risk_revenue"), - "riskPartnerRelated": data.get("risk_partner_related"), - "riskBrand": data.get("risk_brand"), - "featureConfigId": data.get("feature_config"), - "referenceBranch": data.get("reference_branch"), - "treatmentBranches": data.get("treatement_branch"), - "populationPercent": data.get("population_percent"), - "totalEnrolledClients": data.get("total_enrolled_clients"), - "channel": data.get("channel"), - "firefoxMinVersion": data.get("firefox_min_version"), - "firefoxMaxVersion": data.get("firefox_max_version"), - "locales": data.get("locales"), - "countries": data.get("countries"), - "proposedEnrollment": data.get("proposed_enrollment"), - "proposedDuration": data.get("proposed_duration"), - } - }, + "variables": {"input": data}, "query": "mutation updateExperiment($input: ExperimentInput!) \ {\n updateExperiment(input: $input) \ {\n message\n __typename\n }\n}\n", diff --git a/app/tests/integration/nimbus/test_firefox_targeting.py b/app/tests/integration/nimbus/test_firefox_targeting.py index 929b3f1189..7b796a56d7 100644 --- a/app/tests/integration/nimbus/test_firefox_targeting.py +++ b/app/tests/integration/nimbus/test_firefox_targeting.py @@ -86,23 +86,24 @@ def test_check_targeting( ): targeting = helpers.load_targeting_configs()[1] experiment_slug = str(slugify(experiment_name)) - create_desktop_experiment( - experiment_slug, - "desktop", - targeting_config_slug, - public_description="Some sort of words", - risk_revenue=False, - risk_partner_related=False, - risk_brand=False, - feature_config=1, - reference_branch={ + data = { + "hypothesis": "Test Hypothesis", + "application": "DESKTOP", + "changelogMessage": "test updates", + "targetingConfigSlug": targeting, + "publicDescription": "Some sort of Fancy Words", + "riskRevenue": False, + "riskPartnerRelated": False, + "riskBrand": False, + "featureConfigId": 1, + "referenceBranch": { "description": "reference branch", "name": "Branch 1", "ratio": 50, "featureEnabled": True, "featureValue": "{}", }, - treatement_branch=[ + "treatmentBranches": [ { "description": "treatment branch", "name": "Branch 2", @@ -111,15 +112,21 @@ def test_check_targeting( "featureValue": "", } ], - population_percent="75", - total_enrolled_clients=35, - channel="NIGHTLY", - firefox_min_version="FIREFOX_100", - firefox_max_version="FIREFOX_120", - countries=countries_database_id_loader(["CA"]), - proposed_enrollment="14", - proposed_duration="30", - locales=locales_database_id_loader(["en-CA"]), + "populationPercent": "100", + "totalEnrolledClients": 55, + "channel": "NIGHTLY", + "firefoxMinVersion": "FIREFOX_100", + "firefoxMaxVersion": "FIREFOX_120", + "locales": locales_database_id_loader(["en-CA"]), + "countries": countries_database_id_loader(["CA"]), + "proposedEnrollment": "14", + "proposedDuration": "30", + } + create_desktop_experiment( + experiment_slug, + "desktop", + targeting_config_slug, + data, ) experiment_data = load_experiment_data(experiment_slug) targeting = experiment_data["data"]["experimentBySlug"]["jexlTargetingExpression"] diff --git a/app/tests/integration/nimbus/test_telemetry.py b/app/tests/integration/nimbus/test_telemetry.py index 939050e25e..83ad99931d 100644 --- a/app/tests/integration/nimbus/test_telemetry.py +++ b/app/tests/integration/nimbus/test_telemetry.py @@ -72,23 +72,24 @@ def test_check_telemetry_enrollment_unenrollment( requests.delete("http://ping-server:5000/pings") targeting = helpers.load_targeting_configs()[0] experiment_slug = str(slugify(experiment_name)) - create_desktop_experiment( - experiment_slug, - "desktop", - targeting, - public_description="Some sort of words", - risk_revenue=False, - risk_partner_related=False, - risk_brand=False, - feature_config=1, - reference_branch={ + data = { + "hypothesis": "Test Hypothesis", + "application": "DESKTOP", + "changelogMessage": "test updates", + "targetingConfigSlug": targeting, + "publicDescription": "Some sort of Fancy Words", + "riskRevenue": False, + "riskPartnerRelated": False, + "riskBrand": False, + "featureConfigId": 1, + "referenceBranch": { "description": "reference branch", "name": "Branch 1", "ratio": 50, "featureEnabled": True, "featureValue": "{}", }, - treatement_branch=[ + "treatmentBranches": [ { "description": "treatment branch", "name": "Branch 2", @@ -97,8 +98,37 @@ def test_check_telemetry_enrollment_unenrollment( "featureValue": "", } ], - population_percent="100", - total_enrolled_clients=55, + "populationPercent": "100", + "totalEnrolledClients": 55, + } + create_desktop_experiment( + experiment_slug, + "desktop", + targeting, + # public_description="Some sort of words", + # risk_revenue=False, + # risk_partner_related=False, + # risk_brand=False, + # feature_config=1, + # reference_branch={ + # "description": "reference branch", + # "name": "Branch 1", + # "ratio": 50, + # "featureEnabled": True, + # "featureValue": "{}", + # }, + # treatement_branch=[ + # { + # "description": "treatment branch", + # "name": "Branch 2", + # "ratio": 50, + # "featureEnabled": False, + # "featureValue": "", + # } + # ], + # population_percent="100", + # total_enrolled_clients=55, + data, ) summary = SummaryPage(selenium, urljoin(base_url, experiment_slug)).open() summary.launch_and_approve() From a401c1a734003ec82f5caac40ddaf94b309faf5c Mon Sep 17 00:00:00 2001 From: Benjamin Forehand Date: Wed, 14 Sep 2022 11:56:56 -0500 Subject: [PATCH 6/6] Clean up --- .../integration/nimbus/test_telemetry.py | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/app/tests/integration/nimbus/test_telemetry.py b/app/tests/integration/nimbus/test_telemetry.py index 83ad99931d..fbd616eb60 100644 --- a/app/tests/integration/nimbus/test_telemetry.py +++ b/app/tests/integration/nimbus/test_telemetry.py @@ -105,29 +105,6 @@ def test_check_telemetry_enrollment_unenrollment( experiment_slug, "desktop", targeting, - # public_description="Some sort of words", - # risk_revenue=False, - # risk_partner_related=False, - # risk_brand=False, - # feature_config=1, - # reference_branch={ - # "description": "reference branch", - # "name": "Branch 1", - # "ratio": 50, - # "featureEnabled": True, - # "featureValue": "{}", - # }, - # treatement_branch=[ - # { - # "description": "treatment branch", - # "name": "Branch 2", - # "ratio": 50, - # "featureEnabled": False, - # "featureValue": "", - # } - # ], - # population_percent="100", - # total_enrolled_clients=55, data, ) summary = SummaryPage(selenium, urljoin(base_url, experiment_slug)).open()