Skip to content

Add version comparison support to Microsoft/OSInfo#1603

Open
SteveL-MSFT wants to merge 10 commits into
PowerShell:mainfrom
SteveL-MSFT:os-ver-compare
Open

Add version comparison support to Microsoft/OSInfo#1603
SteveL-MSFT wants to merge 10 commits into
PowerShell:mainfrom
SteveL-MSFT:os-ver-compare

Conversation

@SteveL-MSFT

@SteveL-MSFT SteveL-MSFT commented Jun 30, 2026

Copy link
Copy Markdown
Member

PR Summary

  • Add explicit test operation instead of relying on synthetic test
  • Add support for version to have comparison operators: >, <, =, <=, >= (whitespace is allowed)
  • Updated osinfo CLI with new test subcommand and refactored the arg parsing in main.rs
  • Fix how code coverage is collected so report is correct, CC reporting is inconsistent so allowing any CC to pass CI for now. Will continue to work on this.

Example for this PR on fixed CC report:

image

PR Context

Fix #1521

Copilot AI review requested due to automatic review settings June 30, 2026 02:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a native test implementation for the Microsoft/OSInfo DSC resource and extends the version property to support comparison operators during test, enabling minimum/maximum OS version assertions (per #1521).

Changes:

  • Added a dedicated test operation to the Microsoft/OSInfo resource manifest and a corresponding osinfo test CLI subcommand.
  • Implemented version constraint parsing/comparison (>, <, =, >=, <=) for the resource’s test operation via version-compare.
  • Added Pester coverage for version operator behavior in the resource test flow.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
resources/osinfo/tests/osinfo.tests.ps1 Adds Pester tests validating version operator constraints via dsc resource test.
resources/osinfo/src/main.rs Adds test subcommand handling and refactors CLI arg dispatch.
resources/osinfo/osinfo.dsc.resource.json Declares an explicit test capability and documents operator-prefixed version constraints; adds _inDesiredState to schema.
lib/dsc-lib-osinfo/src/lib.rs Implements perform_test, version constraint parsing, and _inDesiredState emission.
lib/dsc-lib-osinfo/Cargo.toml Adds version-compare dependency for OSInfo test/version comparison.
Cargo.toml Adds version-compare to workspace dependencies.
Cargo.lock Locks version-compare dependency version.

Comment thread resources/osinfo/src/main.rs Outdated
Comment thread resources/osinfo/src/main.rs Outdated
Comment thread resources/osinfo/src/main.rs Outdated
Comment thread lib/dsc-lib-osinfo/src/lib.rs
Comment thread lib/dsc-lib-osinfo/src/lib.rs
@SteveL-MSFT SteveL-MSFT marked this pull request as draft June 30, 2026 03:38
@SteveL-MSFT SteveL-MSFT marked this pull request as ready for review June 30, 2026 17:34
@SteveL-MSFT SteveL-MSFT requested a review from Copilot June 30, 2026 17:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Comment thread lib/dsc-lib-osinfo/src/lib.rs
Comment thread helpers.build.psm1
Comment thread resources/osinfo/tests/osinfo.tests.ps1
/// with an ASCII digit. Strings like `">> 1.0"` (double operator) are
/// therefore treated as literal exact-match strings rather than producing a
/// misleading parsed operator.
fn parse_version_constraint(constraint: &str) -> (&str, &str) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not blocking for this PR but a few general notes on the new functionality:

  1. In the current implementation, the version property only gets an autogenerated JSON Schema as type: [string, null] - adding version requirement syntax can't be represented through the schema without either providing the schema directly (schema_with) or as schemars attributes.
  2. In this implementation, we're parsing and validating the string on demand and only in the test operation.
  3. This is an entirely different version requirement syntax from the SemanticVersionReq in dsc-lib which is okay but worth noting.

We could encapsulate this in a couple of type definitions, which would enable better documentation and validation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In this case, I don't believe Linux, macOS, or Windows follows semver, hence not trying to convert them to semver

Comment thread lib/dsc-lib-osinfo/src/lib.rs
Comment thread lib/dsc-lib-osinfo/src/lib.rs
@SteveL-MSFT SteveL-MSFT force-pushed the os-ver-compare branch 4 times, most recently from 3c12edb to 3aa1d92 Compare July 1, 2026 22:20
SteveL-MSFT and others added 10 commits July 1, 2026 19:40
Build-RustProject now uses `cargo llvm-cov build --no-report` when
-CodeCoverage is specified, producing instrumented binaries that write
profraw data when executed. Previously the -CodeCoverage parameter was
accepted but unused — the function always called `cargo build`.

When Pester tests run with -CodeCoverage enabled, LLVM_PROFILE_FILE is
set via a new Get-LlvmProfileFilePattern helper so that externally
invoked instrumented binaries write profraw data to the target directory
where `cargo llvm-cov report` can discover it.

The CI workflow is updated to combine the build and coverage test steps
into a single `./build.ps1 -Clippy -Test -CodeCoverage` invocation,
ensuring the instrumented build is used for coverage collection.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cargo-llvm-cov does not have a `build` subcommand. Instead, use
`cargo llvm-cov show-env` to retrieve the environment variables
(RUSTC_WRAPPER, LLVM_PROFILE_FILE, etc.) and set them in the process.
A normal `cargo build` then produces instrumented binaries
transparently.

Replace Get-LlvmProfileFilePattern with Set-LlvmCovEnvironment /
Reset-LlvmCovEnvironment which set and restore all required env vars.
The env vars are set before the build so both cargo build and Pester
tests benefit from instrumentation, and restored after the LCOV report
is generated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Since Set-LlvmCovEnvironment sets RUSTC_WRAPPER and LLVM_PROFILE_FILE,
normal `cargo build` and `cargo test` already produce instrumented
binaries and write profraw data. Calling `cargo llvm-cov test` on top
of the pre-set env vars caused conflicts and errors.

Remove the -CodeCoverage parameter from both Build-RustProject and
Test-RustProject — the env vars make instrumentation transparent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a step to the coverage-report job that exits with an error when
the coverage percentage on changed Rust code is less than 70%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests covering edition (Windows-only), codename (Linux-only),
bitness, and architecture properties in the OSInfo test operation.
Also tests combining multiple properties to verify partial mismatch
detection.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The coverage report showed 0% because the build step excluded Pester
tests (-ExcludePesterTests). Since the osinfo test operation code paths
are only exercised by Pester tests (not Rust unit tests), the
instrumented binaries never recorded any hits for those functions.

Change the build step to run the 'resources' Pester test group alongside
Rust tests so the instrumented osinfo binary is exercised and profraw
data is recorded.

Also filter out CARGO_LLVM_COV_SHOW_ENV from Set-LlvmCovEnvironment
since propagating that flag causes cargo-llvm-cov to print env vars and
exit rather than performing compilation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The coverage report used github.event.pull_request.base.sha directly
with three-dot git diff syntax. After a rebase or when the base branch
advances, base.sha points to the current tip of the base branch rather
than the common ancestor, causing the diff to include unrelated changes
or miss actual PR changes.

Fix by computing git merge-base between base and head SHAs in the
coverage-report job, then using two-dot diff syntax throughout. This
ensures only the PR's own changes are analyzed regardless of rebases
or upstream merges.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The coverage-report job failed to match LCOV source file paths because
the matching logic did not handle relative paths from cargo-llvm-cov.
Added direct comparison with relative file path and normalized slash
handling. Also added verbose diagnostic output when no match is found
to aid future debugging.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

Microsoft/OSInfo: Support minimum version assertion (>= comparison)

3 participants