(GH-538) Enhance schema export functionality and add error handling for duplicate paths - #1672
(GH-538) Enhance schema export functionality and add error handling for duplicate paths#1672Gijs Reijn (Gijsreyn) wants to merge 14 commits into
Conversation
…handling for duplicate paths
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability and completeness of the Rust-based schema export pipeline (cargo xtask schema export) so schemas can be generated from source and published in versioned folders without silent collisions.
Changes:
- Add duplicate-path detection during schema export to prevent silent overwrites when two types resolve to the same output path.
- Extend
xtask schema exportto support exporting explicit schema version folders and expanding a--release X.Y.Zinto patch/minor/major version folders. - Improve schema-version parsing ergonomics and align several schema IDs/paths so exported locations match the intended repository layout.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| xtask/src/schemas/export.rs | Adds duplicate export-path detection and expands the exported type list. |
| xtask/src/main.rs | Adds --schema-version / --release resolution logic and tests for version resolution. |
| xtask/src/args.rs | Extends CLI args for schema export to accept repeated schema versions or a release expansion. |
| xtask/locales/en-us.toml | Adds localized help/error strings for the new schema export options and errors. |
| lib/dsc-lib/src/dscresources/invoke_result.rs | Adjusts schema export base name to avoid output collisions. |
| lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs | Fixes schema export folder path and broadens $schema validation to accept relevant URIs. |
| lib/dsc-lib-jsonschema/src/dsc_repo/recognized_schema_version.rs | Implements FromStr + error type for schema version folder parsing, with tests. |
| lib/dsc-lib-jsonschema/src/dsc_repo/mod.rs | Re-exports UnrecognizedSchemaVersion for downstream use. |
| lib/dsc-lib-jsonschema/locales/en-us.toml | Adds i18n strings for schema-version parsing errors. |
| dsc/src/util.rs | Switches CLI/server schema generation to use DscRepoSchema generation (bundled vs canonical based on type). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…handling for duplicate paths
Co-authored-by: Mikey Lombardi (He/Him) <michael.t.lombardi@gmail.com>
352828d to
a43c5c8
Compare
…ithub.com/Gijsreyn/operation-methods into PowerShellgh-538/main/schema-export-fixes
98ddc31 to
952a6c5
Compare
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn from_str_round_trips_every_recognized_version() { | ||
| for version in RecognizedSchemaVersion::all() { | ||
| let parsed: RecognizedSchemaVersion = version.to_string().parse().unwrap(); | ||
| assert_eq!(parsed, version); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn from_str_is_case_insensitive_and_trims() { | ||
| let parsed: RecognizedSchemaVersion = " VNEXT ".parse().unwrap(); | ||
| assert_eq!(parsed, RecognizedSchemaVersion::VNext); | ||
| } | ||
|
|
||
| #[test] | ||
| fn from_str_rejects_unrecognized_versions() { | ||
| assert!("v99.0.0".parse::<RecognizedSchemaVersion>().is_err()); | ||
| assert!("not-a-version".parse::<RecognizedSchemaVersion>().is_err()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Nit: We should prefer to do integration testing in the lib/dsc-lib-jsonschema/tests/integration folder - here we're testing a public API.
As I wrote in #1391 about placing public API tests in tests/integration:
The primary motivating factor here is build and test timings. When integration tests are colocated with module code, any changes to either the tests or the implementation requires recompilation because the file changed.
Moving the tests and following the current pattern that mirrors the src directory structure can also help us quickly identify where we are lacking integration tests for the library. Currently, given the existing layout, we need to carefully check every implementation file for test coverage.
This is non-blocking but I wanted to note it for future PRs.
Mikey Lombardi (He/Him) (michaeltlombardi)
left a comment
Mikey Lombardi (He/Him) (michaeltlombardi)
left a comment
There was a problem hiding this comment.
Aside from a non-blocking nit on test placement, I think we should align to the Rust API Guidelines and ensure our error types have the Error suffix for API clarity.
…handling for duplicate paths
Co-authored-by: Mikey Lombardi (He/Him) <michael.t.lombardi@gmail.com>
952a6c5 to
53b3662
Compare
PR Summary
The primary purpose of this change is to make cargo xtask schema export reliable and complete enough to publish versioned DSC schemas from source code. It fixes two silent output collisions, exports every type that derives DscRepoSchema, and adds version targeting so the exporter can produce any recognized schema version folder instead of only vNext.
As a maintainer or contributor, you can now invoke:
cargo xtask schema export— exports schemas/vNext as before.cargo xtask schema export --schema-version v3.2 --schema-version v3.2.3— exports specific version folders.cargo xtask schema export --release 3.3.0— exports the full set a release must publish: schemas/v3.3.0, schemas/v3.3, and schemas/v3.PR Context
As part of #538 and following up on #1406, the exporter needs to produce correct, complete, versioned schemas from source before it can replace the hand-maintained YAML pipeline under schemas/src.