Add Makefile lint target and device settings model with validation to smsgateway #23
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
🚀 New features to boost your workflow:
|
coderabbitai
bot
changed the title
WalkthroughA new Makefile linting target was introduced, and the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Device
participant Gateway
User->>Device: Update device settings (via API/UI)
Device->>Gateway: Send PushNotification (Event: SettingsUpdated)
Gateway->>Device: (optional) Acknowledge or process updated settings
sequenceDiagram
participant Developer
participant Makefile
Developer->>Makefile: make lint
Makefile->>Makefile: Run golangci-lint run -v
Makefile-->>Developer: Output lint results
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
smsgateway/domain_settings.go (7)
1-10: Exported Struct Missing GoDoc Comment
The top‐levelDeviceSettingsstruct aggregates all device configuration categories but lacks a GoDoc comment. Please add a descriptive comment above the type to improve API discoverability.
12-14: Add Validation & Documentation for Encryption Passphrase
SettingsEncryption.Passphrasecurrently only hasomitempty. Consider enforcing a minimum length (e.g.validate:"omitempty,min=1") to prevent empty strings, and add a GoDoc comment describing expected passphrase semantics.
16-19: Verifyhttp_urlValidation Tag & Add Field Docs
Thevalidate:"omitempty,http_url"tag may not be built in to some validator libraries (which typically useurl). Confirm thathttp_urlis registered or switch tovalidate:"omitempty,url". Also add GoDoc comments forSettingsGatewayand its fields.
21-28: Define Typed Enums for Message Settings
Extract the raw string lists inLimitPeriodandSimSelectionModeinto Goconstgroups (typed enums). This prevents typos, centralizes valid values, and ensures theoneoftag stays in sync.
30-32: Add GoDoc for Ping Settings Struct
SettingsPingis clear in code but should include a GoDoc comment explaining what “ping” means in context and any recommended value ranges.
34-36: Add GoDoc for Logs Settings Struct
Please add a documentation comment forSettingsLogs, describing howLifetimeDaysis applied (e.g., rolling window, archive policy, etc.).
38-42: Enhance Webhooks Settings Validation & Docs
- Consider adding
validate:"omitempty,min=1"onSigningKeyto prevent empty but non-nil strings.- Add GoDoc comments for
SettingsWebhooksand its fields to clarify behavior whenInternetRequiredis false or absent.smsgateway/domain_upstream.go (1)
23-23: Update Validation Tag for New Event
Theoneoflist now includesSettingsUpdated. To avoid manual drift, consider deriving valid event names from thePushEventTypeconstants (e.g., via a helper or code‐generated tag) rather than hard-coding strings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Makefile(1 hunks)smsgateway/domain_settings.go(1 hunks)smsgateway/domain_upstream.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
Makefile (1)
29-31: Add Lint Target
The newlinttarget runninggolangci-lintis a valuable addition to enforce code quality across the Go modules.smsgateway/domain_upstream.go (1)
14-15: Add New Push Event Type
Good addition ofPushSettingsUpdatedto represent settings changes in the upstream event system.
|
Caution No docstrings were generated. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
smsgateway/domain_settings.go (1)
1-119: Consider adding a constructor function for DeviceSettingsThe settings structure is well-designed, but creating instances might be error-prone.
Consider adding a constructor function with sensible defaults:
// NewDeviceSettings creates a new DeviceSettings instance with default values func NewDeviceSettings() *DeviceSettings { defaultLimitPeriod := Disabled defaultSimMode := OSDefault return &DeviceSettings{ Messages: &SettingsMessages{ SendIntervalMin: intPtr(1), SendIntervalMax: intPtr(5), LimitPeriod: &defaultLimitPeriod, LimitValue: intPtr(100), SimSelectionMode: &defaultSimMode, LogLifetimeDays: intPtr(30), }, Ping: &SettingsPing{ IntervalSeconds: intPtr(300), // 5 minutes }, Logs: &SettingsLogs{ LifetimeDays: intPtr(30), }, Webhooks: &SettingsWebhooks{ InternetRequired: boolPtr(true), RetryCount: intPtr(3), }, } } // Helper functions func intPtr(i int) *int { return &i } func boolPtr(b bool) *bool { return &b }🧰 Tools
🪛 GitHub Check: Lint
[failure] 86-86:
The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)🪛 GitHub Actions: Go
[error] 86-86: golangci-lint: The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
smsgateway/domain_settings.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Lint
smsgateway/domain_settings.go
[failure] 86-86:
The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)
🪛 GitHub Actions: Go
smsgateway/domain_settings.go
[error] 86-86: golangci-lint: The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (8)
smsgateway/domain_settings.go (8)
3-15: LimitPeriod enum looks goodThe enum definition for message sending limits is well-structured with clear documentation for each constant value.
17-27: SimSelectionMode enum looks goodThe enum definition for SIM selection modes is well-structured with clear documentation for each constant value.
29-48: DeviceSettings structure is well-organizedThe main settings structure is well-designed with:
- Clear documentation for each field
- Appropriate use of pointers for optional fields
- Proper JSON tags with omitempty for nil values
This approach allows for flexible configuration with clear serialization behavior.
50-54: SettingsEncryption structure looks goodThe encryption settings structure is appropriately designed with an optional passphrase field.
56-64: SettingsGateway structure has appropriate validationThe gateway settings include proper HTTP URL validation for the CloudURL field when provided.
93-98: SettingsPing structure looks goodThe ping settings structure has appropriate validation for the interval field.
100-105: SettingsLogs structure looks goodThe logs settings structure has appropriate validation for the lifetime field.
107-118: SettingsWebhooks structure looks goodThe webhooks settings structure has appropriate fields and validation for retry count.
|
|
||
| // SimSelectionMode defines how SIM cards are selected for sending messages. | ||
| // Valid values are "OSDefault", "RoundRobin", or "Random". | ||
| SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" validate:"omitempty,oneof=OSDefault RoundRobin Random"` |
There was a problem hiding this comment.
Line exceeds maximum length limit
This line is 128 characters, exceeding the 120-character limit enforced by linters.
Split the validation tag onto a new line or shorten it:
- SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" validate:"omitempty,oneof=OSDefault RoundRobin Random"`
+ SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty"
+ validate:"omitempty,oneof=OSDefault RoundRobin Random"`Or alternatively:
- SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" validate:"omitempty,oneof=OSDefault RoundRobin Random"`
+ SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty"
+ validate:"omitempty,enum"`And define a custom validator for the enum type.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Lint
[failure] 86-86:
The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)
🪛 GitHub Actions: Go
[error] 86-86: golangci-lint: The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)
🤖 Prompt for AI Agents
In smsgateway/domain_settings.go at line 86, the line exceeds the 120-character
limit due to a long validation tag. To fix this, split the struct tag onto
multiple lines by placing the validation tag on a new line or shorten the
validation string. Alternatively, define a custom validator for the
SimSelectionMode enum type and reference it in the tag to reduce length.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
smsgateway/domain_settings.go (1)
96-96: Line exceeds maximum length limitThis line is 128 characters, exceeding the 120-character limit enforced by linters.
🧹 Nitpick comments (1)
smsgateway/domain_settings.go (1)
53-58: Add test coverage for validation methods.The validation methods are critical for ensuring data integrity but currently lack test coverage. Consider adding comprehensive unit tests that cover:
DeviceSettings.Validate()- test with valid/invalid MessagesSettingsMessages.Validate()- test cross-field validation scenarios:
- Valid: SendIntervalMax >= SendIntervalMin
- Invalid: SendIntervalMax < SendIntervalMin
- Edge cases: nil values, equal values
Would you like me to generate comprehensive unit tests for these validation methods?
Also applies to: 103-109
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 53-57: smsgateway/domain_settings.go#L53-L57
Added lines #L53 - L57 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
smsgateway/domain_settings.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
smsgateway/domain_settings.go
[warning] 53-57: smsgateway/domain_settings.go#L53-L57
Added lines #L53 - L57 were not covered by tests
[warning] 103-106: smsgateway/domain_settings.go#L103-L106
Added lines #L103 - L106 were not covered by tests
[warning] 108-108: smsgateway/domain_settings.go#L108
Added line #L108 was not covered by tests
🔇 Additional comments (5)
smsgateway/domain_settings.go (5)
1-4: LGTM! Good use of nolint directive.The global nolint for line length is appropriate here since validation tags can legitimately exceed line length limits.
6-30: Well-defined enum types with clear documentation.Both
LimitPeriodandSimSelectionModeenums are properly structured with descriptive constants and good documentation.
103-109: Excellent cross-field validation implementation.The custom validation method properly ensures that
SendIntervalMaxis not less thanSendIntervalMin, addressing the constraint mentioned in the documentation. Good use of error wrapping withErrValidationFailed.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 103-106: smsgateway/domain_settings.go#L103-L106
Added lines #L103 - L106 were not covered by tests
[warning] 108-108: smsgateway/domain_settings.go#L108
Added line #L108 was not covered by tests
60-64: Simple and clear encryption settings structure.The
SettingsEncryptionstruct is well-documented with a clear approach to enabling/disabling encryption based on passphrase presence.
111-136: Well-structured settings for ping, logs, and webhooks.All three structs (
SettingsPing,SettingsLogs,SettingsWebhooks) have appropriate validation constraints and clear documentation. The use of pointer types withomitemptyprovides good flexibility for optional configuration.
| func (s DeviceSettings) Validate() error { | ||
| if s.Messages != nil { | ||
| return s.Messages.Validate() | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Incomplete validation in DeviceSettings.Validate() method.
The validation method only checks the Messages field, but other fields like Gateway have validation constraints that should also be validated. Consider validating all non-nil fields with validation requirements.
Apply this diff to improve the validation:
func (s DeviceSettings) Validate() error {
+ if s.Gateway != nil {
+ // Gateway validation is handled by struct tags
+ }
if s.Messages != nil {
- return s.Messages.Validate()
+ if err := s.Messages.Validate(); err != nil {
+ return err
+ }
}
+ // Add validation for other fields as needed
return nil
}Note: If you're using a validation library like go-playground/validator, you might want to integrate that here to validate struct tags across all fields.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (s DeviceSettings) Validate() error { | |
| if s.Messages != nil { | |
| return s.Messages.Validate() | |
| } | |
| return nil | |
| } | |
| func (s DeviceSettings) Validate() error { | |
| if s.Gateway != nil { | |
| // Gateway validation is handled by struct tags | |
| } | |
| if s.Messages != nil { | |
| if err := s.Messages.Validate(); err != nil { | |
| return err | |
| } | |
| } | |
| // Add validation for other fields as needed | |
| return nil | |
| } |
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 53-57: smsgateway/domain_settings.go#L53-L57
Added lines #L53 - L57 were not covered by tests
🤖 Prompt for AI Agents
In smsgateway/domain_settings.go around lines 53 to 58, the Validate() method
only validates the Messages field but ignores other fields like Gateway that
also require validation. Update the method to validate all relevant fields that
have validation constraints, checking each non-nil field and returning any
validation errors found. If using a validation library like
go-playground/validator, integrate it to validate struct tags across all fields
for a more comprehensive and maintainable approach.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
smsgateway/domain_settings_test.go (1)
36-36: Remove placeholder comments or replace with specific TODOs.The "Add more test cases as needed" comments should either be removed or replaced with specific TODO items that describe what test cases are missing.
- // Add more test cases as needed + // TODO: Add test cases for Encryption, Ping, Logs, and Webhooks validation- // Add more test cases as needed + // TODO: Add test cases for LimitPeriod, LimitValue, SimSelectionMode, and LogLifetimeDays validationAlso applies to: 82-82
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
smsgateway/domain_settings_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
smsgateway/domain_settings_test.go (1)
smsgateway/domain_settings.go (4)
DeviceSettings(33-48)SettingsMessages(64-88)LimitPeriod(7-7)SimSelectionMode(21-21)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
smsgateway/domain_settings_test.go (2)
1-7: Package structure and imports look good.The test package follows Go conventions by using the
_testsuffix for external testing, and the imports are appropriate.
94-97: Helper function is well-implemented.The
ptrhelper function is a clean and reusable utility for creating integer pointers in tests.
| func TestSettingsMessages_Validate(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| messages smsgateway.SettingsMessages | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "valid settings", | ||
| messages: smsgateway.SettingsMessages{ | ||
| SendIntervalMin: nil, | ||
| SendIntervalMax: nil, | ||
| LimitPeriod: nil, | ||
| LimitValue: nil, | ||
| SimSelectionMode: nil, | ||
| LogLifetimeDays: nil, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "invalid send interval", | ||
| messages: smsgateway.SettingsMessages{ | ||
| SendIntervalMin: ptr(1), | ||
| SendIntervalMax: ptr(0), // Invalid: should be >= 1 | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| { | ||
| name: "invalid send interval max < min", | ||
| messages: smsgateway.SettingsMessages{ | ||
| SendIntervalMin: ptr(2), | ||
| SendIntervalMax: ptr(1), // Invalid: max < min | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| // Add more test cases as needed | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if err := tt.messages.Validate(); (err != nil) != tt.wantErr { | ||
| t.Errorf("SettingsMessages.Validate() error = %v, wantErr %v", err, tt.wantErr) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Insufficient test coverage for SettingsMessages validation constraints.
The current tests only cover SendIntervalMin/Max validation but miss several important validation scenarios based on the struct tags in domain_settings.go:
Missing test cases for:
LimitPeriodvalidation (should test invalid enum values)LimitValuevalidation (should test values < 1)SimSelectionModevalidation (should test invalid enum values)LogLifetimeDaysvalidation (should test values < 1)- Edge cases for
SendIntervalMin(testing value < 1)
// Add more test cases as needed
+ {
+ name: "invalid send interval min",
+ messages: smsgateway.SettingsMessages{
+ SendIntervalMin: ptr(0), // Invalid: should be >= 1
+ },
+ wantErr: true,
+ },
+ {
+ name: "invalid limit value",
+ messages: smsgateway.SettingsMessages{
+ LimitValue: ptr(0), // Invalid: should be >= 1
+ },
+ wantErr: true,
+ },
+ {
+ name: "invalid log lifetime days",
+ messages: smsgateway.SettingsMessages{
+ LogLifetimeDays: ptr(0), // Invalid: should be >= 1
+ },
+ wantErr: true,
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestSettingsMessages_Validate(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| messages smsgateway.SettingsMessages | |
| wantErr bool | |
| }{ | |
| { | |
| name: "valid settings", | |
| messages: smsgateway.SettingsMessages{ | |
| SendIntervalMin: nil, | |
| SendIntervalMax: nil, | |
| LimitPeriod: nil, | |
| LimitValue: nil, | |
| SimSelectionMode: nil, | |
| LogLifetimeDays: nil, | |
| }, | |
| wantErr: false, | |
| }, | |
| { | |
| name: "invalid send interval", | |
| messages: smsgateway.SettingsMessages{ | |
| SendIntervalMin: ptr(1), | |
| SendIntervalMax: ptr(0), // Invalid: should be >= 1 | |
| }, | |
| wantErr: true, | |
| }, | |
| { | |
| name: "invalid send interval max < min", | |
| messages: smsgateway.SettingsMessages{ | |
| SendIntervalMin: ptr(2), | |
| SendIntervalMax: ptr(1), // Invalid: max < min | |
| }, | |
| wantErr: true, | |
| }, | |
| // Add more test cases as needed | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| if err := tt.messages.Validate(); (err != nil) != tt.wantErr { | |
| t.Errorf("SettingsMessages.Validate() error = %v, wantErr %v", err, tt.wantErr) | |
| } | |
| }) | |
| } | |
| } | |
| func TestSettingsMessages_Validate(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| messages smsgateway.SettingsMessages | |
| wantErr bool | |
| }{ | |
| { | |
| name: "valid settings", | |
| messages: smsgateway.SettingsMessages{ | |
| SendIntervalMin: nil, | |
| SendIntervalMax: nil, | |
| LimitPeriod: nil, | |
| LimitValue: nil, | |
| SimSelectionMode: nil, | |
| LogLifetimeDays: nil, | |
| }, | |
| wantErr: false, | |
| }, | |
| { | |
| name: "invalid send interval", | |
| messages: smsgateway.SettingsMessages{ | |
| SendIntervalMin: ptr(1), | |
| SendIntervalMax: ptr(0), // Invalid: should be >= 1 | |
| }, | |
| wantErr: true, | |
| }, | |
| { | |
| name: "invalid send interval max < min", | |
| messages: smsgateway.SettingsMessages{ | |
| SendIntervalMin: ptr(2), | |
| SendIntervalMax: ptr(1), // Invalid: max < min | |
| }, | |
| wantErr: true, | |
| }, | |
| // Add more test cases as needed | |
| { | |
| name: "invalid send interval min", | |
| messages: smsgateway.SettingsMessages{ | |
| SendIntervalMin: ptr(0), // Invalid: should be >= 1 | |
| }, | |
| wantErr: true, | |
| }, | |
| { | |
| name: "invalid limit value", | |
| messages: smsgateway.SettingsMessages{ | |
| LimitValue: ptr(0), // Invalid: should be >= 1 | |
| }, | |
| wantErr: true, | |
| }, | |
| { | |
| name: "invalid log lifetime days", | |
| messages: smsgateway.SettingsMessages{ | |
| LogLifetimeDays: ptr(0), // Invalid: should be >= 1 | |
| }, | |
| wantErr: true, | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| if err := tt.messages.Validate(); (err != nil) != tt.wantErr { | |
| t.Errorf("SettingsMessages.Validate() error = %v, wantErr %v", err, tt.wantErr) | |
| } | |
| }) | |
| } | |
| } |
🤖 Prompt for AI Agents
In smsgateway/domain_settings_test.go between lines 48 and 92, the test coverage
for SettingsMessages.Validate is incomplete. Add test cases to cover validation
of LimitPeriod with invalid enum values, LimitValue with values less than 1,
SimSelectionMode with invalid enum values, LogLifetimeDays with values less than
1, and edge cases for SendIntervalMin with values less than 1. Each test case
should set the relevant field to an invalid value and assert that Validate
returns an error.
smsgateway/domain_settings_test.go
Outdated
| func TestDeviceSettings_Validate(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| settings smsgateway.DeviceSettings | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "nil messages", | ||
| settings: smsgateway.DeviceSettings{ | ||
| Messages: nil, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "valid messages", | ||
| settings: smsgateway.DeviceSettings{ | ||
| Messages: &smsgateway.SettingsMessages{ | ||
| SendIntervalMin: nil, | ||
| SendIntervalMax: nil, | ||
| LimitPeriod: nil, | ||
| LimitValue: nil, | ||
| SimSelectionMode: nil, | ||
| LogLifetimeDays: nil, | ||
| }, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| // Add more test cases as needed | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if err := tt.settings.Validate(); (err != nil) != tt.wantErr { | ||
| t.Errorf("DeviceSettings.Validate() error = %v, wantErr %v", err, tt.wantErr) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expand test coverage for DeviceSettings validation.
The current tests only cover the Messages field of DeviceSettings. Based on the struct definition in domain_settings.go, DeviceSettings has additional fields (Encryption, Ping, Logs, Webhooks) that should also be tested to ensure comprehensive validation coverage.
Consider adding test cases for:
- Non-nil values for other fields (Encryption, Ping, Logs, Webhooks)
- Invalid scenarios that should trigger validation errors
// Add more test cases as needed
+ {
+ name: "valid settings with all fields",
+ settings: smsgateway.DeviceSettings{
+ Encryption: &smsgateway.SettingsEncryption{},
+ Messages: &smsgateway.SettingsMessages{},
+ Ping: &smsgateway.SettingsPing{},
+ Logs: &smsgateway.SettingsLogs{},
+ Webhooks: &smsgateway.SettingsWebhooks{},
+ },
+ wantErr: false,
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestDeviceSettings_Validate(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| settings smsgateway.DeviceSettings | |
| wantErr bool | |
| }{ | |
| { | |
| name: "nil messages", | |
| settings: smsgateway.DeviceSettings{ | |
| Messages: nil, | |
| }, | |
| wantErr: false, | |
| }, | |
| { | |
| name: "valid messages", | |
| settings: smsgateway.DeviceSettings{ | |
| Messages: &smsgateway.SettingsMessages{ | |
| SendIntervalMin: nil, | |
| SendIntervalMax: nil, | |
| LimitPeriod: nil, | |
| LimitValue: nil, | |
| SimSelectionMode: nil, | |
| LogLifetimeDays: nil, | |
| }, | |
| }, | |
| wantErr: false, | |
| }, | |
| // Add more test cases as needed | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| if err := tt.settings.Validate(); (err != nil) != tt.wantErr { | |
| t.Errorf("DeviceSettings.Validate() error = %v, wantErr %v", err, tt.wantErr) | |
| } | |
| }) | |
| } | |
| } | |
| func TestDeviceSettings_Validate(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| settings smsgateway.DeviceSettings | |
| wantErr bool | |
| }{ | |
| { | |
| name: "nil messages", | |
| settings: smsgateway.DeviceSettings{ | |
| Messages: nil, | |
| }, | |
| wantErr: false, | |
| }, | |
| { | |
| name: "valid messages", | |
| settings: smsgateway.DeviceSettings{ | |
| Messages: &smsgateway.SettingsMessages{ | |
| SendIntervalMin: nil, | |
| SendIntervalMax: nil, | |
| LimitPeriod: nil, | |
| LimitValue: nil, | |
| SimSelectionMode: nil, | |
| LogLifetimeDays: nil, | |
| }, | |
| }, | |
| wantErr: false, | |
| }, | |
| // Add more test cases as needed | |
| { | |
| name: "valid settings with all fields", | |
| settings: smsgateway.DeviceSettings{ | |
| Encryption: &smsgateway.SettingsEncryption{}, | |
| Messages: &smsgateway.SettingsMessages{}, | |
| Ping: &smsgateway.SettingsPing{}, | |
| Logs: &smsgateway.SettingsLogs{}, | |
| Webhooks: &smsgateway.SettingsWebhooks{}, | |
| }, | |
| wantErr: false, | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| if err := tt.settings.Validate(); (err != nil) != tt.wantErr { | |
| t.Errorf("DeviceSettings.Validate() error = %v, wantErr %v", err, tt.wantErr) | |
| } | |
| }) | |
| } | |
| } |
🤖 Prompt for AI Agents
In smsgateway/domain_settings_test.go between lines 9 and 46, the test coverage
for DeviceSettings.Validate() only includes the Messages field. To improve
coverage, add test cases that set non-nil values for the other fields
Encryption, Ping, Logs, and Webhooks in DeviceSettings. Include both valid and
invalid scenarios for these fields to verify that validation errors are
correctly triggered or not. This will ensure comprehensive validation testing
for all fields in the DeviceSettings struct.
capcom6
force-pushed
the
settings/device-settings
branch
from
17246cf to
2e41cae
Compare
Summary by CodeRabbit
New Features
Chores