1819458 - Force enrollment should validate recipe and feature schemas

Status: RESOLVED FIXED

(Firefox :: Nimbus Desktop Client, enhancement, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

https://experimenter.services.mozilla.com/nimbus/import-bookmarks-copy-and-segmentation/summary has to be restarted because it's having schema validation errors.

There are two issues in play here:

a) Experimenter currently does FxMS schema validation against whatever is in the Nightly channel, rather than in the ideal version(s) / channel(s) that the experiment is expected to run against.

b) Force-enrolling experiments via about:studies links (often used for QA) don't currently do schema validation.

At this time, fixing experimenter (EXP-440) will likely be a lot of work.

:brennie thought it ought to be a pretty small amount of work to make force enrollments do schema validation.

I'd guess that the user story wants to look something like this:

As an engineer (QA or otherwise), when I try to force-enroll an experiment with FxMS JSON that violates the schema (and schema validation is not disabled, either in the experiment or globally), an error is displayed, and the forced enrollment fails), so I can catch more FxMS JSON errors sooner and less expensively.

Seems like in an ideal world, the error would be displayed on about:studies, but I suspect it would be faster to get into the field if it were just logged in the JavaScript console.

:barret, :mcoman, thoughts?

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

I've updated the user story, as I realized that in addition to helping QA, would also sometimes help folks developing experiments catch this issue before it even gets to QA.

Comment 2

[Marius Coman [:mcoman], Ecosystem QA, PTO until 20th of October, for any testing requests ping :poiegas]

Hi, Dan! We don’t test an experiment using forced enrollment. When we have a new experiment we try to simulate the user experience by creating a Firefox profile that will target the tested recipe in order to enroll naturally. We have found that this scenario is the most robust and it does not cost us all that much to set up.

We don’t use force enrollment because in this way we cannot validate the recipe targeting and we cannot find potential schema validation issues.

Comment 3

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1819458 - Refactor enrollment logic into EnrollmentsContext class r?emcminn

Comment 4

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1819458 - Use the same enrollment flow for natural and forced enrollment r?emcminn

Depends on D171943

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/921bf8e5a777
Refactor enrollment logic into EnrollmentsContext class r=aminomancer
https://hg.mozilla.org/integration/autoland/rev/d50e9c6e911f
Use the same enrollment flow for natural and forced enrollment r=aminomancer

Comment 6

[Natalia Csoregi [:nataliaCs]]

Backed out for causing evaluate failures.

• backout: https://hg.mozilla.org/integration/autoland/rev/3388dd32fa8437a154979467e59bce3706ab3323
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=RvSJ2S6rSKiuhkc15jPypA.0&revision=d50e9c6e911fa121873364b90291b8a085163904
• failure log: https://treeherder.mozilla.org/logviewer?job_id=408487415&repo=autoland&lineNumber=15943

[task 2023-03-10T03:48:16.233Z] 03:48:16     INFO - TEST-PASS | toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js | Sanity check the temporary file doesn't exist. - true == true - 
[task 2023-03-10T03:48:16.234Z] 03:48:16     INFO - Entering setup bound 
[task 2023-03-10T03:48:16.234Z] 03:48:16     INFO - Leaving setup bound 
[task 2023-03-10T03:48:16.234Z] 03:48:16     INFO - Entering setup bound setup
[task 2023-03-10T03:48:16.235Z] 03:48:16     INFO - Buffered messages logged at 03:48:16
[task 2023-03-10T03:48:16.235Z] 03:48:16     INFO - Leaving setup bound setup
[task 2023-03-10T03:48:16.236Z] 03:48:16     INFO - Entering test bound test_throws_if_no_experiment_in_context
[task 2023-03-10T03:48:16.237Z] 03:48:16     INFO - Buffered messages finished
[task 2023-03-10T03:48:16.237Z] 03:48:16     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js | Uncaught exception in test bound test_throws_if_no_experiment_in_context - at chrome://mochitests/content/browser/toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js:33 - TypeError: RemoteSettingsExperimentLoader.evaluateJexl is not a function
[task 2023-03-10T03:48:16.238Z] 03:48:16     INFO - Stack trace:
[task 2023-03-10T03:48:16.238Z] 03:48:16     INFO - test_throws_if_no_experiment_in_context@chrome://mochitests/content/browser/toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js:33:36
[task 2023-03-10T03:48:16.238Z] 03:48:16     INFO - handleTask@chrome://mochikit/content/browser-test.js:1037:26
[task 2023-03-10T03:48:16.239Z] 03:48:16     INFO - _runTaskBasedTest@chrome://mochikit/content/browser-test.js:1109:18
[task 2023-03-10T03:48:16.239Z] 03:48:16     INFO - Leaving test bound test_throws_if_no_experiment_in_context
[task 2023-03-10T03:48:16.240Z] 03:48:16     INFO - Entering test bound test_evaluate_jexl
[task 2023-03-10T03:48:16.240Z] 03:48:16     INFO - Not taking screenshot here: see the one that was previously logged

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9aa9a6ecae84
Refactor enrollment logic into EnrollmentsContext class r=aminomancer
https://hg.mozilla.org/integration/autoland/rev/3044e2e65c95
Use the same enrollment flow for natural and forced enrollment r=aminomancer

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/9aa9a6ecae84
https://hg.mozilla.org/mozilla-central/rev/3044e2e65c95