Stop using fake noteable_path and object_kind if possible
What does this MR do and why?
This is a follow up from !2380 (merged) and part of #968 to avoid this from happening ever again: #1376 (closed)
The around half year long incident was fixed by !2375 (merged) where we added the missing implementation noteable_kind
for IncidentEvent
. Without it, the application raises NotImplementedError
and halt all the processors, potentially crashing the application.
Why was it not detected by the tests? Because we're faking too many things in the fake events. We stubbed noteable_path
which will use noteable_kind
. This merge request removes the following stubs:
-
object_kind
-- default removed, TODO: remove all the stubs for all tests. It doesn't make sense to stub it unless we want something abnormal. This should be completely based on the event we're using. -
key
-- I don't think this is used? -
noteable_path
-- Now we can detectNotImplementedError
better -
url
-- I don't think this is used. -
payload
-- the default implementation is exactly returning it, and there's no point to stub it again.
The rationale going down the path to greatly remove stubs was described in the description of !2380 (merged)
Expected impact & dry-runs
These are strongly recommended to assist reviewers and reduce the time to merge your change.
See https://gitlab.com/gitlab-org/quality/triage-ops/-/tree/master/doc/scheduled#testing-policies-with-a-dry-run on how to perform dry-runs for new policies.
See https://gitlab.com/gitlab-org/quality/triage-ops/-/blob/master/doc/reactive/best_practices.md#use-the-sandbox-to-test-new-processors on how to make sure a new processor can be tested.
Action items
-
If adding environment variables for reactive processors, update config/triage-web.yaml
and.gitlab/ci/triage-web.yml
-
(If applicable) Add documentation to the handbook pages for Triage Operations => - (If applicable) Identify the affected groups and how to communicate to them:
-
/cc @ person_or_group
=> -
Relevant Slack channels => -
Engineering week-in-review
-
Part of #968