Skip to content

Execute interpolated Pester describe blocks if PSES signals that it is possible #1713

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

Closed
wants to merge 4 commits into from

Conversation

bergmeister
Copy link
Contributor

@bergmeister bergmeister commented Jan 22, 2019

PR Summary

Fixes #1710
This PR requires this PSES to be merged as a pre-requisite as PSES sends us info if the installed Pester version supports this.
In the future it would be nice to enhance the prompt to ask the user to install the latest version of Pester for them if they are on an old version, for the moment, I only added some info to make them aware.

image

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • PR has tests - Tested manually (various scenarios: 4.6.0 is installed or not, check that it does not break the file tab menus and commands and check that run/debug works as expected)
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

Copy link
Contributor

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

LGTM

const answer = await vscode.window.showErrorMessage(
"This Describe block's TestName parameter cannot be evaluated. " +
"This Describe block's TestName parameter cannot be evaluated in versions of Pester before 4.6.0. " +
Copy link
Member

@TylerLeonhardt TylerLeonhardt Jan 29, 2019

Choose a reason for hiding this comment

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

Maybe something like this is a little more actionable?

Suggested change
"This Describe block's TestName parameter cannot be evaluated in versions of Pester before 4.6.0. " +
"This Describe block's TestName could not be parsed. Either try again with Pester 4.6.0 or higher, or remove any inline parameters in your TestName." +

Thoughts @rkeithhill?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, something like this but it might ne even better if the ending is more like or remove any variable expressions in the Describe block name

@rkeithhill
Copy link
Contributor

@bergmeister See PR #1776 as an alternative approach. By moving the Pester version detection logic into an InvokePesterStub.ps1 script that ships with the extension, you should be able to vastly simplify the PSES PR that returns the line number. There will be no need to detect the Pester version in the PR.

@rkeithhill
Copy link
Contributor

I propose that we close this PR in favor of PR #1776

@bergmeister bergmeister closed this Mar 3, 2019
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.

Execute Pester Describe block using interpolated string using new Pester 4.6 parameter
4 participants