Skip to content

Conversation

@JacobsonMT
Copy link
Contributor

Oncall notifier was missing from test config and our tests did not catch this fact. This PR:

  • Adds the oncall notifier to AllKnownConfigsForTesting so it gets tested alongside other notifiers.
  • Fixes the invalid value for httpMethod in oncall's FullValidConfigForTesting.
  • Improves the ErrInvalidMethod to show the invalid method value being attempted.
  • Improves the tests in receivers_test.go to:
    • Centralize the code that collects all notifiers.
    • Fail if some notifiers aren't present in AllKnownConfigsForTesting.

@JacobsonMT JacobsonMT requested a review from a team as a code owner June 19, 2025 18:47
@github-project-automation github-project-automation bot moved this to In review in Alerting Jun 19, 2025
@JacobsonMT JacobsonMT force-pushed the jacobsonmt/fix-missing-notifier-tests branch from 168bb03 to 2d2db59 Compare June 19, 2025 18:52
@JacobsonMT JacobsonMT merged commit 289e45f into main Jun 19, 2025
6 checks passed
@JacobsonMT JacobsonMT deleted the jacobsonmt/fix-missing-notifier-tests branch June 19, 2025 20:51
@github-project-automation github-project-automation bot moved this from In review to Done in Alerting Jun 19, 2025
stevesg pushed a commit to grafana/mimir that referenced this pull request Jun 27, 2025
#### What this PR does

Update grafana/alerting and grafana/prometheus-alertmanager modules.

[grafana/alerting
diff](grafana/alerting@3e20fda...863b097):
- grafana/alerting#347
- grafana/alerting#348
- grafana/alerting#349
 
[grafana/prometheus-alertmanager
diff](grafana/prometheus-alertmanager@92c8f63...be61a67):
- grafana/prometheus-alertmanager#117

The main change is the change of `WebhookConfig.Timeout` type from
`time.Duration` to `model.Duration` to make marshalling to JSON and YAML
the same. It does not affect YAML marshalling, only JSON and is needed
for Grafana so in the future it can start sending JSON encoded
configurations with alertmanager receivers. More details are in
grafana/prometheus-alertmanager#117

#### Checklist

- [ ] Tests updated.
- [ ] Documentation added.
- [ ] `CHANGELOG.md` updated - the order of entries should be
`[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry
is not needed, please add the `changelog-not-needed` label to the PR.
- [ ]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants