Fix examples/config-sample.json and cover the example configs with a test - #4709
Open
VictorZaca wants to merge 3 commits into
Open
Fix examples/config-sample.json and cover the example configs with a test#4709VictorZaca wants to merge 3 commits into
VictorZaca wants to merge 3 commits into
Conversation
The sample config has not been valid since the 2023 disk layout rework and currently fails to parse, so the file the README points users at cannot be used: - sector_size was null, but SectorSize became a required object - partitions were missing the dev_path key, which the parser reads - the /home size used the Percent unit, which no longer exists - /home started at 20 GiB while / ended at 20993 MiB, overlapping it - gfx_driver kept the old "All open-source (default)" value Sizes and keys now mirror what the installer itself writes when saving a configuration.
Nothing in the test suite or CI reads examples/, which is how the sample config could stay broken for years while every other config surface kept working. Parse both example files through ArchConfigHandler, and check the partition entries directly since the parser only reaches them when the configured device exists on the machine, which is never true in CI.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the bundled example configuration (examples/config-sample.json) so it conforms to the current config schema and no longer crashes when used, and adds tests to prevent future drift between example configs and the parser expectations.
Changes:
- Fix
examples/config-sample.jsonto include required partition fields (dev_path,sector_sizeobjects), correct units/layout, and updategfx_driverto a valid enum value. - Add fixtures for the example config/creds files.
- Add tests to parse the example config/creds via
ArchConfigHandlerand to validate disk partition invariants directly from the JSON.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/test_args.py |
Adds tests that parse the example config/creds and validate partition invariants using Size.parse_args. |
tests/conftest.py |
Adds session-scoped fixtures pointing to examples/config-sample.json and examples/creds-sample.json. |
examples/config-sample.json |
Fixes sample config fields/values to match current parsing expectations (partition schema, sector sizes, non-overlap, gfx driver). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The loops in test_example_config_partitions would not execute if device_modifications or a device's partitions were empty, so the test could pass while checking nothing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
examples/config-sample.jsoncannot be parsed by the current config format. Runningarchinstall --config examples/config-sample.jsonon a machine that actually has the configureddevice raises before the installer starts.
Five separate problems, all in the sample rather than in the parser:
dev_path.DiskLayoutConfiguration.parse_argindexes itdirectly (
partition['dev_path']), so it raisesKeyError: 'dev_path'."sector_size": nullin everysizeandstart.Size.parse_argshands it toSectorSize.parse_args, which indexesarg['value'], so it raisesTypeError: 'NoneType' object is not subscriptable./homeuses"unit": "Percent", which is not a member ofUnit, so it raisesKeyError: 'Percent'./and/homeoverlap./starts at 513 MiB and is 20 GiB long, so it ends at 20993 MiB,but
/homestarts at 20 GiB (20480 MiB), so it raisesValueError: Partitions overlap."gfx_driver": "All open-source (default)"no longer matches anyGfxDrivervalue; theenum is
All open-source.Fixes:
dev_pathis set tonullon each partition, everysector_sizebecomes an explicit 512 Bobject,
/homebecomes a fixed 10 GiB starting at 20993 MiB, andgfx_driveris corrected. Thelayout the sample describes is otherwise unchanged, apart from
/homeno longer claiming apercentage the format does not support.
Test
The reason this went unnoticed is that partition entries are only parsed when the configured device
is present on the machine:
device_handler.get_device()returnsNoneand the entry is skippedwith a silent
continue, so no CI runner ever reaches that code. The added coverage works aroundthat:
test_example_config_parsingrunsexamples/config-sample.jsonandexamples/creds-sample.jsonthrough
ArchConfigHandler, which catches everything outside the disk section (item 5 above, andany future drift in the other sections).
test_example_config_partitionsreads the partition entries straight from the JSON and assertsthe invariants the parser would enforce:
dev_pathpresent, sizes parseable throughSize.parse_args, first partition at no less than 1 MiB, and no overlap between consecutivepartitions (items 1 to 4).
Reverting either change to the sample makes the corresponding test fail.