Skip to content

Update ConciseView to use TargetObject if applicable #11075

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 3 commits into from
Dec 10, 2019

Conversation

SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Nov 14, 2019

PR Summary

If the ErrorRecord contains a TargetObject, try to use that instead of InvocationInfo where it's not as useful to the user.

Before:

PS> 1 + 1 | should -be 3
InvalidResult: /Users/steve/.local/share/powershell/Modules/Pester/4.4.0/Functions/Assertions/Should.ps1
Line |
 206 |         throw ( New-ShouldErrorRecord -Message $testResult.FailureMessage -File $file -Line $lineNumber -LineText $lineText )
     |         ^ Expected 3, but got 2.

After:

PS> 1 + 1 | should -be 3
InvalidResult:
Line |
   1 | 1 + 1 | should -be 3
     | ^ Expected 3, but got 2.

PR Context

PR Checklist

@SteveL-MSFT
Copy link
Member Author

@PoshChan please retry static

@PoshChan
Copy link
Collaborator

@SteveL-MSFT, successfully started retry of PowerShell-CI-static-analysis

Copy link
Member

@adityapatwardhan adityapatwardhan left a comment

Choose a reason for hiding this comment

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

Signed off as maintainer. Would rely of area expert for code review.

Copy link
Collaborator

@JamesWTruher JamesWTruher left a comment

Choose a reason for hiding this comment

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

I'm still uncomfortable with the obvious special handling for Pester

@adityapatwardhan
Copy link
Member

@SteveL-MSFT Please resolve merge conflict

@SteveL-MSFT SteveL-MSFT added this to the rc.1-consider milestone Dec 7, 2019
@adityapatwardhan adityapatwardhan merged commit 6e4da24 into PowerShell:master Dec 10, 2019
@TravisEz13 TravisEz13 modified the milestones: rc.1-approved, 7.0.0-rc.1 Dec 11, 2019
@daxian-dbw daxian-dbw added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Dec 14, 2019
@ghost
Copy link

ghost commented Dec 16, 2019

🎉v7.0.0-rc.1 has been released which incorporates this pull request.:tada:

Handy links:

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

Successfully merging this pull request may close these issues.

6 participants