Add full feature-level coverage for Config contract in tests/feature#106
Add full feature-level coverage for Config contract in tests/feature#106
tests/feature#106Conversation
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
tests/feature
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds a comprehensive test suite ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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)
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/feature/config_test.go (1)
34-39: Add false-path assertions for bool coverage completeness.Current bool tests only validate truthy paths; include explicit false and default-false cases to catch one-sided regressions.
Proposed test additions
func (s *ConfigTestSuite) TestEnvBool() { s.T().Setenv("TESTS_CONFIG_ENV_BOOL", "true") + s.T().Setenv("TESTS_CONFIG_ENV_BOOL_FALSE", "false") s.True(facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL")) + s.False(facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL_FALSE")) s.True(facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL_MISSING", true)) + s.False(facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL_MISSING_FALSE", false)) } @@ func (s *ConfigTestSuite) TestGetBool() { facades.Config().Add("tests.config.bool", true) + facades.Config().Add("tests.config.bool_false", false) s.True(facades.Config().GetBool("tests.config.bool")) + s.False(facades.Config().GetBool("tests.config.bool_false")) s.True(facades.Config().GetBool("tests.config.bool.missing", true)) + s.False(facades.Config().GetBool("tests.config.bool.missing.false", false)) }Also applies to: 62-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/feature/config_test.go` around lines 34 - 39, Update the TestEnvBool test to include false-path assertions: set an env var like "TESTS_CONFIG_ENV_BOOL_FALSE" to "false" and assert facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL_FALSE") is false, and add an assertion for a missing key with an explicit default false (e.g. facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL_MISSING_FALSE", false) == false). Do the same corresponding change for the other bool test referenced (lines ~62-67) so both truthy and falsy/default-false cases are covered; locate the checks via the TestEnvBool test function and calls to facades.Config().EnvBool.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/feature/config_test.go`:
- Around line 12-18: The suite is missing the tests package bootstrap; add an
import for "goravel/tests" and embed tests.TestCase into the ConfigTestSuite
struct (or implement SetupSuite() to call tests.Boot() equivalents) so that
bootstrap.Boot() runs before tests; specifically update the ConfigTestSuite
definition (type ConfigTestSuite struct { suite.Suite }) to include
tests.TestCase and add the "goravel/tests" import to align with other feature
tests like orm_test.go/db_test.go/auth_test.go.
---
Nitpick comments:
In `@tests/feature/config_test.go`:
- Around line 34-39: Update the TestEnvBool test to include false-path
assertions: set an env var like "TESTS_CONFIG_ENV_BOOL_FALSE" to "false" and
assert facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL_FALSE") is false, and add
an assertion for a missing key with an explicit default false (e.g.
facades.Config().EnvBool("TESTS_CONFIG_ENV_BOOL_MISSING_FALSE", false) ==
false). Do the same corresponding change for the other bool test referenced
(lines ~62-67) so both truthy and falsy/default-false cases are covered; locate
the checks via the TestEnvBool test function and calls to
facades.Config().EnvBool.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4d6789e-fda5-41da-be28-345ffcbb6b17
📒 Files selected for processing (1)
tests/feature/config_test.go
|
@copilot Fix the comments, and the file lint is incorrect, fix it as well. |
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Fixed in f19831c. I aligned |
Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.
Main features
Environment variable retrieval
Env,EnvString, andEnvBoolmethods for reading environment variables with fallback defaultsConfiguration value management
Addmethod to dynamically add configuration values at runtimeGet,GetString,GetInt,GetBool, andGetDurationmethods for type-safe retrieval with default fallbacksStructured unmarshalling
UnmarshalKeyto map configuration entries into typed structs usingmapstructuretagsCode examples
Environment variable retrieval:
Typed configuration retrieval:
Structured unmarshalling:
Error handling for invalid unmarshalling targets: