GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Terminate Start-PSPester if Pester module not found by djweber · Pull Request #5069 · PowerShell/PowerShell

Conversation

David (djweber) commented Oct 9, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Currently, if a user does not clone with the --recursive flag or run git submodule update --init, Start-PSPester will fail to run due to the Pester module missing. While the error hints at the Pester module not being found, there is no suggested way of fixing the issue presented to the user. This change causes a warning to appear at the beginning of the execution of Start-PSPester if the Pester module cannot be found.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

David (djweber) requested review from Travis Plunk (TravisEz13), Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) as code owners

October 9, 2017 21:50

David (djweber) mentioned this pull request

Oct 9, 2017

Update testing-guidelines.md to remind users about submodules #5061

Merged

Aditya Patwardhan (adityapatwardhan) reviewed

Oct 9, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

Aditya Patwardhan (adityapatwardhan) Oct 9, 2017

Copy link

Copy Markdown

Member

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.

Should this be an error instead? We would not be able to run tests without Pester.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

David (djweber) Oct 9, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Author

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.

Aditya Patwardhan (@adityapatwardhan) I could change this to use Write-Error, but now I'm wondering if we should use throw instead -- after all, there is no point in continuing at all if Pester can't be found. This would terminate the script with a single error instead of several (more vague) errors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) Oct 9, 2017

Copy link

Copy Markdown

Member

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.

Actually if we follow the pattern from: https://github.com/DdWr/PowerShell/blob/3fdd9f1b639df805f895c56cf574f2039ec15a76/build.psm1#L415

We should do Write-Warning and then return.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

David (djweber) Oct 9, 2017

Copy link

Copy Markdown

Author

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.

Aditya Patwardhan (@adityapatwardhan) Changes made.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

David (djweber) changed the title ~~Add warning to Start-PSPester if Pester module not found~~ Add terminating error to Start-PSPester if Pester module not found

Oct 9, 2017

David (djweber) changed the title ~~Add terminating error to Start-PSPester if Pester module not found~~ Terminate Start-PSPester if Pester module not found

Oct 9, 2017

Dongbo Wang (daxian-dbw) reviewed

Oct 10, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

Dongbo Wang (daxian-dbw) Oct 10, 2017

Copy link

Copy Markdown

Member

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.

A minor comment about the message -- how about this:

Write-Warning @"
Pester module not found.
Make sure that the proper git submodules are installed by running:
    git submodule update --init
"@

It looks like this:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) self-assigned this

Oct 10, 2017

David (djweber) commented

Oct 10, 2017

View reviewed changes

Comment thread

build.psm1 Outdated

David (djweber) Oct 10, 2017

Copy link

Copy Markdown

Author

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.

Dongbo Wang (@daxian-dbw) I made the module check a little stricter here than simply checking for the path.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

David (djweber) mentioned this pull request

Oct 10, 2017

Add warning to Start-PSPester if Pester module not found #5067

Closed


          Add warning to Start-PSPester if Pester module not found

bd1fdc4

Currently, if a user does not clone with the --recursive flag or run
git submodule update --init, Start-PSPester will fail to run due to
the Pester module missing. While the error hints at the Pester module not
being found, there is no suggested way of fixing the issue presented to
the user. This change makes a helpful warning to appear at the beginning
of the execution of Start-PSPester if the Pester module cannot be found.

Aditya Patwardhan (adityapatwardhan) requested changes

Oct 10, 2017

View reviewed changes

Comment thread

build.psm1

		[switch]$IncludeFailingTest
		)

		if (-not (Get-Module -ListAvailable -Name $Pester -ErrorAction SilentlyContinue))

Aditya Patwardhan (adityapatwardhan) Oct 10, 2017

Copy link

Copy Markdown

Member

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.

I actually prefer the Test-Path instead. With -ListAvailable the Pester version that comes installed with Windows 10 will be found. There are difference in those versions which might cause false errors in running tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

David (djweber) Oct 10, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Author

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.

Aditya Patwardhan (@adityapatwardhan) Since the full path to the project's Pester module is being passed in here, I believe this would not be an issue:

PS /Users/david/Code/PowerShell> Get-Module -ListAvailable /Users/david/Code/PowerShell/src/powershell-unix/bin/Linux/netcoreapp2.0/osx.10.12-x64/publish/Modules/Pester  | Format-Table
Path -Groupby Name

   Name: Pester

Path
----
/Users/david/Code/PowerShell/src/powershell-unix/bin/Linux/netcoreapp2.0/osx.10.12-x64/publish/Modules/Pester/Pester.psd1

However, if we only passed in Pester, I could see that being a cause for concern:

PS /Users/david/Code/PowerShell> Get-Module -ListAvailable Pester | Format-Table Path -Groupby Name
   
   Name: Pester

Path
----
/usr/local/microsoft/powershell/6.0.0-beta.8/Modules/Pester/4.0.8/Pester.psd1
/usr/local/microsoft/powershell/6.0.0-beta.8/Modules/Pester/Pester.psd1

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) Oct 10, 2017

Copy link

Copy Markdown

Member

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.

Makes sense. Misread it being Pester instead of $Pester.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) approved these changes

Oct 10, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) approved these changes

Oct 10, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) left a comment

Copy link

Copy Markdown

Member

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.

Thanks for your contribution!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) merged commit 42e8deb into PowerShell:master

Oct 10, 2017

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Add warning to Start-PSPester if Pester module not found (PowerShell#…

32cd3c0

…5069)

Currently, if a user does not clone with the `--recursive` flag or run `git submodule update --init`, `Start-PSPester` will fail to run due to the missing Pester module. While the error hints at the Pester module not being found, there is no suggested way of fixing the issue presented to the user.
This change makes a helpful warning to appear at the beginning of the execution of `Start-PSPester` if the Pester module cannot be found.

Reviewers

Dongbo Wang (daxian-dbw)

daxian-dbw approved these changes

Aditya Patwardhan (adityapatwardhan)

adityapatwardhan approved these changes

Travis Plunk (TravisEz13)

Awaiting requested review from TravisEz13

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Dongbo Wang (daxian-dbw)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

David (djweber) commented Oct 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

David (djweber) Oct 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

David (djweber) Oct 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

David (djweber) commented Oct 9, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

David (djweber) Oct 9, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

David (djweber) Oct 10, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.