-
Notifications
You must be signed in to change notification settings - Fork 10
Backward compatibility for validating JSON files using Test-JSON command #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backward compatibility for validating JSON files using Test-JSON command #78
Conversation
83d9639
to
f501fb1
Compare
f501fb1
to
30fdda2
Compare
e346110
to
aae1513
Compare
aae1513
to
14e5eed
Compare
…and. - `Test-JSON` command was introduced in PowerShell 6.1. This commit adds a backward compatibility test for JSON files by implementing a custom Test-JSON function that uses the `Newtonsoft.Json` library for versions prior to PowerShell 6.1.
14e5eed
to
1dc3619
Compare
…kward compatibility (PowerShell 5.1).
08d24a3
to
54de2e0
Compare
There was a problem hiding this comment.
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 backward compatibility for JSON validation by skipping the Test-Json
call on Windows PowerShell 5.1 (with a warning), refines error handling, and updates supported hash algorithms.
- Limit supported checksum hash functions to SHA256, SHA384, and SHA512
- In
ValidateJSONChecksumFile
, warn and exit early ifTest-Json
is missing, and unify error messaging - Add Pester tests for the skip logic and update documentation with a compatibility note
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
containers-toolkit/Private/CommonToolUtilities.psm1 | Remove legacy hash algorithms, enforce failure on invalid JSON, skip validation when Test-Json is missing |
Tests/CommonToolUtilities.Tests.ps1 | Add test for skipping JSON validation, fix error message expectations |
README.md | Add a note about module compatibility with PowerShell versions |
Comments suppressed due to low confidence (3)
README.md:35
- [nitpick] The note suggests PowerShell 5.1 is unsupported, but you've added runtime compatibility for PS 5.1 in this PR. Consider clarifying that JSON validation is skipped on PS 5.1 but core functionality still works.
> NOTE: While this module is developed and tested with PowerShell 7, it may work with PowerShell 5.1, but it is not guaranteed. PowerShell 7 is recommended for the best experience.
Tests/CommonToolUtilities.Tests.ps1:537
- [nitpick] This test checks that no exception is thrown and that
Test-Json
isn’t invoked, but it doesn’t assert that the warning is emitted. Consider adding a check for the expected warning output to fully cover the new path.
It "should skip JSON validation when Test-Json is not available" {
Tests/CommonToolUtilities.Tests.ps1:538
- There's no space between the mock script block and
-ParameterFilter
, which will break Pester parsing. Add a space before-ParameterFilter
.
Mock Get-Command -ModuleName "CommonToolUtilities" -MockWith { return $null }-ParameterFilter {
4151183
to
e86b188
Compare
PR Description
Background
The
Test-Json
cmdlet was introduced in PowerShell 6.1 as a native way to validate JSON content. However, this command does not exist in Windows PowerShell 5.1 or earlier, which leads to runtime failures when scripts rely on it.During the BuildKit installation process, attempts to use
Test-Json
in older environments result in the following error:Fix
This PR remedies this by skipping JSON validation, with a warning, if the
Test-JSON
command does not exist.Relevant Links
Checklist
As part of our commitment to engineering excellence, before submitting this PR, please ensure:
In addition, after this PR has been reviewed, please ensure: