bottlerocket-os / bottlerocket-os/bottlerocket-settings-sdk

Add serialization tests for settings models to catch missing `skip_serializing_if`

Open
#112 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.