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

Build locally without inlining #1872

Merged
merged 6 commits into from
Mar 20, 2021
Merged

Build locally without inlining #1872

merged 6 commits into from
Mar 20, 2021

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Mar 9, 2021

For a while I've been inlining the whole script into one module but debugging and changing the code is difficult that way, because changes need to be done in the respective file, not Pester.psm1. Investigating how to just dot-source when building locally.

For a while I've been inlining the whole script into one module but debugging and changing the code is difficult that way, because changes need to be done in the respective file, not Pester.psm1. Investigating how to just dot-source when building locally
@nohwnd
Copy link
Member Author

nohwnd commented Mar 9, 2021

At the moment the biggest issue is that module Pester.Runtime.psm1 is changed to be just .ps1 script, which makes strict mode apply to it from the tests where we import the file directly. There are many places where I did not consider strict mode, and so many tests are failing. I will probably just disable the strict mode in tests, because it does not do what we thought it does, and pursuit having the code strict in a different milestone. It's useful, but not more useful than having the dot-sourcing / inlining build.

@nohwnd nohwnd self-assigned this Mar 9, 2021
@nohwnd
Copy link
Member Author

nohwnd commented Mar 9, 2021

@fflaten I guess you are making a bit of changes on Pester, so you might like this. No more doing changes just to have them overwritten on build, or setting breakpoints in the source file and never have it hit...

@nohwnd nohwnd marked this pull request as ready for review March 9, 2021 21:07
@fflaten
Copy link
Collaborator

fflaten commented Mar 9, 2021

I've stopped modifying the src-files a long time ago. All dev and debug directly in bin/Pester.psm1 with git stash as archive 😄
This sounds very good, will look into it the next couple of days when I find some time - unless you've already merged by then.

@nohwnd
Copy link
Member Author

nohwnd commented Mar 9, 2021

There is basically nothing to it. I just dot-source the files (almost) as I did originally. And only inline the code when releasing or in CI. There are few fixes needed to make this work, like when we are omitting lines from errors that belong to pester, or when importing dlls relative to the current file path.

And also all the dependency modules became script files, because otherwise there was a ton of problems with variables that are inherited from parent that were not inherited due to Pester.Runtime being a module. On the other hand making it non-module had big impact on Pester.Runtime tests, because then all the code run in the same session state, which led to a lot of conflicts. So I just dot source it into a module in the test.

@nohwnd nohwnd merged commit d6645c8 into v5.0 Mar 20, 2021
@nohwnd nohwnd deleted the build-locally-without-inlining branch March 20, 2021 12:01
Comment on lines +2 to +19
$path = "$PSScriptRoot/bin/netstandard2.0/Pester.dll"
if ((Get-Variable -Name "PESTER_BUILD" -ValueOnly -ErrorAction Ignore)) {
$path = "$PSScriptRoot/../bin/bin/netstandard2.0/Pester.dll"
}
else {
$path = "$PSScriptRoot/../bin/bin/netstandard2.0/Pester.dll"
} # endif
& $SafeCommands['Add-Type'] -Path $path
}
else {
& $SafeCommands['Add-Type'] -Path "$PSScriptRoot/bin/net452/Pester.dll"
$path = "$PSScriptRoot/bin/net452/Pester.dll"
if ((Get-Variable -Name "PESTER_BUILD" -ValueOnly -ErrorAction Ignore)) {
$path = "$PSScriptRoot/../bin/bin/net452/Pester.dll"
}
else {
$path = "$PSScriptRoot/../bin/bin/net452/Pester.dll"
} # endif
& $SafeCommands['Add-Type'] -Path $path
Copy link
Collaborator

Choose a reason for hiding this comment

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

quick follow-up: what is the idea with the inner else-blocks (L6-L8 and L16-18)?

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.

None yet

2 participants