★ wanayoo — archive 1999 https://github.com/PowerShell/PowerShell/pull/7869Nouvelle recherche | Portail wanayoo
Skip to content

/PowerShell

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

Test changes needed for running in a container #7869

Merged
merged 2 commits into from Sep 28, 2018

Conversation

Reviewers

@iSazonov iSazonov

@SteveL-MSFT SteveL-MSFT

@adityapatwardhan adityapatwardhan

@TravisEz13 TravisEz13

Assignees

@adityapatwardhan adityapatwardhan

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@JamesWTruher @adityapatwardhan @SteveL-MSFT @iSazonov
@JamesWTruher
Copy link
Member

commented Sep 26, 2018 •
edited

PR Summary

These are changes which will enable our tests to be run within containers for the 9 different Linux platforms that we provide. In order to successfully run in a container an environment variable __InContainer must be present to avoid running the help test which starts a browser. The best place to define this variable is in the Dockerfile, ala:
ENV __InContainer 1

Additionally, these tests may be executed against the already installed PowerShell.

This is the first step in completely automating our tests against packaged (rather than built) PowerShell.

PR Checklist

@JamesWTruher JamesWTruher assigned adityapatwardhan Sep 26, 2018

@JamesWTruher JamesWTruher requested review from TravisEz13 and adityapatwardhan Sep 26, 2018

@adityapatwardhan

adityapatwardhan requested changes Sep 26, 2018
View changes

test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
@@ -19,8 +19,13 @@ Describe "Test-Connection" -tags "CI" {
# $targetAddressIPv6 = "::1"
$targetAddressIPv6 = [System.Net.Dns]::GetHostEntry($targetName).AddressList[0].IPAddressToString
$UnreachableAddress = "10.11.12.13"
$realName = "google-public-dns-a.google.com"
$realAddress = [System.Net.Dns]::GetHostEntry($realName).AddressList[0].IPAddressToString
# this resolves to an actual IP raher than 127.0.0.1

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

typo raher -> rather
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
# this resolves to an actual IP raher than 127.0.0.1
# this can also include both IPv4 and IPv6, so select InterNetwork rather than InternNetworkV6
$realAddress = [System.Net.Dns]::GetHostEntry($hostName).AddressList |
?{$_.AddressFamily -eq "InterNetwork"} |

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

Use Where-Object instead
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
$realName = "google-public-dns-a.google.com"
$realAddress = [System.Net.Dns]::GetHostEntry($realName).AddressList[0].IPAddressToString
# this resolves to an actual IP raher than 127.0.0.1
# this can also include both IPv4 and IPv6, so select InterNetwork rather than InternNetworkV6

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

typo InternNetworkV6 -> InterNetworkV6
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
$realAddress = [System.Net.Dns]::GetHostEntry($hostName).AddressList |
?{$_.AddressFamily -eq "InterNetwork"} |
Select-Object -First 1 |
%{ $_.IPAddressToString }

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

Use ForEach-Object instead
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated

$result2 = Test-Connection $realName -Count 1 -IPv4 -MaxHops 1 -DontFragment
# explicitly go to google dns. this test will pass even in the destination is unreachable

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

typo in -> if
Show resolved Hide resolved ...rshell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
@@ -198,17 +205,17 @@ Describe "Test-Connection" -tags "CI" {
# TODO: We skip the MTUSizeDetect tests on Unix because we expect 'TtlExpired' but get 'TimeOut' internally from .Net Core
# Skipping on VSTS in Windows due to `TimedOut`
Context "MTUSizeDetect" {
It "MTUSizeDetect works" -Pending:(!$isWindows -or (Test-IsVstsWindows)) {
$result = Test-Connection $realName -MTUSizeDetect
It "MTUSizeDetect works" -Pending:(!$isWindows) {

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

Can you double check if this still needs to be maked as Pending. It worked on my Ubuntu 18.04 VM.

This comment has been minimized.

Sign in to view
Copy link
@JamesWTruher

JamesWTruher Sep 27, 2018

Author Member

it's definitely failing on my mac, I'll mark it pending for $IsMacOS
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
$result.Status | Should -BeExactly "Success"
$result.MTUSize | Should -BeGreaterThan 0
}

It "Quiet works" -Pending:(!$isWindows -or (Test-IsVstsWindows)) {
$result = Test-Connection $realName -MTUSizeDetect -Quiet
It "Quiet works" -Pending:(!$isWindows) {

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

Same as above.
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
@@ -237,15 +245,15 @@ Describe "Test-Connection" -tags "CI" {
$pingReplies[0].Address | Should -BeExactly $realAddress
$pingReplies[0].Status | Should -BeExactly "Success"
if (!$isWindows) {
$pingReplies[0].Buffer.Count | Should -Be 0
$pingReplies[0].Buffer.Count | Should -Match '^0$|^32$'
} else {
$pingReplies[0].Buffer.Count | Should -Be 32
}
}

# Hangs in VSTS Linux

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

Can you remove this comment?

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 27, 2018

Member

Please remove comment if it does not hang in VSTS.
test/powershell/engine/Help/HelpSystem.OnlineHelp.Tests.ps1
@@ -48,7 +48,8 @@ Describe 'Online help tests for PowerShell Core Cmdlets' -Tags "CI" {
Describe 'Get-Help -Online opens the default web browser and navigates to the cmdlet help content' -Tags "Feature" {

$skipTest = [System.Management.Automation.Platform]::IsIoT -or
[System.Management.Automation.Platform]::IsNanoServer
[System.Management.Automation.Platform]::IsNanoServer -or
$env:__InContainer -eq 1

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 26, 2018

Member

Where is this defined?

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 27, 2018

Member

Please add a comment about $env:__InContainer

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Sep 27, 2018

Member

The code to open help in a browser is pretty simple. Perhaps rather than skipping these tests in non-desktop environments, we should instead have a test hook to validate that it hit the right code path or even start a test executable passed the URL as an argument?

This comment has been minimized.

Sign in to view
Copy link
@JamesWTruher

JamesWTruher Sep 27, 2018

Author Member

there is a test hook here already, and it is used to validate all the uris. This is a single test which starts the browser.

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Sep 27, 2018

Member

I'm ok with this for now
@adityapatwardhan

adityapatwardhan requested changes Sep 27, 2018
View changes

test/powershell/engine/Help/HelpSystem.OnlineHelp.Tests.ps1
@@ -48,7 +48,8 @@ Describe 'Online help tests for PowerShell Core Cmdlets' -Tags "CI" {
Describe 'Get-Help -Online opens the default web browser and navigates to the cmdlet help content' -Tags "Feature" {

$skipTest = [System.Management.Automation.Platform]::IsIoT -or
[System.Management.Automation.Platform]::IsNanoServer
[System.Management.Automation.Platform]::IsNanoServer -or
$env:__InContainer -eq 1

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 27, 2018

Member

Please add a comment about $env:__InContainer
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1 Outdated
@@ -237,15 +245,15 @@ Describe "Test-Connection" -tags "CI" {
$pingReplies[0].Address | Should -BeExactly $realAddress
$pingReplies[0].Status | Should -BeExactly "Success"
if (!$isWindows) {
$pingReplies[0].Buffer.Count | Should -Be 0
$pingReplies[0].Buffer.Count | Should -Match '^0$|^32$'
} else {
$pingReplies[0].Buffer.Count | Should -Be 32
}
}

# Hangs in VSTS Linux

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan Sep 27, 2018

Member

Please remove comment if it does not hang in VSTS.
@adityapatwardhan

This comment has been minimized.

Sign in to view
Copy link
Member

commented Sep 27, 2018

@JamesWTruher Please also have a look at the test failure in CI for Linux and MacOS.

Also please push a commit with [Feature] to run all tests.

@JamesWTruher JamesWTruher force-pushed the JamesWTruher:containertestfixes branch Sep 27, 2018

@adityapatwardhan

This comment has been minimized.

Sign in to view
Copy link
Member

commented Sep 27, 2018

@JamesWTruher Test-Connection test is failing on Linux and macOS with Exception calling "GetHostEntry" with "1" argument(s): "No such device or address"

Also please push a commit with [Feature]
@SteveL-MSFT

SteveL-MSFT requested changes Sep 27, 2018
View changes

test/powershell/engine/Help/HelpSystem.OnlineHelp.Tests.ps1
@@ -48,7 +48,8 @@ Describe 'Online help tests for PowerShell Core Cmdlets' -Tags "CI" {
Describe 'Get-Help -Online opens the default web browser and navigates to the cmdlet help content' -Tags "Feature" {

$skipTest = [System.Management.Automation.Platform]::IsIoT -or
[System.Management.Automation.Platform]::IsNanoServer
[System.Management.Automation.Platform]::IsNanoServer -or
$env:__InContainer -eq 1

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Sep 27, 2018

Member

The code to open help in a browser is pretty simple. Perhaps rather than skipping these tests in non-desktop environments, we should instead have a test hook to validate that it hit the right code path or even start a test executable passed the URL as an argument?

@JamesWTruher JamesWTruher force-pushed the JamesWTruher:containertestfixes branch Sep 27, 2018

@JamesWTruher

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Sep 27, 2018

@SteveL-MSFT please see my comment above re: starting a browser
@SteveL-MSFT

SteveL-MSFT approved these changes Sep 27, 2018
View changes

@JamesWTruher
[feature] Fix a number of typos 
make the `MTUSizeDetect` tests pending for MacOS only.
Loading status checks…
c7d0f7a

@JamesWTruher JamesWTruher force-pushed the JamesWTruher:containertestfixes branch to c7d0f7a Sep 28, 2018

@adityapatwardhan

adityapatwardhan approved these changes Sep 28, 2018
View changes

@adityapatwardhan adityapatwardhan merged commit 98cf44c into PowerShell:master Sep 28, 2018
8 checks passed

8 checks passed

CodeFactor No issues found.
Details
PowerShell-CI-linux #PR-7869-20180928.01 succeeded
Details
PowerShell-CI-macos #PR-7869-20180928.01 succeeded
Details
PowerShell-CI-spelling #PR-7869-20180928.01 succeeded
Details
PowerShell-CI-windows #PR-7869-20180928.01 succeeded
Details
WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.