Skip to content
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

Add assembly version check during module import #1790

Merged
merged 17 commits into from
Apr 20, 2021

Conversation

fflaten
Copy link
Collaborator

@fflaten fflaten commented Nov 25, 2020

PR Summary

PowerShell (.NET) doesn't allow loading different version of the same assembly in a process/appdomain. This can give the user a non-terminating error if they import different versions of Pester in the same PowerShell-session. Usually the error can be ignored, but some versions of the Pester module can require changes in the assembly and the user might see weird errors as a result.

The PR adds a minimum assembly version-check during import of the assembly. The requirement is stored as value in the manifest. If the user has the same or newer (until this idea breaks) assembly loaded, the assembly will not be loaded again during import. If the version is older than the requirement, an exception will be thrown to stop the module import, warning about the conflict. Else, import as normal (current behavior).

Fix #1770

PR Checklist

  • PR has meaningful title
  • Summary describes changes
  • PR is ready to be merged
    • If not, use the arrow next to Create Pull Request to mark it as a draft. PR can be marked Ready for review when it's ready.
  • Tests are added/update (if required)
  • Documentation is updated/added (if required)

build.ps1 Outdated Show resolved Hide resolved
src/Pester.Types.ps1 Show resolved Hide resolved
@nohwnd nohwnd added this to the 5.2 milestone Apr 18, 2021
@nohwnd
Copy link
Member

nohwnd commented Apr 18, 2021

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

build.ps1 Outdated
if ($Clean) {
$content += @(
$manifest = Test-ModuleManifest -Path "$PSScriptRoot/bin/Pester.psd1"
Copy link
Member

Choose a reason for hiding this comment

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

This fails on PS3 with "The wildcard is incorrect"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well that was weird. Looks like Test-ModuleManifest invoked Import-Dependency for some absurd reason. 😕
Stacktrace?

Copy link
Member

Choose a reason for hiding this comment

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

I don't have any, I can only see it not fail when running on ps7.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted to PS3 build with (ugly) workaround in case PS3 build is still required. If it's okay to use PS7 for build then we should use Test-ModuleManifest again.

The PS7/vs2019 run only failed due to the psversion-output commit. It made passthru object $r from p-tests become arrays which broke the if failed-check after.

Copy link
Member

Choose a reason for hiding this comment

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

We can build on ps7, but I would keep both approaches in case I need to debug something specifically on PS3. Thx. And thanks for helping here, I had no idea what was happening. 🙂

src/Pester.psd1 Outdated Show resolved Hide resolved
@nohwnd nohwnd marked this pull request as ready for review April 19, 2021 08:17
@nohwnd
Copy link
Member

nohwnd commented Apr 19, 2021

Other than the nitpick and the possible reversal to use ps7 for build this is imho good to go. I am also okay with keeping it PS3 compatible and building with the existing setup. If nothing the build will be slightly faster because it is on a deployed agent, and not on a shared (hosted) agent.

Co-authored-by: Jakub Jareš <me@jakubjares.com>
@nohwnd nohwnd merged commit 92214f4 into pester:v5.0 Apr 20, 2021
@fflaten fflaten deleted the assemblycheck branch April 21, 2021 15:11
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.

Parameterized tests not working (Check pester assembly version)
2 participants