Skip to content

opts: add tests for weight/throttle device opts validators#7113

Merged
thaJeztah merged 1 commit into
docker:masterfrom
nikolauspschuetz:test-device-opts-validators
Jul 25, 2026
Merged

opts: add tests for weight/throttle device opts validators#7113
thaJeztah merged 1 commit into
docker:masterfrom
nikolauspschuetz:test-device-opts-validators

Conversation

@nikolauspschuetz

Copy link
Copy Markdown
Contributor

opts/weightdevice.go and opts/throttledevice.go had no tests, unlike the other opts in the package. This adds table-driven coverage for ValidateWeightDevice, ValidateThrottleBpsDevice, ValidateThrottleIOpsDevice, and the Set/GetList paths — boundaries (weight 0/10/1000, uint16 overflow), rate parsing (units, integer-only iops), and the format/error cases.

Test-only, no behavior change. Part of #201.

opts/weightdevice.go and opts/throttledevice.go had no tests, unlike the
sibling opts. Add table-driven coverage for ValidateWeightDevice,
ValidateThrottleBpsDevice, ValidateThrottleIOpsDevice and the Set/GetList
paths, including boundary (weight 0/10/1000/overflow) and error cases.

Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
@nikolauspschuetz
nikolauspschuetz force-pushed the test-device-opts-validators branch from ff077f2 to 8f3477f Compare July 20, 2026 20:47
@nikolauspschuetz
nikolauspschuetz marked this pull request as ready for review July 20, 2026 22:19
@thaJeztah

Copy link
Copy Markdown
Member

Thx! Changes reminds me that I still want to look at removing some of the validation from the CLI itself, because it's very specific;
https://github.com/nikolauspschuetz/docker-cli/blob/8f3477fca1be4b3ba81290bf3b9e60931b47b6a4/opts/weightdevice.go#L20-L23
https://github.com/nikolauspschuetz/docker-cli/blob/8f3477fca1be4b3ba81290bf3b9e60931b47b6a4/opts/weightdevice.go#L28-L30

Ultimately those checks belong on the API / daemon-side, and the CLI would be only responsible for parsing it into the right types (at most see if the path is well-formed), But I still need to double check that the API produces a useful error, and not some obscure one 😓

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
thaJeztah merged commit 19a0ee3 into docker:master Jul 25, 2026
102 of 103 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants