test: add nil-kind coverage for IsNilObject (#352)#353
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can customize the tone of the review comments and chat replies.Configure the |
Contributor
|
感谢您提出Pull Request，我会尽快Review。我会在1-2日内进行查看或者回复，如果遇到节假日可能会处理较慢，敬请谅解。 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands test coverage around nil-detection and YAML array parsing, and updates runtime behavior to better handle YAML-parsed slice types and 304-not-modified sync responses. It also includes widespread license-header normalization and some README/.gitignore updates across the repo.
Changes:
- Add tests for
IsNilObjectacross additional nil-able kinds (map/slice/chan/func/ptr) and add YAML array parsing tests. - Extend
storage.Configslice getters to support values parsed as[]interface{}(common from YAML/Viper). - Fix async sync behavior to acknowledge notifications even when config fetch returns HTTP 304 by making
SyncWithNamespacereturn an error value and updating notification IDs on successful responses.
Reviewed changes
Copilot reviewed 80 out of 81 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/utils_test.go | Adds coverage for IsNilObject with more nil-able kinds. |
| utils/utils.go | Comment/header formatting updates; nil detection logic unchanged. |
| utils/parse/yml/parser_test.go | Adds YAML array parsing tests for the yml parser. |
| utils/parse/yml/parser.go | Import formatting only. |
| utils/parse/yaml/parser_test.go | Adds YAML array parsing tests for the yaml parser. |
| utils/parse/yaml/parser.go | Import formatting only. |
| utils/parse/properties/parser_test.go | License header normalization. |
| utils/parse/properties/parser.go | License header normalization. |
| utils/parse/parser.go | License header normalization and comment formatting. |
| utils/parse/normal/parser_test.go | License header normalization. |
| utils/parse/normal/parser.go | License header normalization. |
| storage/repository_test.go | Adds tests for slice conversion behavior (YAML []interface{} cases). |
| storage/repository.go | Adds []interface{}→[]string/[]int conversions in slice getters. |
| storage/event_dispatch_test.go | License header normalization. |
| storage/event_dispatch.go | License header normalization. |
| storage/change_event_test.go | License header normalization. |
| storage/change_event.go | Comment formatting updates. |
| start_test.go | Import ordering tweak and license header normalization. |
| start.go | Comment formatting updates. |
| protocol/http/request_test.go | Import reordering/cleanup and license header normalization. |
| protocol/http/request_server_test.go | Minor formatting and license header normalization. |
| protocol/http/request.go | Import ordering tweak and license header normalization. |
| protocol/auth/sign/sign_test.go | License header normalization. |
| protocol/auth/sign/sign.go | License header normalization. |
| protocol/auth/auth.go | License header normalization. |
| mock_server_test.go | Comment formatting updates. |
| extension/sign.go | License header normalization. |
| extension/sgin_test.go | License header normalization. |
| extension/load_balance_test.go | Import ordering and comment formatting. |
| extension/load_balance.go | Comment formatting updates. |
| extension/format_parser_test.go | Import ordering and license header normalization. |
| extension/format_parser.go | License header normalization. |
| extension/file_test.go | Import ordering and license header normalization. |
| extension/file.go | Comment formatting updates. |
| extension/cache_test.go | Import ordering and comment formatting updates. |
| extension/cache.go | Comment formatting updates. |
| env/server/server_test.go | Import ordering and license header normalization. |
| env/server/server.go | License header normalization. |
| env/request_config.go | Comment formatting updates. |
| env/file/json/raw_test.go | Import ordering and license header normalization. |
| env/file/json/raw.go | Import ordering and comment formatting updates. |
| env/file/json/json_test.go | Import ordering and license header normalization. |
| env/file/json/json.go | Import ordering and comment formatting updates. |
| env/file/file_handler.go | Comment formatting updates. |
| env/config/json/json_config_test.go | Import ordering and license header normalization. |
| env/config/json/json_config.go | License header normalization. |
| env/config/config_test.go | Import ordering and license header normalization. |
| env/config/config.go | License header normalization. |
| env/config/apollo_config_test.go | Import ordering and license header normalization. |
| env/config/apollo_config.go | Comment formatting updates. |
| env/app_config_test.go | Import ordering and license header normalization. |
| env/app_config.go | Comment formatting updates. |
| constant/config_file_format.go | Comment formatting updates. |
| component/serverlist/sync_test.go | Import ordering and license header normalization. |
| component/serverlist/sync.go | Import ordering and license header normalization. |
| component/serverlist/app_config_server_test.go | Comment formatting updates. |
| component/remote/sync_test.go | Import ordering and comment formatting updates. |
| component/remote/sync.go | Updates sync calls to match new SyncWithNamespace signature. |
| component/remote/remote.go | Changes ApolloConfig.SyncWithNamespace to return (*config.ApolloConfig, error). |
| component/remote/async_test.go | Adds regression test for 304-not-modified notification acknowledgement. |
| component/remote/async.go | Updates notification ID on successful SyncWithNamespace (including 304). |
| component/remote/abs.go | Implements new SyncWithNamespace signature and propagates request errors. |
| component/notify/mock_server_test.go | License header normalization. |
| component/notify/componet_notify_test.go | License header normalization. |
| component/notify/componet_notify.go | Import ordering and license header normalization. |
| component/notify/change_event_test.go | Import ordering and license header normalization. |
| component/log/log_test.go | License header normalization. |
| component/log/log.go | Comment formatting updates. |
| component/common_test.go | Import ordering and license header normalization. |
| component/common.go | License header normalization. |
| cluster/roundrobin/round_robin_test.go | Import ordering and license header normalization. |
| cluster/roundrobin/round_robin.go | Comment formatting updates. |
| cluster/load_balance.go | Comment formatting updates. |
| client_test.go | Updates mocks for new SyncWithNamespace signature; import reordering. |
| client.go | Updates call sites for new SyncWithNamespace return values. |
| agcache/memory/memory_test.go | Import ordering and license header normalization. |
| agcache/memory/memory.go | Comment formatting updates. |
| agcache/cache.go | Comment formatting updates. |
| README_CN.md | Adds Chinese README. |
| README.md | Adds language toggle and English content cleanup. |
| .gitignore | Ignores test-generated JSON artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
24
to
30
| "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| _ "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| "github.com/apolloconfig/agollo/v4/env" | ||
| "github.com/apolloconfig/agollo/v4/env/config" | ||
| _ "github.com/apolloconfig/agollo/v4/env/file/json" | ||
| jsonFile "github.com/apolloconfig/agollo/v4/env/file/json" | ||
| "github.com/apolloconfig/agollo/v4/extension" |
Comment on lines
+27
to
+31
| _ "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| "github.com/apolloconfig/agollo/v4/cluster/roundrobin" | ||
| "github.com/apolloconfig/agollo/v4/component/remote" | ||
| "github.com/apolloconfig/agollo/v4/env/config" | ||
| _ "github.com/apolloconfig/agollo/v4/env/file/json" |
| "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| "github.com/apolloconfig/agollo/v4/component/remote" | ||
| "github.com/apolloconfig/agollo/v4/env/config" | ||
| _ "github.com/apolloconfig/agollo/v4/env/file/json" |
| CallBack(namespace string) http.CallBack | ||
| // SyncWithNamespace 通过 namespace 同步 apollo 配置 | ||
| SyncWithNamespace(namespace string, appConfigFunc func() config.AppConfig) *config.ApolloConfig | ||
| SyncWithNamespace(namespace string, appConfigFunc func() config.AppConfig) (*config.ApolloConfig, error) |
Comment on lines
+170
to
+176
| // 尝试直接转换为 []string | ||
| if v, ok := value.([]string); ok { | ||
| return v | ||
| } | ||
| return v | ||
|
|
||
| // 尝试从 []interface{} 转换为 []string (YAML 数组解析后的类型) | ||
| if ifaceSlice, ok := value.([]interface{}); ok { |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR broadens test coverage around nil detection and YAML array parsing, extends config slice getters to handle YAML-parsed []interface{} values, and adjusts remote sync behavior/API to prevent repeated notifications on 304 responses. It also includes widespread license-header normalization plus README and .gitignore updates.
Changes:
- Add tests for
IsNilObjectacross nil-able kinds, and YAML/YML array parsing. - Extend
Configslice getters ([]string/[]int) to convert from[]interface{}(common from YAML parsing) and add corresponding repository tests. - Change remote
SyncWithNamespaceAPI to return(config, error)and update notification IDs on successful syncs (including 304), plus add a regression test for the 304 loop scenario.
Reviewed changes
Copilot reviewed 80 out of 81 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/utils_test.go | Add IsNilObject nil-kind tests; update header. |
| utils/utils.go | Comment/header normalization (no logic change). |
| utils/parse/yml/parser_test.go | Add YML array parsing test; import order/header. |
| utils/parse/yml/parser.go | Import order/header normalization. |
| utils/parse/yaml/parser_test.go | Add YAML array parsing test; import order/header. |
| utils/parse/yaml/parser.go | Import order/header normalization. |
| utils/parse/properties/parser_test.go | Header normalization. |
| utils/parse/properties/parser.go | Header normalization. |
| utils/parse/parser.go | Comment/header normalization. |
| utils/parse/normal/parser_test.go | Header normalization. |
| utils/parse/normal/parser.go | Header normalization. |
| storage/repository_test.go | Add slice-conversion tests (YAML []interface{} cases). |
| storage/repository.go | Convert []interface{} to []string/[]int in getters. |
| storage/event_dispatch_test.go | Header normalization. |
| storage/event_dispatch.go | Header normalization. |
| storage/change_event_test.go | Header normalization. |
| storage/change_event.go | Comment/header normalization. |
| start_test.go | Import order/header normalization. |
| start.go | Comment/header normalization. |
| protocol/http/request_test.go | Import order/header normalization. |
| protocol/http/request_server_test.go | Minor formatting/header normalization. |
| protocol/http/request.go | Import order/header normalization. |
| protocol/auth/sign/sign_test.go | Header normalization. |
| protocol/auth/sign/sign.go | Header normalization. |
| protocol/auth/auth.go | Header normalization. |
| mock_server_test.go | Comment/header normalization. |
| extension/sign.go | Header normalization. |
| extension/sgin_test.go | Header normalization. |
| extension/load_balance_test.go | Import order/comment normalization. |
| extension/load_balance.go | Comment/header normalization. |
| extension/format_parser_test.go | Import order/header normalization. |
| extension/format_parser.go | Header normalization. |
| extension/file_test.go | Import order/header normalization. |
| extension/file.go | Comment/header normalization. |
| extension/cache_test.go | Import order/comment normalization. |
| extension/cache.go | Comment/header normalization. |
| env/server/server_test.go | Import order/header normalization. |
| env/server/server.go | Header normalization. |
| env/request_config.go | Comment/header normalization. |
| env/file/json/raw_test.go | Import order/header normalization. |
| env/file/json/raw.go | Import order/comment normalization. |
| env/file/json/json_test.go | Import order/header normalization. |
| env/file/json/json.go | Import order/comment normalization. |
| env/file/file_handler.go | Comment/header normalization. |
| env/config/json/json_config_test.go | Import order/header normalization. |
| env/config/json/json_config.go | Header normalization. |
| env/config/config_test.go | Import order/header normalization. |
| env/config/config.go | Header normalization. |
| env/config/apollo_config_test.go | Import order/header normalization. |
| env/config/apollo_config.go | Comment/header normalization. |
| env/app_config_test.go | Import order/header normalization. |
| env/app_config.go | Comment/header normalization. |
| constant/config_file_format.go | Comment/header normalization. |
| component/serverlist/sync_test.go | Import order/header normalization. |
| component/serverlist/sync.go | Import order/header normalization. |
| component/serverlist/app_config_server_test.go | Comment/header normalization. |
| component/remote/sync_test.go | Import order/comment normalization. |
| component/remote/sync.go | Adapt to new SyncWithNamespace signature. |
| component/remote/remote.go | Change interface method signature to return error. |
| component/remote/async_test.go | Add regression test for 304 notification loop. |
| component/remote/async.go | Update notify ID on successful syncs (incl. 304). |
| component/remote/abs.go | Return errors from SyncWithNamespace. |
| component/notify/mock_server_test.go | Header normalization. |
| component/notify/componet_notify_test.go | Header normalization. |
| component/notify/componet_notify.go | Import order/header normalization. |
| component/notify/change_event_test.go | Import order/header normalization. |
| component/log/log_test.go | Header normalization. |
| component/log/log.go | Comment/header normalization. |
| component/common_test.go | Import order/header normalization. |
| component/common.go | Header normalization. |
| cluster/roundrobin/round_robin_test.go | Import order/header normalization. |
| cluster/roundrobin/round_robin.go | Comment/header normalization. |
| cluster/load_balance.go | Comment/header normalization. |
| client_test.go | Update mocks for new SyncWithNamespace signature. |
| client.go | Adapt call site to new SyncWithNamespace signature. |
| agcache/memory/memory_test.go | Import order/header normalization. |
| agcache/memory/memory.go | Comment/header normalization. |
| agcache/cache.go | Comment/header normalization. |
| README_CN.md | Add Chinese README. |
| README.md | Add language switch + English rewrite/example tweaks. |
| .gitignore | Ignore additional test-generated JSON artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
24
to
+26
| "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| _ "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| "github.com/apolloconfig/agollo/v4/env" |
Comment on lines
+413
to
+420
| // JSON/YAML 数字默认解析为 float64 | ||
| // 验证值是否为整数（无小数部分） | ||
| if v != float64(int(v)) { | ||
| log.Debugf("convert to []int fail! float64 value has fractional part: %v", v) | ||
| return defaultValue | ||
| } | ||
| result = append(result, int(v)) | ||
| case string: |
Comment on lines
+170
to
+176
| // 尝试直接转换为 []string | ||
| if v, ok := value.([]string); ok { | ||
| return v | ||
| } | ||
| return v | ||
|
|
||
| // 尝试从 []interface{} 转换为 []string (YAML 数组解析后的类型) | ||
| if ifaceSlice, ok := value.([]interface{}); ok { |
| Check out our [Wiki](https://github.com/apolloconfig/agollo/wiki) or [godoc](http://godoc.org/github.com/zouyx/agollo) for more information. | ||
|
|
||
| 如果你觉得该工具还不错或者有问题，一定要让我知道，可以发邮件或者[留言](https://github.com/apolloconfig/agollo/issues)。 | ||
| If you find this tool useful or encounter any issues, please let me know via email or by [creating an issue](https://github.com/apolloconfig/agollo/issues)。 |
Comment on lines
27
to
29
| "github.com/apolloconfig/agollo/v4/env/config" | ||
| _ "github.com/apolloconfig/agollo/v4/env/file/json" | ||
| jsonFile "github.com/apolloconfig/agollo/v4/env/file/json" |
| . "github.com/tevid/gohamcrest" | ||
|
|
||
| "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| _ "github.com/apolloconfig/agollo/v4/agcache/memory" |
Comment on lines
+27
to
+31
| _ "github.com/apolloconfig/agollo/v4/agcache/memory" | ||
| "github.com/apolloconfig/agollo/v4/cluster/roundrobin" | ||
| "github.com/apolloconfig/agollo/v4/component/remote" | ||
| "github.com/apolloconfig/agollo/v4/env/config" | ||
| _ "github.com/apolloconfig/agollo/v4/env/file/json" |
Comment on lines
+211
to
+218
| case float64: | ||
| // JSON/YAML 数字默认解析为 float64 | ||
| // 验证值是否为整数（无小数部分） | ||
| if v != float64(int(v)) { | ||
| log.Debugf("convert to []int fail! float64 value has fractional part: %v", v) | ||
| return defaultValue | ||
| } | ||
| result = append(result, int(v)) |
Comment on lines
+33
to
34
| SyncWithNamespace(namespace string, appConfigFunc func() config.AppConfig) (*config.ApolloConfig, error) | ||
| } |
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
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.
No description provided.