ocket8888 commented on code in PR #6762:
URL: https://github.com/apache/trafficcontrol/pull/6762#discussion_r871619565
##########
traffic_ops/testing/api/v4/cdnnotifications_test.go:
##########
@@ -53,26 +80,21 @@ func CreateTestCDNNotifications(t *testing.T) {
var opts client.RequestOptions
for _, cdn := range testData.CDNs {
resp, _, err :=
TOSession.CreateCDNNotification(tc.CDNNotificationRequest{CDN: cdn.Name,
Notification: "test notification: " + cdn.Name}, opts)
Review Comment:
That's not quite a duplicate test. If I change the notifications code so
that no matter what you POST to it, it just stores the static string `test
notification: cdn2` it would have failed the old tests, but will pass the tests
as of this rewrite, or if I change it to only store the first notification it
gets and use it as the text content of all future notifications, or if a
database change is introduced with a bad constraint such that it will store one
notification just fine but any and all future ones erroneously violate a
constraint and TO doesn't handle the resulting db error properly and just
returns a 200. So there's a case not being covered anymore. Those are all
pretty unlikely scenarios, but they used to be tested against and won't be
anymore.
It probably suffices to check two before the tests become truly degenerate,
but as it's a read-only operation I wouldn't see the harm in doing them all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
