Repository: favonia/cloudflare-ddns
Pull request #273 (2022-11-16 08:20:33)

fix(setter): improve monitor messages

codecov[bot]: # [Codecov](https://codecov.io/gh/favonia/cloudflare-ddns/pull/273?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) Report
> :exclamation: No coverage uploaded for pull request base (`main@b24cce6`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#section-missing-base-commit).
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##             main     #273   +/-   ##
=======================================
  Coverage        ?   85.95%           
=======================================
  Files           ?       29           
  Lines           ?     1986           
  Branches        ?        0           
=======================================
  Hits            ?     1707           
  Misses          ?      268           
  Partials        ?       11           
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `85.95% <100.00%> (?)` | |

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.

| [Impacted Files](https://codecov.io/gh/favonia/cloudflare-ddns/pull/273?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | Coverage Δ | |
|---|---|---|
| [internal/setter/setter.go](https://codecov.io/gh/favonia/cloudflare-ddns/pull/273/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#diff-aW50ZXJuYWwvc2V0dGVyL3NldHRlci5nbw==) | `100.00% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia)


Patch:
diff --git a/internal/setter/setter.go b/internal/setter/setter.go
index 1d3b149..5b8b89b 100644
--- a/internal/setter/setter.go
+++ b/internal/setter/setter.go
@@ -123,7 +123,7 @@ func (s *setter) Set(ctx context.Context, ppfmt pp.PP,
 				// If the updating succeeds, we can move on to the next stage!
 				ppfmt.Noticef(pp.EmojiUpdateRecord,
 					"Updated a stale %s record of %q (ID: %s)", recordType, domainDescription, id)
-				monitorMessage = fmt.Sprintf("%s %s set to %s", recordType, domainDescription, ip.String())
+				monitorMessage = fmt.Sprintf("Set %s %s to %s", recordType, domainDescription, ip.String())
 
 				// Now it's up to date! Note that matchedIDs must be empty; otherwise uptodate would have been true.
 				uptodate = true
@@ -155,7 +155,7 @@ func (s *setter) Set(ctx context.Context, ppfmt pp.PP,
 		if id, ok := s.Handle.CreateRecord(ctx, ppfmt,
 			domain, ipnet, ip, ttl, proxied); ok {
 			ppfmt.Noticef(pp.EmojiCreateRecord, "Added a new %s record of %q (ID: %s)", recordType, domainDescription, id)
-			monitorMessage = fmt.Sprintf("%s %s set to %s", recordType, domainDescription, ip.String())
+			monitorMessage = fmt.Sprintf("Set %s %s to %s", recordType, domainDescription, ip.String())
 
 			// Now it's up to date! matchedIDs and unmatchedIDsToUpdate must both be empty at this point
 			uptodate = true
@@ -172,7 +172,7 @@ func (s *setter) Set(ctx context.Context, ppfmt pp.PP,
 
 	// We meant to clear all records, and we did it!
 	if numUndeletedUnmatched == 0 && !ip.IsValid() {
-		monitorMessage = fmt.Sprintf("%s %s cleared", recordType, domainDescription)
+		monitorMessage = fmt.Sprintf("Cleared %s %s", recordType, domainDescription)
 	}
 
 	// We should also delete all duplicate records even if they are up to date.
@@ -189,7 +189,12 @@ func (s *setter) Set(ctx context.Context, ppfmt pp.PP,
 		ppfmt.Errorf(pp.EmojiError,
 			"Failed to complete updating of %s records of %q; records might be inconsistent",
 			recordType, domainDescription)
-		return false, fmt.Sprintf("%s %s possibly inconsistent", recordType, domainDescription)
+		if ip.IsValid() {
+			monitorMessage = fmt.Sprintf("Failed to set %s %s", recordType, domainDescription)
+		} else {
+			monitorMessage = fmt.Sprintf("Failed to clear %s %s", recordType, domainDescription)
+		}
+		return false, monitorMessage
 	}
 
 	return true, monitorMessage


Test patch:
diff --git a/internal/setter/setter_test.go b/internal/setter/setter_test.go
index e24b116..7ed3317 100644
--- a/internal/setter/setter_test.go
+++ b/internal/setter/setter_test.go
@@ -58,7 +58,7 @@ func TestSet(t *testing.T) {
 		"0/1-false": {
 			ip1,
 			true,
-			`AAAA sub.test.org set to ::1`,
+			`Set AAAA sub.test.org to ::1`,
 			1,
 			false,
 			func(m *mocks.MockPP) {
@@ -74,7 +74,7 @@ func TestSet(t *testing.T) {
 		"1unmatched/300-false": {
 			ip1,
 			true,
-			`AAAA sub.test.org set to ::1`,
+			`Set AAAA sub.test.org to ::1`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -95,7 +95,7 @@ func TestSet(t *testing.T) {
 		"1unmatched-updatefail/300-false": {
 			ip1,
 			true,
-			`AAAA sub.test.org set to ::1`,
+			`Set AAAA sub.test.org to ::1`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -116,7 +116,7 @@ func TestSet(t *testing.T) {
 		"1unmatched-nil/300-false": {
 			invalidIP,
 			true,
-			`AAAA sub.test.org cleared`,
+			`Cleared AAAA sub.test.org`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -129,6 +129,22 @@ func TestSet(t *testing.T) {
 				)
 			},
 		},
+		"1unmatched-nil/fail/300-false": {
+			invalidIP,
+			false,
+			`Failed to clear AAAA sub.test.org`,
+			300,
+			false,
+			func(m *mocks.MockPP) {
+				m.EXPECT().Errorf(pp.EmojiError, "Failed to complete updating of %s records of %q; records might be inconsistent", "AAAA", "sub.test.org") //nolint:lll
+			},
+			func(ctx context.Context, ppfmt pp.PP, m *mocks.MockHandle) {
+				gomock.InOrder(
+					m.EXPECT().ListRecords(ctx, ppfmt, domain, ipNetwork).Return(map[string]netip.Addr{record1: ip1}, true),
+					m.EXPECT().DeleteRecord(ctx, ppfmt, domain, ipNetwork, record1).Return(false),
+				)
+			},
+		},
 		"1matched/300-false": {
 			ip1,
 			true,
@@ -181,7 +197,7 @@ func TestSet(t *testing.T) {
 		"2unmatched/300-false": {
 			ip1,
 			true,
-			`AAAA sub.test.org set to ::1`,
+			`Set AAAA sub.test.org to ::1`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -207,7 +223,7 @@ func TestSet(t *testing.T) {
 		"2unmatched-updatefail/300-false": {
 			ip1,
 			true,
-			`AAAA sub.test.org set to ::1`,
+			`Set AAAA sub.test.org to ::1`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -234,7 +250,7 @@ func TestSet(t *testing.T) {
 		"2unmatched-updatefailtwice/300-false": {
 			ip1,
 			true,
-			`AAAA sub.test.org set to ::1`,
+			`Set AAAA sub.test.org to ::1`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -259,7 +275,7 @@ func TestSet(t *testing.T) {
 		"2unmatched-updatefail-deletefail-updatefail/300-false": {
 			ip1,
 			false,
-			`AAAA sub.test.org possibly inconsistent`,
+			`Failed to set AAAA sub.test.org`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -284,7 +300,7 @@ func TestSet(t *testing.T) {
 		"2unmatched-updatefailtwice-createfail/300-false": {
 			ip1,
 			false,
-			`AAAA sub.test.org possibly inconsistent`,
+			`Failed to set AAAA sub.test.org`,
 			300,
 			false,
 			func(m *mocks.MockPP) {
@@ -321,7 +337,7 @@ func TestSet(t *testing.T) {
 		"impossible-invalidIP-records/300-false": {
 			invalidIP,
 			true,
-			`AAAA sub.test.org cleared`,
+			`Cleared AAAA sub.test.org`,
 			300,
 			false,
 			func(m *mocks.MockPP) {