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

Remove DCOM support from *-Computer cmdlets (replacement for 5152) #5277

Conversation

JamesWTruher
Copy link
Member

Since DCOM is not supported in corefx there was a great deal of dead code in the computer cmdlets.
This PR removes all vestiges of DCOM support from:

  • Rename-Computer
  • Restart-Computer
  • Stop-Computer

removing about 4500 lines of dead code. Also, tests are updated to provide more complete coverage.
I also removed test-connection completely to make way for @iSazonov upcoming PR to improve coverage in tests, I created some test hook code which will test the cmdlet code without actually calling the WMI method to restart/rename/stop the system.

this is a replacement for PR 515

Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments.

{
var fieldInfo = typeof(InternalTestHooks).GetField(property, BindingFlags.Static | BindingFlags.NonPublic);
if (fieldInfo != null)
{
fieldInfo.SetValue(null, value);
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove extra line.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

$result.HasSucceeded | should be $true
$result.NewComputerName | should be $newname
$WarnVar | should BeNullOrEmpty

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove extra line.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

{
Disable-Testhook -testhookname TestWaitStopComputer
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove extra line.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Stop-Computer -ErrorVariable StopError 2>$null
$StopError.Exception.Message | Should match 0x300000
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove extra line.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

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.

LGTM

Since DCOM is not supported in corefx there was a great deal of dead code in the computer cmdlets.
This PR removes all vestiges of DCOM support from:

- Rename-Computer
- Restart-Computer
- Stop-Computer

removing about 4500 lines of dead code. Also, tests are updated to provide more complete coverage.
I also removed test-connection completely to make way for @iSazonov upcoming PR to improve coverage in tests, I created some test hook code which will test the cmdlet code without actually calling the WMI method to restart/rename/stop the system
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking-Change breaking change that may affect users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants