1953641 - TriggerActionSchemas test is out of date, validating only the triggers in CFRMessageProvider

Status: ASSIGNED

(Firefox :: Messaging System, task, P1)

Description

[Shane Hughes [:aminomancer]]

Any trigger that's in ASRouterTriggerListeners has to be in the trigger action doc for this test to pass, but really all triggers should be documented anyway, even the ones that call sendTriggerMessage directly. And we seem to be succeeding on that front. But as for the schema itself, it's falling out of date.

In theory this test is supposed to fail if there's a message with a trigger that's not accounted for in the schema. But because the test only validates the triggers used in CFRMessageProvider, and we don't use that provider anymore, we've effectively lost coverage on the schema.

Instead of validating CFRMessageProvider, which is basically frozen in time, we should validate OnboardingMessageProvider and PanelTestProvider. Those providers are used routinely, more regularly updated, so more likely to contain the current stable of triggers. It would be nice to add NimbusRolloutMessageProvider and InflightAssetsMessageProvider, but since we remotely deploy messages to old versions, it's possible they won't validate in current versions if something in the schema changes.

---

Further, I think it would be wise to change test_trigger_docs so that it doesn't only validate the triggers that have proper listeners. There are other triggers that are invoked without a listener, by calling ASRouter.sendTriggerMessage. It wouldn't be realistic to search the codebase to find those instances dynamically. While we could maintain a list of those triggers in the test, the point of this test is to stop engineers from landing a patch if they forgot to document their new trigger. So it's already assuming that the engineer forgot the doc. And if they forgot the documentation, it's even more likely that they'd forget to manually add the trigger to this obscure test.

So it makes sense to use some kind of heuristic that can find the trigger even if the engineer forgot to add any of the supporting docs. There isn't really a foolproof way to do that, but it seems the best option within reason is to concatenate all the trigger listeners with all the unique trigger ids found in the aforementioned providers. That gives us the most complete list of trigger ids. And we should be using that list not just to check if the trigger has a doc, but also to check if the trigger has a schema.

Comment 1

[Shane Hughes [:aminomancer]]

Attachment: WIP: Bug 1953641 - Update trigger action schemas test. r=#omc-reviewers