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.

Sign up for GitHub

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

Jump to bottom

Check state and report reason if it's not "opened" #11574

Merged
merged 6 commits into from Jan 30, 2020
Merged

Check state and report reason if it's not "opened" #11574

merged 6 commits into from Jan 30, 2020

Conversation

TylerLeonhardt
Copy link
Member

@TylerLeonhardt TylerLeonhardt commented Jan 14, 2020 •
edited

PR Summary

Occasionally, the "Can enter using NamedPipeConnectionInfo" test fails. This will allow us to collect more information about the failure.

PR Context

PR Checklist

@TylerLeonhardt TylerLeonhardt changed the title wait for runspace to be opened Check state and report reason if it's not "opened" Jan 14, 2020
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 14, 2020
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 14, 2020
@TylerLeonhardt TylerLeonhardt marked this pull request as ready for review January 14, 2020 01:20
@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Jan 14, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Jan 14, 2020
@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart windows

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-Windows

@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart macos

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-macOS

@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart linux

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-Linux

@TylerLeonhardt
Copy link
Member Author

I'm running CI a few more times to see if I can get it to fail

TylerLeonhardt
TylerLeonhardt commented Jan 14, 2020
View reviewed changes
test/powershell/Modules/Microsoft.PowerShell.Core/Enter-PSHostProcess.Tests.ps1 Outdated
$rs.Dispose()
$ps.Dispose()
${rs}?.Dispose()
${ps}?.Dispose()
Copy link
Member Author

Choose a reason for hiding this comment

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

@adityapatwardhan 😎

Copy link
Member

Choose a reason for hiding this comment

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

Don't use experimental feature in tests :(
This will not work in a RC/GA release if this feature is not made permanent. Lets refrain from taking dependencies on experimental features in tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to normal null check.

@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart all

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart all

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart all

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart all

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

@TylerLeonhardt
Copy link
Member Author

@PoshChan please restart all

@PoshChan
Copy link
Collaborator

@TylerLeonhardt, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

@TylerLeonhardt
Copy link
Member Author

Alright it's not failing. Let's get this in and when it does fail we'll have more knowledge as to why

adityapatwardhan
adityapatwardhan requested changes Jan 30, 2020
View reviewed changes
test/powershell/Modules/Microsoft.PowerShell.Core/Enter-PSHostProcess.Tests.ps1 Outdated
$rs.Dispose()
$ps.Dispose()
${rs}?.Dispose()
${ps}?.Dispose()
Copy link
Member

Choose a reason for hiding this comment

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

Don't use experimental feature in tests :(
This will not work in a RC/GA release if this feature is not made permanent. Lets refrain from taking dependencies on experimental features in tests.

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 30, 2020
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 30, 2020
@adityapatwardhan adityapatwardhan merged commit 6911aa3 into PowerShell:master Jan 30, 2020
@ghost
Copy link

ghost commented Mar 26, 2020

🎉v7.1.0-preview.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
Reviewers

@PoshChan PoshChan PoshChan left review comments

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@PaulHigin PaulHigin PaulHigin approved these changes

@iSazonov iSazonov iSazonov approved these changes

Assignees

@adityapatwardhan adityapatwardhan

Labels
CL-Test Indicates that a PR should be marked as a test change in the Change Log
Projects
None yet
Milestone
7.1.0-preview.1
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@TylerLeonhardt @PoshChan @adityapatwardhan @PaulHigin @iSazonov @daxian-dbw