10000 Fix `Start-Transcript` error when `$Transcript` is a `PSObject` wrapped string by kborowinski · Pull Request #24963 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Fix Start-Transcript error when $Transcript is a PSObject wrapped string #24963

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

Merged
merged 2 commits into from
Feb 7, 2025

Conversation

kborowinski
Copy link
Contributor

PR Summary

Fix #24740

PR Context

Unwrap transcript filename stored as PSObject in $Transcript preference variable.

PR Checklist

Start-Transcript
Write-Host -Message $message -InformationAction Ignore
Stop-Transcript
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use ValidateTranscription function?

Copy link
Contributor Author
@kborowinski kborowinski Feb 6, 2025

Choose a reason for hiding this comment

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

Not without a bit of refactoring. We would need to implement extra support just for this case, so I have chosen to follow similar existing tests, like:

    It "Transcription should record Write-Host output when InformationAction is set to SilentlyContinue" {
        [String]$message = New-Guid
        $script = {
            Start-Transcript -Path $transcriptFilePath
            Write-Host -Message $message -InformationAction SilentlyContinue
            Stop-Transcript
        }

        & $script

        $transcriptFilePath | Should -Exist
        $transcriptFilePath | Should -FileContentMatch $message
    }

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why won't it work?

        $script = "Start-Transcript"
        $outputFilePath = expected_file_path
        ValidateTranscription -scriptToExecute $script -outputFilePath $outputFilePath

Copy link
Contributor Author
@kborowinski kborowinski Feb 6, 2025

Choose a reason for hiding this comment

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

I need to test if PSObject wrapped string in $Transcript preference variable works with Start-Transcript. This will not work, because when $Transcript is not defined, the Start-Transcript will create the transcript file automatically either in:

  • on Windows: $HOME\Documents
  • on Linux or macOS: $HOME

and unwrapping will not be tested at all.

I would have to refactor ValidateTranscription to support this case. It would need to be something like this:

$ps.AddScript({
    param([PSObject]$filePath)
    $Global:Transcript = $filePath
}).AddArgument(
    $outputFilePath
).Invoke()

plus some extra logic to handle this case

Copy link
Collaborator

Choose a reason for hiding this comment

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

        $script = "$Global:Transcript = testfile; Start-Transcript"
        $outputFilePath = expected_file_path
        ValidateTranscription -scriptToExecute $script -outputFilePath $outputFilePath

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, now you are just flexing 😂, how did I miss that 😉

BTW, Pester does not like this:

It "Should create Transcript file with 'Transcript' preference variable" {
    $script = "$Global:Transcript = $transcriptFilePath; Start-Transcript"
    ValidateTranscription -scriptToExecute $script -outputFilePath $transcriptFilePath
}

image

Thankfully, this works:

It "Should create Transcript file with 'Transcript' preference variable" {
    $script = "Set-Variable -Scope Global -Name Transcript -Value $transcriptFilePath; Start-Transcript"
    ValidateTranscription -scriptToExecute $script -outputFilePath $transcriptFilePath
}

Will update PR ASAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One more change. We need to cast the $transcriptFilePath string to the PSObject, otherwise it defeats the purpose:

It "Should create Transcript file with 'Transcript' preference variable" {
    $script = "Set-Variable -Scope Global -Name Transcript -Value ([PSObject]'$transcriptFilePath'); Start-Transcript"
    ValidateTranscription -scriptToExecute $script -outputFilePath $transcriptFilePath
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd expect if we assign to psvariable it is already wrapped in PSObject. It seems the original issue is about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, when you assign it does, but when you use Set-Variable it does not. I had to do the cast, otherwise the test was passing on current release without fix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a comment in the test line to exclude regression.

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 6, 2025
@iSazonov

This comment was marked as outdated.

This comment was marked as outdated.

@iSazonov

This comment was marked as outdated.

This comment was marked as outdated.

@iSazonov iSazonov merged commit 0beb24e into PowerShell:master Feb 7, 2025
39 of 41 checks passed
Copy link
Contributor
microsoft-github-policy-service bot commented Feb 7, 2025

📣 Hey @kborowinski, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Start-Transcript throws when $Transcript is a PSObject wrapped string
2 participants
0