Skip to content

Add test for save script using Save-PSResource without -IncludeXml#1613

Closed
o-l-a-v wants to merge 3 commits intoPowerShell:masterfrom
o-l-a-v:test-add-script-save-psresource-without-includexml
Closed

Add test for save script using Save-PSResource without -IncludeXml#1613
o-l-a-v wants to merge 3 commits intoPowerShell:masterfrom
o-l-a-v:test-add-script-save-psresource-without-includexml

Conversation

@o-l-a-v
Copy link
Contributor

@o-l-a-v o-l-a-v commented Mar 31, 2024

PR Summary

Add test for #1609

Which probably broke when fixing #1335

PR Context

Add test "Save script without using -IncludeXML" for saving script using Save-PSResource without -IncludeXml.

Also fix typo and missing -IncludeXml in existing test "Save script using -IncludeXML".

PR Checklist

@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Mar 31, 2024

@microsoft-github-policy-service agree

Copy link
Member

@anamnavi anamnavi left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for adding this test and correcting the regression of our older test. I've added a few suggestions, mostly nitpicks to use PowerShell commands as we do throughout our other tests, but once that's in I'll approve, thanks @o-l-a-v !

It "Save script without using -IncludeXML" {
Save-PSResource -Name $testScriptName -Repository $PSGalleryName -Path $SaveDir -TrustRepository | Should -Not -Throw

$SavedScriptFile = [System.IO.Path]::Combine($SaveDir,('{0}.ps1' -f $testScriptName))
Copy link
Member

Choose a reason for hiding this comment

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

Just a nitpick to use Join-Path to be consistent with the rest of our tests

Suggested change
$SavedScriptFile = [System.IO.Path]::Combine($SaveDir,('{0}.ps1' -f $testScriptName))
$SavedScriptFile = Join-Path -Path $SaveDir -ChildPath "$testScriptName.ps1"

Save-PSResource -Name $testScriptName -Repository $PSGalleryName -Path $SaveDir -TrustRepository | Should -Not -Throw

$SavedScriptFile = [System.IO.Path]::Combine($SaveDir,('{0}.ps1' -f $testScriptName))
[System.IO.File]::Exists($SavedScriptFile) | Should -BeTrue
Copy link
Member

Choose a reason for hiding this comment

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

Just a nitpick to use powershell command Test-Path to be consistent with the rest of our tests

Suggested change
[System.IO.File]::Exists($SavedScriptFile) | Should -BeTrue
Test-Path -Path $SavedScriptFile | Should -BeTrue

@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Apr 10, 2024

@anamnavi : @SydneyhSmith told me to put tests into #1614 in #1614 (comment). So close this PR and continue in mentioned PR?

@anamnavi
Copy link
Member

@o-l-a-v ah yes, I missed that earlier. I'll close this PR. And add the comments from this to that PR.

@anamnavi anamnavi closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants