Fix wrong output for `New-Service` in variable assignment and `-OutVariable` by kvprasoon · Pull Request #10444 · PowerShell/PowerShell · GitHub

New-Service cmdlet's output doesn't have all properties when assigned to a variable or using -OutVariable.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
Breaking changes
- None
- OR
- Experimental feature(s) needed
  - Experimental feature name(s):
User-facing changes
- Not Applicable
- OR
- Documentation needed
  - Issue filed: Wrong output for New-Service in variable assignment and -OutVariable #10443
Testing - New and feature
- N/A or can only be tested interactively
- OR
- Make sure you've added a new test if existing tests do not effectively test the code changed
Tooling
- I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
- OR
- I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
  - Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
  - Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
  - Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
  - Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

Disclaimer: Novice in c# and really don't know the impact of the change, the code change was by referring similar usage in the same file.

CI failure seems to be something different.

PRASOON KARUNAN V (@kvprasoon) you might need to look at how the test is constructed. If the test is expecting the object to be disposed, it might explain why the tests are erroring out.

PRASOON KARUNAN V (@kvprasoon) Of the three errors, I think the last failure may be expected based on how your test is written.

When you create a new service with a $null display name, the display name gets assigned the value in the name field. If you look at the error output, it indicates you're checking if the display name is $null and that fails because the actual value of the display name is the service name value. You'll have to update your Pester test for display name to ensure that it checks against the value of the name property if the display name is $null.

For the first two errors, they indicate that the display name is null when it is not. I wonder if there is a timing issue (i.e. I wonder if the service display name property value does not show up immediately after creating the service). You should try creating some services outside of Pester, show their properties when the new service objects come back, then invoke Get-Service to get the services, and see if the properties are set the way you expect as well. Plus, experiment with timing of those a bit.

Kirk Munro (@KirkMunro) Rain Sallow (/u/ta11ow) (@vexx32) , My earlier comment was for the previous commit, for latest one (not current), the output doesn't contain description and the StartupType in output is StartType, corrected it.

Rather than make this check conditional such that it only runs some of the time, how about just making it check for the appropriate value and having it run all the time?

$service.displayname | Should -Be $(if ($displayname) {$displayname} else {$name})

Like the idea, but not really a fan of that particular syntax.

Maybe:

$testvalue = if ($DisplayName) { $displayname } else { $name }
$service.Displayname | should -BeExactly $testvalue

There is absolutely nothing wrong with using a subexpression here.

Didn't say it was wrong, just overly long and complicated for a single line. 🙂

I couldn't disagree more, and the suggestion that the proposed line was "overly long and complicated" is misinformation.

With a condition as simple as this one, it is far better to keep the code focused on the tests being executed by having the condition inline than have more code to look over and more variables to think about.

As an added bonus, once we get a null-coalescing operator, inline code such as what was proposed becomes much, much simpler.

Rather than make this check conditional such that it only runs some of the time, how about just making it check for the appropriate value and having it run all the time?
$service.displayname | Should -Be $(if ($displayname) {$displayname} else {$name})

This will implicitly add the check, if DisplayName is null, then ServiceName becomes DisplayName

LGTM

…riable` (PowerShell#10444)

🎉v7.0.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

…riable` (PowerShell#10444)

PRASOON KARUNAN V (kvprasoon) added 4 commits August 25, 2019 15:57

remove using Using

92ad20a

add tests

3449d0b

add tests

a1a30fa

correct path

87e75d9

PRASOON KARUNAN V (kvprasoon) requested review from Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) as code owners August 25, 2019 11:04

PRASOON KARUNAN V (kvprasoon) added 2 commits August 25, 2019 16:38

code factor fix

dde12a4

adding to existing tests

d8b4fe6

Rain Sallow (/u/ta11ow) (vexx32) reviewed Aug 25, 2019

View reviewed changes

Comment thread src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs Outdated

removing stale comment

1b112d7

fix tests

74e6a61

Kirk Munro (KirkMunro) reviewed Aug 26, 2019

View reviewed changes

refactor assertion

f4daab1

Kirk Munro (KirkMunro) approved these changes Aug 26, 2019

View reviewed changes

Aditya Patwardhan (adityapatwardhan) approved these changes Sep 3, 2019

View reviewed changes

Aditya Patwardhan (adityapatwardhan) changed the title Fix: Wrong output for New-Service in variable assignment and -OutVariable Fix wrong output for New-Service in variable assignment and -OutVariable Sep 3, 2019

Aditya Patwardhan (adityapatwardhan) merged commit f687702 into PowerShell:master Sep 3, 2019

PRASOON KARUNAN V (kvprasoon) deleted the new-srv-bug branch September 3, 2019 18:15

Travis Plunk (TravisEz13) added this to the 7.0.0-preview.4 milestone Sep 5, 2019

Travis Plunk (TravisEz13) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Sep 5, 2019

Ilya (iSazonov) assigned Aditya Patwardhan (adityapatwardhan) Sep 6, 2019

Travis Plunk (TravisEz13) pushed a commit to TravisEz13/PowerShell that referenced this pull request Sep 14, 2019

Fix wrong output for New-Service in variable assignment and `-OutVa…

6aa9050

…riable` (PowerShell#10444)

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026

Fix wrong output for New-Service in variable assignment and `-OutVa…

218c5f5

…riable` (PowerShell#10444)

Fix wrong output for `New-Service` in variable assignment and `-OutVariable` by kvprasoon · Pull Request #10444 · PowerShell/PowerShell · GitHub

Uh oh!

Fix wrong output for New-Service in variable assignment and -OutVariable - #10444

Fix wrong output for New-Service in variable assignment and -OutVariable#10444Aditya Patwardhan (adityapatwardhan) merged 9 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom kvprasoon:new-srv-bugkvprasoon/PowerShell:new-srv-bugCopy head branch name to clipboard

Conversation

PRASOON KARUNAN V (kvprasoon) commented Aug 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

PRASOON KARUNAN V (kvprasoon) commented Aug 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PRASOON KARUNAN V (kvprasoon) commented Aug 25, 2019

Uh oh!

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Aug 25, 2019

Uh oh!

Kirk Munro (KirkMunro) commented Aug 26, 2019

Uh oh!

PRASOON KARUNAN V (kvprasoon) commented Aug 26, 2019

Uh oh!

Kirk Munro (KirkMunro) Aug 26, 2019

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) Aug 26, 2019

Choose a reason for hiding this comment

Uh oh!

Kirk Munro (KirkMunro) Aug 26, 2019

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) Aug 26, 2019

Choose a reason for hiding this comment

Uh oh!

Kirk Munro (KirkMunro) Aug 26, 2019

Choose a reason for hiding this comment

Uh oh!

PRASOON KARUNAN V (kvprasoon) Aug 26, 2019 • 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!

Kirk Munro (KirkMunro) left a comment

Choose a reason for hiding this comment

Uh oh!

Deleted user (ghost) commented Sep 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fix wrong output for New-Service in variable assignment and -OutVariable#10444
Aditya Patwardhan (adityapatwardhan) merged 9 commits into
PowerShell:masterfrom
kvprasoon:new-srv-bug

PRASOON KARUNAN V (kvprasoon) commented Aug 25, 2019 •
edited
Loading

PRASOON KARUNAN V (kvprasoon) commented Aug 25, 2019 •
edited
Loading

PRASOON KARUNAN V (kvprasoon) Aug 26, 2019 •
edited
Loading