bottlerocket-os / bottlerocket-os/bottlerocket-settings-sdk
Add serialization tests for settings models to catch missing `skip_serializing_if`
- Dominant language
- Rust
- Stars
- 5
- Forks
- 36
- Avg merge
- 4d 12h
- Merged PRs (30d)
- 2
Description
### Background
PR #110 attempted to fix a bug where fail_cgroup_v1 was serializing as "fail_cgroup_v1": null instead of being omitted when None. The field was missing `skip_serializing_if = "Option::is_none"`.
This fix attempt revealed a gap in our test coverage: we only verify deserialization (JSON → struct), not serialization (struct → JSON). The bug would have been caught immediately with proper serialization tests.
### Gap
Settings models lack tests that verify serialization output. Specifically:
1. Default serialization - A default-constructed settings struct should serialize to {}, not include null values for optional fields
2. Round-trip consistency - Deserialize → serialize should produce equivalent JSON
### Proposed Solution
Add serialization tests for settings models, below is a very simplified version, we could take it from there and make it more comprehensive.
```rust
#[test]
fn test_default_serialization() {
let settings = MySettings::default();
let json = serde_json::to_value(&settings).unwrap();
assert_eq!(json, serde_json::json!({}));
}
```
This pattern should be applied to all settings models in the settings SDK to prevent similar regressions.
### References
- https://github.com/bottlerocket-os/bottlerocket-settings-sdk/pull/110
Contributor guide
Research direction
Read PR #110 first, then locate the settings models and their existing deserialization tests. Add coverage for default serialization omitting optional null fields and for deserialize-then-serialize equivalence across all settings models; the tests should fail for the missing skip_serializing_if case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100