Skip to content

Conversation

piksel
Copy link
Member

@piksel piksel commented Apr 2, 2021

Note: This intentionally includes tests that currently fail.

Service Compliance information moved to wiki page.

The general point of this PR is to add tests to make sure that all services comply with the first Service Compliance rule:

Services should not fail to send messages when the title param is passed. The service does not need to use it in any specific way, but it should not return an error (unless failing for some other reason).

Added test resulted in the following services that did not pass:

  • Telegram
    Supports Params? ✔ Yes!
    Supports Title prop? ⚫ No
    Silently ignores title? ❌ Error: title is not a valid config key [channels notification parsemode preview]

  • IFTT
    Supports Params? ✔ Yes!
    Uses title prop? ⚫ No
    Silently ignores title? ❌Error "title is not a valid config key [events messagevalue value1 value2 value3]"

  • OpsGenie
    Supports Params? ✔ Yes!
    Uses title prop? ⚫ No
    Silently ignores title? ❌ Error: title is not a valid config key [actions alias description details entity note priority responders source tags user visibleto]

Fixes for the above services were added, and all three now pass. IFTT and OpsGenie can actually make use of the title as well if configured, but Telegram just ignores it.

@piksel piksel force-pushed the test/service-compliance branch from 485bc97 to b8c4d0c Compare April 2, 2021 22:01
@piksel piksel added this to the v0.5.0 milestone Apr 2, 2021
piksel added 3 commits April 20, 2021 14:26
note: the title is ignored by default (titlevalue set to 0)
this also changes the behaviour of long messages, since the maximum length of the Message field for opsgenie is 140 characters.
in both of these cases, the Description will be used for the full message, and the message will be set to the title (or to the first 130 characters of the message if no title is provided).
@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #160 (b5f98f7) into main (31ce8d4) will decrease coverage by 0.19%.
The diff coverage is 55.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #160      +/-   ##
==========================================
- Coverage   64.16%   63.96%   -0.20%     
==========================================
  Files          70       70              
  Lines        2009     2026      +17     
==========================================
+ Hits         1289     1296       +7     
- Misses        606      611       +5     
- Partials      114      119       +5     
Impacted Files Coverage Δ
pkg/services/ifttt/ifttt_config.go 84.00% <0.00%> (-16.00%) ⬇️
pkg/services/opsgenie/opsgenie_config.go 85.71% <ø> (ø)
pkg/services/telegram/telegram_config.go 60.86% <ø> (ø)
pkg/services/opsgenie/opsgenie.go 66.10% <64.28%> (-3.29%) ⬇️
pkg/services/gotify/gotify.go 76.92% <100.00%> (ø)
pkg/services/join/join.go 0.00% <0.00%> (ø)
pkg/services/smtp/smtp.go 82.01% <0.00%> (ø)
pkg/services/xmpp/xmpp.go 27.27% <0.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31ce8d4...b5f98f7. Read the comment docs.

@piksel piksel merged commit 23170b8 into main Apr 20, 2021
@piksel piksel deleted the test/service-compliance branch April 20, 2021 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant