Skip to content

fix(config): 根治 TestDefaultConfigValues 非 hermetic 红测(#90/#107)#148

Merged
NeverENG merged 1 commit into
mainfrom
fix/config-hermetic-defaults
Jun 14, 2026
Merged

fix(config): 根治 TestDefaultConfigValues 非 hermetic 红测(#90/#107)#148
NeverENG merged 1 commit into
mainfrom
fix/config-hermetic-defaults

Conversation

@NeverENG

@NeverENG NeverENG commented Jun 14, 2026

Copy link
Copy Markdown
Owner

问题

TestDefaultConfigValuesNewGlobalConfig() 断言「纯默认值」,但 NewGlobalConfig 会先填默认、再用 config.json 覆盖。默认与 json 不一致的字段(MaxMemTableSize:默认 1024 vs json 10000)恒红;且测试结果依赖 config.json 是否被找到——非 hermetic。

修法(已与用户确认:抽纯默认构造 + 测试 hermetic)

  • 抽出 defaultGlobalConfig():返回纯代码默认值,不读文件、不解析命令行
  • NewGlobalConfig() = defaultGlobalConfig() + Init()(文件覆盖) + ParseFlags()(命令行覆盖)
  • TestDefaultConfigValues 改测 defaultGlobalConfig()——名副其实,且不再依赖外部文件

测试

go build ./... 通过;config 包全绿(此前唯一红点 MaxMemTableSize 消除),其余 config 测试无回归。

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Improved configuration initialization structure for better maintainability.
  • Tests

    • Enhanced configuration tests to better validate default values in isolation.

NewGlobalConfig 会先填默认再被 config.json 覆盖,旧测试却把它当纯默认值断言,
导致默认与 json 不一致的字段(MaxMemTableSize 1024 vs 10000)恒红,且依赖文件存在。
抽出 defaultGlobalConfig()(纯默认,不读文件/不解析命令行),NewGlobalConfig 在其上
叠加 Init+ParseFlags;测试改测 defaultGlobalConfig(),名副其实且不再依赖外部文件。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@NeverENG NeverENG merged commit bc16d98 into main Jun 14, 2026
3 of 4 checks passed
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a4c999e-9289-40d1-b24b-e6930b3e3721

📥 Commits

Reviewing files that changed from the base of the PR and between c051e47 and 4d7bd34.

📒 Files selected for processing (2)
  • config/global.go
  • config/global_test.go

📝 Walkthrough

Walkthrough

NewGlobalConfig() is refactored to delegate initial struct construction to a new internal defaultGlobalConfig() helper, which builds a GlobalConfig from code defaults including a derived log directory. TestDefaultConfigValues is updated to call defaultGlobalConfig() directly to avoid external config.json overwriting expected defaults.

Changes

Config default construction refactor

Layer / File(s) Summary
defaultGlobalConfig helper and wiring
config/global.go, config/global_test.go
Adds defaultGlobalConfig() that returns a fully populated GlobalConfig from code defaults (using defaultLogDir()). NewGlobalConfig() now calls it instead of constructing the literal inline. TestDefaultConfigValues switches from NewGlobalConfig() to defaultGlobalConfig() with comments noting the hermetic isolation from external file loading.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related issues

Poem

🐇 A config once tangled with files from afar,
Now defaults are pure, kept safe in a jar.
No config.json to muddy the test,
The rabbit extracted what's simply the best.
Clean defaults, clean code — what a hop! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/config-hermetic-defaults

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

🐯 BanGD 数据库内核评审

整体风险:🟢 低

变更总结:本 PR 在 config 包的配置初始化层引入配置构造的责任分离:从原有的 NewGlobalConfig()(默认值 → 文件覆盖 → 命令行覆盖 捆绑在一个函数内),拆分为 defaultGlobalConfig()(纯代码默认值,不涉及 I/O) + NewGlobalConfig()(在其上叠加 Init() 文件覆盖与 ParseFlags() 命令行覆盖)。动机是修复 TestDefaultConfigValues 的非 hermetic 问题——该测试调用 NewGlobalConfig() 断言纯默认值,但后者会读取 config.json 覆盖字段,导致测试结果依赖外部文件是否存在/内容。改动位于配置层的构造逻辑,不涉及底层的存储引擎、WAL、并发模型或内存管理。是测试可观测性/工程质量层面的改进,不改变任何运行时行为。

本评审不阻塞合入;架构级建议以 Issue 形式跟踪,普通问题在下方内联列出。

未发现需要从架构层面改进的问题。


本次评审消耗 token:共 15131 tokens(输入 14815,输出 316,缓存命中 0,缓存写入 0)|维度 [memory, storage, performance, resource, error]|对抗式复核 3 票/条,过滤疑似误报 0 条

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant