Conversation
- Add default values for API endpoints in configuration - Add default values for logging format, output, levels, and log token/message settings - Add tests to verify API and log configuration defaults and their override via environment variables - Improve tests to ensure config defaults are restored after modification - Add test coverage for logging config fallback to defaults when not specified - Add test to confirm all API endpoints have defined default values fix #854 Signed-off-by: appleboy <appleboy.tw@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds default values for API endpoints and logging configuration to fix issue #854, where missing defaults could cause initialization failures. The changes ensure that all API endpoints and logging settings have proper fallback values.
Key Changes
- Added default values in
setDefaults()for 7 API endpoints (push, stat_go, stat_app, config, sys_stat, metric, health URIs) - Added default values for logging configuration (format, access_log, access_level, error_log, error_level, hide_token, hide_messages)
- Added comprehensive test coverage to verify defaults work correctly and can be overridden via environment variables
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| config/config.go | Adds viper.SetDefault() calls for API endpoints and logging configuration to ensure defaults are always available |
| config/config_test.go | Adds backup/restore logic for TestLoadWrongDefaultYAMLConfig and four new test functions to verify API/logging defaults and environment variable overrides |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Update tests to use Platform constants' String method instead of hardcoded strings - Refactor test input for marshaling and unmarshaling to use dynamic string values from Platform constants - Improve JSON marshaling test to compare with dynamically generated expected output Signed-off-by: appleboy <appleboy.tw@gmail.com>
…ests - Switch to using t.Cleanup for environment variable cleanup in tests - Remove manual os.Unsetenv calls at the end of test functions - Improve test reliability by ensuring proper cleanup even on failure Signed-off-by: appleboy <appleboy.tw@gmail.com>
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.
fix #854