From bba19eac9ff607f1381454472e438c6128eb8649 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 30 Aug 2021 11:57:24 -0500 Subject: [PATCH 1/5] test multiple channel configs --- smoketest/oncallnotify_test.go | 10 ++++++---- smoketest/oncallnotifytod_test.go | 9 +++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/smoketest/oncallnotify_test.go b/smoketest/oncallnotify_test.go index c2584e3ea2..4e420e088b 100644 --- a/smoketest/oncallnotify_test.go +++ b/smoketest/oncallnotify_test.go @@ -34,11 +34,12 @@ func TestOnCallNotify(t *testing.T) { insert into notification_channels (id, type, name, value) values - ({{uuid "chan"}}, 'SLACK', '#test', {{slackChannelID "test"}}); - + ({{uuid "chan1"}}, 'SLACK', '#test1', {{slackChannelID "test1"}}), + ({{uuid "chan2"}}, 'SLACK', '#test2', {{slackChannelID "test2"}}); + insert into schedule_data (schedule_id, data) values - ({{uuid "sid"}}, '{"V1":{"OnCallNotificationRules": [{"ChannelID": {{uuidJSON "chan"}} }]}}'); + ({{uuid "sid"}}, '{"V1":{"OnCallNotificationRules": [{"ChannelID": {{uuidJSON "chan1"}} }, {"ChannelID": {{uuidJSON "chan2"}} }]}}'); ` h := harness.NewHarness(t, sql, "outgoing-messages-schedule-id") defer h.Close() @@ -56,5 +57,6 @@ func TestOnCallNotify(t *testing.T) { }) require.NoError(t, err) - h.Slack().Channel("test").ExpectMessage("on-call", "testschedule", "bob", "joe") + h.Slack().Channel("test1").ExpectMessage("on-call", "testschedule", "bob", "joe") + h.Slack().Channel("test2").ExpectMessage("on-call", "testschedule", "bob", "joe") } diff --git a/smoketest/oncallnotifytod_test.go b/smoketest/oncallnotifytod_test.go index 57ab9c53e5..d3ca1205b6 100644 --- a/smoketest/oncallnotifytod_test.go +++ b/smoketest/oncallnotifytod_test.go @@ -26,11 +26,12 @@ func TestOnCallNotifyTOD(t *testing.T) { insert into notification_channels (id, type, name, value) values - ({{uuid "chan"}}, 'SLACK', '#test', {{slackChannelID "test"}}); + ({{uuid "chan1"}}, 'SLACK', '#test1', {{slackChannelID "test1"}}), + ({{uuid "chan2"}}, 'SLACK', '#test2', {{slackChannelID "test2"}}); insert into schedule_data (schedule_id, data) values - ({{uuid "sid"}}, '{"V1":{"OnCallNotificationRules": [{"ChannelID": {{uuidJSON "chan"}}, "Time": "00:00" }]}}'); + ({{uuid "sid"}}, '{"V1":{"OnCallNotificationRules": [{"ChannelID": {{uuidJSON "chan1"}}, "Time": "00:00" },{"ChannelID": {{uuidJSON "chan2"}}, "Time": "00:00" }]}}'); ` h := harness.NewHarness(t, sql, "outgoing-messages-schedule-id") defer h.Close() @@ -39,6 +40,6 @@ func TestOnCallNotifyTOD(t *testing.T) { h.FastForward(24 * time.Hour) - h.Slack().Channel("test").ExpectMessage("on-call", "testschedule", "bob") - + h.Slack().Channel("test1").ExpectMessage("on-call", "testschedule", "bob") + h.Slack().Channel("test2").ExpectMessage("on-call", "testschedule", "bob") } From 41ae9ef523139d990f4b4d3d94af13278c1f3033 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 30 Aug 2021 12:07:49 -0500 Subject: [PATCH 2/5] don't check for unexpected messages on test abort --- smoketest/harness/slack.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/smoketest/harness/slack.go b/smoketest/harness/slack.go index cc008e0f7a..82798e39ea 100644 --- a/smoketest/harness/slack.go +++ b/smoketest/harness/slack.go @@ -24,7 +24,8 @@ type SlackChannel interface { type slackServer struct { h *Harness *mockslack.Server - channels map[string]*slackChannel + hasFailure bool + channels map[string]*slackChannel } type slackChannel struct { @@ -47,7 +48,7 @@ func (s *slackServer) WaitAndAssert() { s.h.Trigger() var hasFailure bool for _, ch := range s.channels { - hasFailure = hasFailure || ch.hasUnexpectedMessages() + hasFailure = s.hasFailure || hasFailure || ch.hasUnexpectedMessages() } if hasFailure { @@ -95,6 +96,7 @@ func (ch *slackChannel) ExpectMessage(keywords ...string) { select { case <-timeout.C: + ch.h.slack.hasFailure = true ch.h.t.Fatalf("timeout waiting for slack message: Channel=%s; ID=%s; keywords=%v\nGot: %#v", ch.name, ch.id, keywords, ch.h.slack.Messages(ch.id)) default: } From e1fbbca4d315c326b46467580a94caf118f3606d Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 30 Aug 2021 12:08:02 -0500 Subject: [PATCH 3/5] test for dedup --- smoketest/oncallnotifytod_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smoketest/oncallnotifytod_test.go b/smoketest/oncallnotifytod_test.go index d3ca1205b6..7787379493 100644 --- a/smoketest/oncallnotifytod_test.go +++ b/smoketest/oncallnotifytod_test.go @@ -31,7 +31,7 @@ func TestOnCallNotifyTOD(t *testing.T) { insert into schedule_data (schedule_id, data) values - ({{uuid "sid"}}, '{"V1":{"OnCallNotificationRules": [{"ChannelID": {{uuidJSON "chan1"}}, "Time": "00:00" },{"ChannelID": {{uuidJSON "chan2"}}, "Time": "00:00" }]}}'); + ({{uuid "sid"}}, '{"V1":{"OnCallNotificationRules": [{"ChannelID": {{uuidJSON "chan1"}}, "Time": "00:00" },{"ChannelID": {{uuidJSON "chan1"}}, "Time": "01:00" },{"ChannelID": {{uuidJSON "chan2"}}, "Time": "00:00" }]}}'); ` h := harness.NewHarness(t, sql, "outgoing-messages-schedule-id") defer h.Close() @@ -40,6 +40,7 @@ func TestOnCallNotifyTOD(t *testing.T) { h.FastForward(24 * time.Hour) + // should only send 1 message to each channel h.Slack().Channel("test1").ExpectMessage("on-call", "testschedule", "bob") h.Slack().Channel("test2").ExpectMessage("on-call", "testschedule", "bob") } From a6ba1bef77d28f6afe9dfe050996a14e8d7d54cd Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 30 Aug 2021 12:09:41 -0500 Subject: [PATCH 4/5] fix TOD notifications for multi-channel --- engine/schedulemanager/update.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/engine/schedulemanager/update.go b/engine/schedulemanager/update.go index bfdfcafa5d..6168b9afdd 100644 --- a/engine/schedulemanager/update.go +++ b/engine/schedulemanager/update.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/json" "fmt" + "sort" "time" "github.com/google/uuid" @@ -234,7 +235,7 @@ func (db *DB) update(ctx context.Context) error { } // Notify changed schedules - needsOnCallNotification := make(map[string]uuid.UUID) + needsOnCallNotification := make(map[string][]uuid.UUID) for schedID := range changedSchedules { data := scheduleData[schedID] if data == nil { @@ -245,7 +246,7 @@ func (db *DB) update(ctx context.Context) error { continue } - needsOnCallNotification[schedID] = r.ChannelID + needsOnCallNotification[schedID] = append(needsOnCallNotification[schedID], r.ChannelID) } } @@ -253,7 +254,7 @@ func (db *DB) update(ctx context.Context) error { var hadChange bool for i, r := range data.V1.OnCallNotificationRules { if r.NextNotification != nil && !r.NextNotification.After(now) { - needsOnCallNotification[schedID] = r.ChannelID + needsOnCallNotification[schedID] = append(needsOnCallNotification[schedID], r.ChannelID) } newTime := nextOnCallNotification(now.In(tz[schedID]), r) @@ -277,10 +278,18 @@ func (db *DB) update(ctx context.Context) error { } } - for schedID, chanID := range needsOnCallNotification { - _, err = tx.StmtContext(ctx, db.scheduleOnCallNotification).ExecContext(ctx, uuid.New(), chanID, schedID) - if err != nil { - return err + for schedID, chanIDs := range needsOnCallNotification { + sort.Slice(chanIDs, func(i, j int) bool { return chanIDs[i].String() < chanIDs[j].String() }) + var lastID uuid.UUID + for _, chanID := range chanIDs { + if chanID == lastID { + continue + } + lastID = chanID + _, err = tx.StmtContext(ctx, db.scheduleOnCallNotification).ExecContext(ctx, uuid.New(), chanID, schedID) + if err != nil { + return err + } } } From cece534bb47282da575f34cbb4966b82cce13296 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 30 Aug 2021 12:26:18 -0500 Subject: [PATCH 5/5] check new channels at beginning of test --- smoketest/oncallnotify_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smoketest/oncallnotify_test.go b/smoketest/oncallnotify_test.go index 4e420e088b..2ce6f59762 100644 --- a/smoketest/oncallnotify_test.go +++ b/smoketest/oncallnotify_test.go @@ -44,7 +44,8 @@ func TestOnCallNotify(t *testing.T) { h := harness.NewHarness(t, sql, "outgoing-messages-schedule-id") defer h.Close() - h.Slack().Channel("test").ExpectMessage("on-call", "testschedule", "bob") + h.Slack().Channel("test1").ExpectMessage("on-call", "testschedule", "bob") + h.Slack().Channel("test2").ExpectMessage("on-call", "testschedule", "bob") h.FastForward(time.Hour)