Repository: favonia/cloudflare-ddns
Pull request #769 (2024-06-25 16:53:12)

fix(monitor): distinguish success pinging from logging in SendResponse
The bug was introduced in #762.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/769?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) Report
Attention: Patch coverage is `80.00000%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 90.91%. Comparing base [(`9cdfec3`)](https://app.codecov.io/gh/favonia/cloudflare-ddns/commit/9cdfec393a3ef24803fc7a1280515c1fda72102e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) to head [(`55c51e2`)](https://app.codecov.io/gh/favonia/cloudflare-ddns/commit/55c51e237006cced970176ebefae222e965bda17?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).

| [Files](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/769?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | Patch % | Lines |
|---|---|---|
| [cmd/ddns/ddns.go](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/769?src=pr&el=tree&filepath=cmd%2Fddns%2Fddns.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#diff-Y21kL2RkbnMvZGRucy5nbw==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/769?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #769      +/-   ##
==========================================
+ Coverage   90.90%   90.91%   +0.01%     
==========================================
  Files          52       52              
  Lines        2122     2125       +3     
==========================================
+ Hits         1929     1932       +3     
  Misses        182      182              
  Partials       11       11              
```

| [Flag](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/769/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/769/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | `90.91% <80.00%> (+0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/769?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).


Patch:
diff --git a/cmd/ddns/ddns.go b/cmd/ddns/ddns.go
index aca8f6b..b4e5152 100644
--- a/cmd/ddns/ddns.go
+++ b/cmd/ddns/ddns.go
@@ -57,7 +57,7 @@ func initConfig(ctx context.Context, ppfmt pp.PP) (*config.Config, setter.Setter
 func stopUpdating(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Setter) {
 	if c.DeleteOnStop {
 		resp := updater.DeleteIPs(ctx, ppfmt, c, s)
-		monitor.SendResponseAll(ctx, ppfmt, c.Monitors, resp)
+		monitor.SendResponseAll(ctx, ppfmt, c.Monitors, resp, false)
 		notifier.SendResponseAll(ctx, ppfmt, c.Notifiers, resp)
 	}
 }
@@ -123,7 +123,7 @@ func realMain() int { //nolint:funlen
 			monitor.SuccessAll(ctx, ppfmt, c.Monitors, "Started (no action)")
 		} else {
 			resp := updater.UpdateIPs(ctxWithSignals, ppfmt, c, s)
-			monitor.SendResponseAll(ctx, ppfmt, c.Monitors, resp)
+			monitor.SendResponseAll(ctx, ppfmt, c.Monitors, resp, true)
 			notifier.SendResponseAll(ctx, ppfmt, c.Notifiers, resp)
 		}
 
diff --git a/internal/monitor/base.go b/internal/monitor/base.go
index d18b3ba..c35eed3 100644
--- a/internal/monitor/base.go
+++ b/internal/monitor/base.go
@@ -36,11 +36,14 @@ type Monitor interface {
 	ExitStatus(ctx context.Context, ppfmt pp.PP, code int, message string) bool
 }
 
-func SendResponse(ctx context.Context, ppfmt pp.PP, m Monitor, r response.Response) bool {
+func SendResponse(ctx context.Context, ppfmt pp.PP, m Monitor, r response.Response, ping bool) bool {
 	msg := strings.Join(r.MonitorMessages, "\n")
-	if r.Ok {
+	switch {
+	case r.Ok && ping:
+		return m.Success(ctx, ppfmt, msg)
+	case r.Ok && !ping:
 		return m.Log(ctx, ppfmt, msg)
-	} else {
+	default:
 		return m.Failure(ctx, ppfmt, msg)
 	}
 }
diff --git a/internal/monitor/composite.go b/internal/monitor/composite.go
index 382ee5c..bfac28e 100644
--- a/internal/monitor/composite.go
+++ b/internal/monitor/composite.go
@@ -70,10 +70,10 @@ func ExitStatusAll(ctx context.Context, ppfmt pp.PP, ms []Monitor, code int, mes
 }
 
 // SendResponseAll calls [SendResponse] for each monitor in ms.
-func SendResponseAll(ctx context.Context, ppfmt pp.PP, ms []Monitor, resp response.Response) bool {
+func SendResponseAll(ctx context.Context, ppfmt pp.PP, ms []Monitor, resp response.Response, ping bool) bool {
 	ok := true
 	for _, m := range ms {
-		if !SendResponse(ctx, ppfmt, m, resp) {
+		if !SendResponse(ctx, ppfmt, m, resp, ping) {
 			ok = false
 		}
 	}


Test patch:
diff --git a/internal/monitor/composite_test.go b/internal/monitor/composite_test.go
index a71a850..0bc1857 100644
--- a/internal/monitor/composite_test.go
+++ b/internal/monitor/composite_test.go
@@ -132,10 +132,13 @@ func TestSendResponseAll(t *testing.T) {
 	notifierMessages := []string{"ocean"}
 
 	for name, tc := range map[string]struct {
-		ok bool
+		ok   bool
+		ping bool
 	}{
-		"ok":    {true},
-		"notok": {false},
+		"success": {true, true},
+		"log":     {true, false},
+		"fail1":   {false, true},
+		"fail2":   {false, false},
 	} {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
@@ -146,9 +149,12 @@ func TestSendResponseAll(t *testing.T) {
 
 			for range 5 {
 				m := mocks.NewMockMonitor(mockCtrl)
-				if tc.ok {
+				switch {
+				case tc.ok && tc.ping:
+					m.EXPECT().Success(context.Background(), mockPP, monitorMessage)
+				case tc.ok && !tc.ping:
 					m.EXPECT().Log(context.Background(), mockPP, monitorMessage)
-				} else {
+				default:
 					m.EXPECT().Failure(context.Background(), mockPP, monitorMessage)
 				}
 				ms = append(ms, m)
@@ -159,7 +165,7 @@ func TestSendResponseAll(t *testing.T) {
 				MonitorMessages:  monitorMessages,
 				NotifierMessages: notifierMessages,
 			}
-			monitor.SendResponseAll(context.Background(), mockPP, ms, resp)
+			monitor.SendResponseAll(context.Background(), mockPP, ms, resp, tc.ping)
 		})
 	}
 }