added coverage and fixed the integration test suite#1
Open
mdugas wants to merge 4 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR parameterizes the IP-check URL (defaulting to https://ifconfig.me), removes a Docker Compose healthcheck, expands unit tests for IP checking, record updates, notifications, rrset parsing, cycle orchestration, and error types, adds integration test helpers, and updates Docker packaging and examples. ChangesComprehensive Test Coverage Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
integration_test.go (1)
30-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a unique ntfy topic per test to prevent stale-message false positives.
A shared topic (
ntfyTopic) allows earlier test messages to satisfy laterwaitForNtfyMessagechecks. This can hide real regressions.Also applies to: 249-260, 267-276, 324-343, 370-394
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration_test.go` around lines 30 - 33, Replace the shared ntfyTopic constant with a per-test unique topic string (e.g., generate at runtime using test identifier or a random/UUID suffix) and update every test that currently reads ntfyTopic to use the new per-test topic; specifically change the global ntfyTopic usage and callers like waitForNtfyMessage to accept/receive the test-specific topic (or build it inside each test using t.Name() or a UUID) so each test publishes/subscribes to a fresh ntfy topic and avoids stale-message cross-test interference.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration_test.go`:
- Around line 514-540: TestIntegration_Client_Get is calling setupIntegration
(and getting recordsClient and cleanup) even though the test only exercises
PowerDNS Get, which adds an unnecessary ntfy dependency; remove the call to
setupIntegration and the unused recordsClient/cleanup, and instead instantiate
only what this test needs: create httpClient := newTestHTTPClient() and
concreteClient := newRecordsClient(pdnsKey, pdnsURL, httpClient), keep
waitForPowerDNS(...) unchanged, then proceed with the existing Get assertions;
update the function to eliminate references to recordsClient and cleanup so the
test no longer depends on setupIntegration.
- Around line 396-417: buildBinary and runBinary can hang and currently swallow
process errors; update both to use a context with a short timeout (e.g.,
context.WithTimeout) when creating the command so builds/runs cannot block
forever, and propagate errors instead of masking them. In buildBinary replace
exec.Command with exec.CommandContext and on error include the captured output
in t.Fatalf; in runBinary use exec.CommandContext, capture stdout and stderr (or
CombinedOutput), check the returned error from cmd.Run/CombinedOutput, and
derive a sensible exit code: if err is nil use ProcessState.ExitCode(), if err
is *exec.ExitError use err.ProcessState.ExitCode(), and for non-start errors
return a distinct non-zero code (e.g., -1) and include the error text in the
returned output so test failures are observable; ensure ProcessState nil is not
treated as success.
In `@ip.go`:
- Around line 16-17: In getLiveIP, the error message still mentions the
hardcoded "ifconfig.me" endpoint even though the endpoint is passed via the url
parameter; update the error returns (e.g., the fmt.Errorf/return error paths in
getLiveIP) to reference the url variable and include the underlying err (for
example: "failed to create request to %s: %w" or similar), and do the same for
any other error paths in getLiveIP that mention ifconfig.me so messages reflect
the actual injected url.
In `@notify_test.go`:
- Around line 22-23: In notify_test.go the test HTTP handlers are ignoring
errors from io.ReadAll and json.Unmarshal (seen around uses of io.ReadAll,
json.Unmarshal and the receivedBody variable); update each handler to check the
error returned by io.ReadAll and return an HTTP error (e.g., http.Error with
status 400) or fail the test when read fails, then check json.Unmarshal's error
and similarly reject the request if decoding fails, ensuring every occurrence
(the blocks around io.ReadAll/json.Unmarshal at the shown spots) handles and
surfaces the error instead of discarding it.
- Around line 230-236: The test currently calls errors.As(err, &discordErr) and
errors.As(err, &ntfyErr) which will both match the same first
*NotificationError; instead, walk the joined/chain of errors starting from err
(using errors.Unwrap in a loop or errors.As in a loop over each unwrapped error)
and collect all *NotificationError instances, then assert that one has Service
== "discord" and one has Service == "ntfy"; update the assertions to check the
collected slice/map of NotificationError.Service values rather than calling
errors.As twice on the original err.
---
Outside diff comments:
In `@integration_test.go`:
- Around line 30-33: Replace the shared ntfyTopic constant with a per-test
unique topic string (e.g., generate at runtime using test identifier or a
random/UUID suffix) and update every test that currently reads ntfyTopic to use
the new per-test topic; specifically change the global ntfyTopic usage and
callers like waitForNtfyMessage to accept/receive the test-specific topic (or
build it inside each test using t.Name() or a UUID) so each test
publishes/subscribes to a fresh ntfy topic and avoids stale-message cross-test
interference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3edb9d29-6cde-4e85-a781-4c9f255cb74a
📒 Files selected for processing (11)
cycle_test.godocker-compose.integration.ymlerrors_test.gointegration_test.goip.goip_test.gomain.gomain_test.gonotify_test.gorrset_test.goupdate_test.go
💤 Files with no reviewable changes (2)
- docker-compose.integration.yml
- main_test.go
Comment on lines
+396
to
+417
| func buildBinary(t *testing.T) string { | ||
| t.Helper() | ||
| tmpDir := t.TempDir() | ||
| binary := filepath.Join(tmpDir, "iptracker") | ||
| output, err := exec.Command("go", "build", "-o", binary, ".").CombinedOutput() | ||
| if err != nil { | ||
| t.Fatalf("failed to build binary: %v\n%s", err, output) | ||
| } | ||
| return binary | ||
| } | ||
|
|
||
| func runBinary(t *testing.T, binary string, args ...string) (string, int) { | ||
| t.Helper() | ||
| cmd := exec.Command(binary, args...) | ||
| var stderr bytes.Buffer | ||
| cmd.Stderr = &stderr | ||
| cmd.Run() | ||
| exitCode := 0 | ||
| if cmd.ProcessState != nil { | ||
| exitCode = cmd.ProcessState.ExitCode() | ||
| } | ||
| return stderr.String(), exitCode |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify process execution patterns in integration_test.go
rg -n 'exec\.Command(Context)?\(|CombinedOutput\(|cmd\.Run\(|ProcessState' integration_test.goRepository: Kronus-Lab/iptracker
Length of output: 284
🏁 Script executed:
#!/bin/bash
# Check imports at top of integration_test.go
head -30 integration_test.go | cat -n
# Check for context and time package imports
echo "---"
rg -n '^import|context|time' integration_test.go | head -20Repository: Kronus-Lab/iptracker
Length of output: 1805
🏁 Script executed:
#!/bin/bash
# Get full function implementations with more context
sed -n '395,420p' integration_test.go | cat -nRepository: Kronus-Lab/iptracker
Length of output: 910
🏁 Script executed:
#!/bin/bash
# Search for all exec.Command patterns in the entire file
rg -n 'exec\.Command' integration_test.goRepository: Kronus-Lab/iptracker
Length of output: 190
Add timeouts and fix error handling for spawned processes.
buildBinary and runBinary lack timeouts and can block indefinitely. More critically, runBinary ignores cmd.Run() errors and returns exit code 0 when ProcessState is nil (process failed to start), masking actual failures.
Proposed fix
+// also add: import "errors"
func buildBinary(t *testing.T) string {
t.Helper()
tmpDir := t.TempDir()
binary := filepath.Join(tmpDir, "iptracker")
- output, err := exec.Command("go", "build", "-o", binary, ".").CombinedOutput()
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
+ output, err := exec.CommandContext(ctx, "go", "build", "-o", binary, ".").CombinedOutput()
if err != nil {
t.Fatalf("failed to build binary: %v\n%s", err, output)
}
return binary
}
func runBinary(t *testing.T, binary string, args ...string) (string, int) {
t.Helper()
- cmd := exec.Command(binary, args...)
+ ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+ defer cancel()
+ cmd := exec.CommandContext(ctx, binary, args...)
var stderr bytes.Buffer
cmd.Stderr = &stderr
- cmd.Run()
- exitCode := 0
- if cmd.ProcessState != nil {
- exitCode = cmd.ProcessState.ExitCode()
+ err := cmd.Run()
+ if err != nil {
+ var exitErr *exec.ExitError
+ if !errors.As(err, &exitErr) {
+ t.Fatalf("failed to start binary: %v", err)
+ }
}
- return stderr.String(), exitCode
+ if cmd.ProcessState == nil {
+ t.Fatal("process exited without process state")
+ }
+ return stderr.String(), cmd.ProcessState.ExitCode()
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integration_test.go` around lines 396 - 417, buildBinary and runBinary can
hang and currently swallow process errors; update both to use a context with a
short timeout (e.g., context.WithTimeout) when creating the command so
builds/runs cannot block forever, and propagate errors instead of masking them.
In buildBinary replace exec.Command with exec.CommandContext and on error
include the captured output in t.Fatalf; in runBinary use exec.CommandContext,
capture stdout and stderr (or CombinedOutput), check the returned error from
cmd.Run/CombinedOutput, and derive a sensible exit code: if err is nil use
ProcessState.ExitCode(), if err is *exec.ExitError use
err.ProcessState.ExitCode(), and for non-start errors return a distinct non-zero
code (e.g., -1) and include the error text in the returned output so test
failures are observable; ensure ProcessState nil is not treated as success.
Comment on lines
+514
to
+540
| func TestIntegration_Client_Get(t *testing.T) { | ||
| recordsClient, _, _, cleanup := setupIntegration(t) | ||
| defer cleanup() | ||
|
|
||
| pdnsURL := envOrDefault("PDNS_URL", defaultPDNSURL) | ||
| pdnsKey := envOrDefault("PDNS_API_KEY", defaultPDNSKey) | ||
| waitForPowerDNS(t, pdnsKey, pdnsURL) | ||
|
|
||
| httpClient := newTestHTTPClient() | ||
| concreteClient := newRecordsClient(pdnsKey, pdnsURL, httpClient) | ||
|
|
||
| rrsets, err := concreteClient.Get(context.Background(), testZone, testRecord, powerdns.RRTypePtr(powerdns.RRTypeA)) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
| if len(rrsets) != 1 { | ||
| t.Fatalf("expected 1 rrset, got %d", len(rrsets)) | ||
| } | ||
| if len(rrsets[0].Records) != 1 || rrsets[0].Records[0].Content == nil { | ||
| t.Fatal("expected 1 record with non-nil content") | ||
| } | ||
| if *rrsets[0].Records[0].Content != testInitialIP { | ||
| t.Errorf("expected IP %s, got %s", testInitialIP, *rrsets[0].Records[0].Content) | ||
| } | ||
|
|
||
| _ = recordsClient | ||
| } |
There was a problem hiding this comment.
Decouple TestIntegration_Client_Get from ntfy setup.
TestIntegration_Client_Get currently depends on setupIntegration (and therefore ntfy health) even though it only validates PowerDNS Get. This adds unrelated flakiness, and recordsClient is unused.
Proposed direction
func TestIntegration_Client_Get(t *testing.T) {
- recordsClient, _, _, cleanup := setupIntegration(t)
- defer cleanup()
-
pdnsURL := envOrDefault("PDNS_URL", defaultPDNSURL)
pdnsKey := envOrDefault("PDNS_API_KEY", defaultPDNSKey)
waitForPowerDNS(t, pdnsKey, pdnsURL)
+ zone := "clientget.example.net."
+ record := "test." + zone
+ seedZone(t, pdnsKey, pdnsURL, zone)
+ defer deleteZone(t, pdnsKey, pdnsURL, zone)
+
httpClient := newTestHTTPClient()
concreteClient := newRecordsClient(pdnsKey, pdnsURL, httpClient)
+ seedRecord(t, concreteClient, zone, record, testInitialIP)
- rrsets, err := concreteClient.Get(context.Background(), testZone, testRecord, powerdns.RRTypePtr(powerdns.RRTypeA))
+ rrsets, err := concreteClient.Get(context.Background(), zone, record, powerdns.RRTypePtr(powerdns.RRTypeA))
...
-
- _ = recordsClient
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integration_test.go` around lines 514 - 540, TestIntegration_Client_Get is
calling setupIntegration (and getting recordsClient and cleanup) even though the
test only exercises PowerDNS Get, which adds an unnecessary ntfy dependency;
remove the call to setupIntegration and the unused recordsClient/cleanup, and
instead instantiate only what this test needs: create httpClient :=
newTestHTTPClient() and concreteClient := newRecordsClient(pdnsKey, pdnsURL,
httpClient), keep waitForPowerDNS(...) unchanged, then proceed with the existing
Get assertions; update the function to eliminate references to recordsClient and
cleanup so the test no longer depends on setupIntegration.
Comment on lines
+16
to
+17
| func getLiveIP(ctx context.Context, httpClient *http.Client, url string) (string, error) { | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) |
There was a problem hiding this comment.
Update stale endpoint wording in error path.
Line 25 still hardcodes ifconfig.me even though the endpoint is now injected via url; this can mislead troubleshooting.
Suggested patch
- return "", &IPCheckError{Err: fmt.Errorf("reach ifconfig.me: %w", err)}
+ return "", &IPCheckError{Err: fmt.Errorf("reach IP check endpoint: %w", err)}Also applies to: 25-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ip.go` around lines 16 - 17, In getLiveIP, the error message still mentions
the hardcoded "ifconfig.me" endpoint even though the endpoint is passed via the
url parameter; update the error returns (e.g., the fmt.Errorf/return error paths
in getLiveIP) to reference the url variable and include the underlying err (for
example: "failed to create request to %s: %w" or similar), and do the same for
any other error paths in getLiveIP that mention ifconfig.me so messages reflect
the actual injected url.
Comment on lines
+22
to
+23
| body, _ := io.ReadAll(r.Body) | ||
| json.Unmarshal(body, &receivedBody) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files notify_test.goRepository: Kronus-Lab/iptracker
Length of output: 79
🏁 Script executed:
# Find and examine notify_test.go
fd notify_test.go -type fRepository: Kronus-Lab/iptracker
Length of output: 235
🏁 Script executed:
# Check if notify_test.go exists and get line count
if [ -f "notify_test.go" ]; then
wc -l notify_test.go
echo "---"
# Show lines 20-25
sed -n '20,25p' notify_test.go
echo "---"
# Show lines 79-84
sed -n '79,84p' notify_test.go
echo "---"
# Show lines 241-245
sed -n '241,245p' notify_test.go
echo "---"
# Show lines 260-265
sed -n '260,265p' notify_test.go
fiRepository: Kronus-Lab/iptracker
Length of output: 931
🏁 Script executed:
# Look at the test functions containing these handlers
# For lines 22-23, find the parent test function
sed -n '1,25p' notify_test.go
# For lines 81-82
sed -n '70,85p' notify_test.go
# For lines 243
sed -n '235,250p' notify_test.go
# For lines 262-263
sed -n '250,270p' notify_test.goRepository: Kronus-Lab/iptracker
Length of output: 2398
Handle unchecked request body read/unmarshal errors in test handlers.
The handlers ignore io.ReadAll and json.Unmarshal errors at lines 22–23, 81–82, 243, and 262–263, which can cause tests to silently proceed with corrupted or empty payloads and trigger linter failures. Add explicit error handling in each handler to ensure requests with read/decode failures are rejected.
Suggested fixes
body, _ := io.ReadAll(r.Body)
json.Unmarshal(body, &receivedBody)
+ body, err := io.ReadAll(r.Body)
+ if err != nil {
+ t.Errorf("failed to read request body: %v", err)
+ w.WriteHeader(http.StatusBadRequest)
+ return
+ }
+ if err := json.Unmarshal(body, &receivedBody); err != nil {
+ t.Errorf("failed to decode discord payload: %v", err)
+ w.WriteHeader(http.StatusBadRequest)
+ return
+ } body, _ := io.ReadAll(r.Body)
receivedBody = string(body)
+ body, err := io.ReadAll(r.Body)
+ if err != nil {
+ t.Errorf("failed to read request body: %v", err)
+ w.WriteHeader(http.StatusBadRequest)
+ return
+ }
+ receivedBody = string(body) io.ReadAll(r.Body)
+ if _, err := io.ReadAll(r.Body); err != nil {
+ t.Errorf("failed to read request body: %v", err)
+ w.WriteHeader(http.StatusBadRequest)
+ return
+ } body, _ := io.ReadAll(r.Body)
bodyStr = string(body)
+ body, err := io.ReadAll(r.Body)
+ if err != nil {
+ t.Errorf("failed to read request body: %v", err)
+ w.WriteHeader(http.StatusBadRequest)
+ return
+ }
+ bodyStr = string(body)Also applies to: 81–82, 243, 262–263
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 23-23: Error return value of json.Unmarshal is not checked
(errcheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@notify_test.go` around lines 22 - 23, In notify_test.go the test HTTP
handlers are ignoring errors from io.ReadAll and json.Unmarshal (seen around
uses of io.ReadAll, json.Unmarshal and the receivedBody variable); update each
handler to check the error returned by io.ReadAll and return an HTTP error
(e.g., http.Error with status 400) or fail the test when read fails, then check
json.Unmarshal's error and similarly reject the request if decoding fails,
ensuring every occurrence (the blocks around io.ReadAll/json.Unmarshal at the
shown spots) handles and surfaces the error instead of discarding it.
Comment on lines
+230
to
+236
| var discordErr, ntfyErr *NotificationError | ||
| if !errors.As(err, &discordErr) { | ||
| t.Fatalf("expected discord NotificationError in joined error, got %T", err) | ||
| } | ||
| if !errors.As(err, &ntfyErr) { | ||
| t.Fatalf("expected ntfy NotificationError in joined error, got %T", err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the test file
fd -t f "notify_test.go"Repository: Kronus-Lab/iptracker
Length of output: 79
🏁 Script executed:
# Read the notify_test.go file, focusing on lines around 230-236
wc -l notify_test.goRepository: Kronus-Lab/iptracker
Length of output: 83
🏁 Script executed:
# Read lines 220-245 to get context around the flagged lines
sed -n '220,245p' notify_test.goRepository: Kronus-Lab/iptracker
Length of output: 945
🏁 Script executed:
# Look for the sendNotifications function definition
grep -n "func sendNotifications" notify_test.goRepository: Kronus-Lab/iptracker
Length of output: 46
🏁 Script executed:
# Search for sendNotifications function in the codebase
rg "func sendNotifications" -t goRepository: Kronus-Lab/iptracker
Length of output: 175
🏁 Script executed:
# Read the sendNotifications function in notify.go
grep -n "func sendNotifications" notify.go -A 30Repository: Kronus-Lab/iptracker
Length of output: 616
🏁 Script executed:
# Search for NotificationError type definition
rg "type NotificationError" -t go -A 5Repository: Kronus-Lab/iptracker
Length of output: 235
🏁 Script executed:
# Check what notifyDiscord and notifyNtfy return
rg "func notifyDiscord|func notifyNtfy" -t go -A 10Repository: Kronus-Lab/iptracker
Length of output: 1221
🏁 Script executed:
# Let me verify how errors.As() works with errors.Join()
# Create a test to understand the behavior
cat > /tmp/test_errors.go << 'EOF'
package main
import (
"errors"
"fmt"
)
type NotificationError struct {
Service string
Err error
}
func (e *NotificationError) Error() string {
return fmt.Sprintf("%s: %v", e.Service, e.Err)
}
func main() {
// Simulate the test scenario
discordErr := &NotificationError{Service: "discord", Err: errors.New("discord failed")}
ntfyErr := &NotificationError{Service: "ntfy", Err: errors.New("ntfy failed")}
// This is what sendNotifications returns
joinedErr := errors.Join(discordErr, ntfyErr)
// This is what the test does
var extractedDiscord, extractedNtfy *NotificationError
if errors.As(joinedErr, &extractedDiscord) {
fmt.Printf("First As found: %s\n", extractedDiscord.Service)
}
if errors.As(joinedErr, &extractedNtfy) {
fmt.Printf("Second As found: %s\n", extractedNtfy.Service)
}
// Check if they're the same object
fmt.Printf("Are they the same object? %v\n", extractedDiscord == extractedNtfy)
fmt.Printf("Discord Service: %s, Ntfy Service: %s\n", extractedDiscord.Service, extractedNtfy.Service)
}
EOF
# Run with Go to understand error behavior
go run /tmp/test_errors.goRepository: Kronus-Lab/iptracker
Length of output: 192
🏁 Script executed:
# Let me check what test function this is part of
sed -n '200,236p' notify_test.goRepository: Kronus-Lab/iptracker
Length of output: 1201
errors.As twice doesn't verify both joined notification failures.
When called on a joined error, errors.As() finds and returns the first matching error type. Both errors.As() calls will extract the same first *NotificationError, so the test passes even if only one service actually failed. This test needs to unwrap all errors in the joined error and verify that both "discord" and "ntfy" services are represented.
Suggested fix
- var discordErr, ntfyErr *NotificationError
- if !errors.As(err, &discordErr) {
- t.Fatalf("expected discord NotificationError in joined error, got %T", err)
- }
- if !errors.As(err, &ntfyErr) {
- t.Fatalf("expected ntfy NotificationError in joined error, got %T", err)
- }
+ var joined interface{ Unwrap() []error }
+ if !errors.As(err, &joined) {
+ t.Fatalf("expected joined error, got %T", err)
+ }
+
+ services := map[string]bool{}
+ for _, e := range joined.Unwrap() {
+ var nerr *NotificationError
+ if errors.As(e, &nerr) {
+ services[nerr.Service] = true
+ }
+ }
+ if !services["discord"] || !services["ntfy"] {
+ t.Fatalf("expected both discord and ntfy NotificationError entries, got services=%v", services)
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@notify_test.go` around lines 230 - 236, The test currently calls
errors.As(err, &discordErr) and errors.As(err, &ntfyErr) which will both match
the same first *NotificationError; instead, walk the joined/chain of errors
starting from err (using errors.Unwrap in a loop or errors.As in a loop over
each unwrapped error) and collect all *NotificationError instances, then assert
that one has Service == "discord" and one has Service == "ntfy"; update the
assertions to check the collected slice/map of NotificationError.Service values
rather than calling errors.As twice on the original err.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @mdugas. The following files were modified: * `ip.go` * `main.go` These files were ignored: * `cycle_test.go` * `errors_test.go` * `integration_test.go` * `ip_test.go` * `main_test.go` * `notify_test.go` * `rrset_test.go` * `update_test.go` These file types are not supported: * `docker-compose.integration.yml`
…into tests/add-coverage
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dockerfile (1)
1-4: Drop redundant-ldflags "-extldflags '-static'"whenCGO_ENABLED=0
golang:1.26(tag1.26) is available on Docker Hub, so the base image tag shouldn’t break CI.- With
CGO_ENABLED=0,-ldflags "-extldflags '-static'"is redundant (no external C linker is used), so it can be removed as a minor cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dockerfile` around lines 1 - 4, Remove the redundant static linker flags from the build step: in the Dockerfile update the RUN command that sets CGO_ENABLED=0 and runs go build (which produces the iptracker binary) to omit -ldflags "-extldflags '-static'"; keep CGO_ENABLED=0 and the rest of the build invocation (go mod download && CGO_ENABLED=0 go build -a -o iptracker -v ./...) so the build remains static without the extra ldflags.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@dockerfile`:
- Around line 1-4: Remove the redundant static linker flags from the build step:
in the Dockerfile update the RUN command that sets CGO_ENABLED=0 and runs go
build (which produces the iptracker binary) to omit -ldflags "-extldflags
'-static'"; keep CGO_ENABLED=0 and the rest of the build invocation (go mod
download && CGO_ENABLED=0 go build -a -o iptracker -v ./...) so the build
remains static without the extra ldflags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8c68ce1-d15e-43b3-8e67-e398756504be
📒 Files selected for processing (3)
containerfs/etc/passwddocker-compose.example.ymldockerfile
✅ Files skipped from review due to trivial changes (2)
- docker-compose.example.yml
- containerfs/etc/passwd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit