-
Notifications
You must be signed in to change notification settings - Fork 223
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
Add simple integration test, with an LSP client PowerShell module #944
Changes from all commits
c708a1b
e36001a
9e67dea
b66a9ac
6711016
be4587e
2158e3b
4952bad
7ba21c2
558abc0
5c35561
81631a1
adcb527
b16429d
0435cb6
7e9a76d
9b66b61
d949558
b061b65
6b13e22
29ccd29
2896933
7b3ee70
5a2b3d0
3bb6df4
228b5eb
516cbb0
beeef02
dae7511
f8654d6
14b8884
4069a40
a6e2e99
e8529dd
b0d704c
1b90683
c0bcefb
d9b3a0b
c48b586
e2e3c9b
a7e599c
47cdb90
d5a67c2
dc2506e
135d47d
37ddedf
9f5666f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
|
||
$script:ExceptionRegex = [regex]::new('\s*Exception: (.*)$', 'Compiled,Multiline,IgnoreCase') | ||
function ReportLogErrors | ||
{ | ||
param( | ||
[Parameter()][string]$LogPath, | ||
|
||
[Parameter()][ref]<#[int]#>$FromIndex = 0, | ||
|
||
[Parameter()][string[]]$IgnoreException = @() | ||
) | ||
|
||
$logEntries = Parse-PsesLog $LogPath | | ||
Where-Object Index -ge $FromIndex.Value | ||
|
||
# Update the index to the latest in the log | ||
$FromIndex.Value = ($FromIndex.Value,$errorLogs.Index | Measure-Object -Maximum).Maximum | ||
|
||
$errorLogs = $logEntries | | ||
Where-Object LogLevel -eq Error | | ||
Where-Object { | ||
$match = $script:ExceptionRegex.Match($_.Message.Data) | ||
|
||
(-not $match) -or ($match.Groups[1].Value.Trim() -notin $IgnoreException) | ||
} | ||
|
||
if ($errorLogs) | ||
{ | ||
$errorLogs | ForEach-Object { Write-Error "ERROR from PSES log: $($_.Message.Data)" } | ||
} | ||
} | ||
|
||
Describe "Loading and running PowerShellEditorServices" { | ||
BeforeAll { | ||
Import-Module -Force "$PSScriptRoot/../../module/PowerShellEditorServices" | ||
Import-Module -Force "$PSScriptRoot/../../tools/PsesPsClient/out/PsesPsClient" | ||
Import-Module -Force "$PSScriptRoot/../../tools/PsesLogAnalyzer" | ||
|
||
$logIdx = 0 | ||
$psesServer = Start-PsesServer | ||
$client = Connect-PsesServer -PipeName $psesServer.SessionDetails.languageServicePipeName | ||
} | ||
|
||
# This test MUST be first | ||
It "Starts and responds to an initialization request" { | ||
$request = Send-LspInitializeRequest -Client $client | ||
$response = Get-LspResponse -Client $client -Id $request.Id | ||
$response.Id | Should -BeExactly $request.Id | ||
|
||
ReportLogErrors -LogPath $psesServer.LogPath -FromIndex ([ref]$logIdx) | ||
rjmholt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
# This test MUST be last | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if this is the right approach. Maybe it should be 2
So the tests would be structured like this: Describe "Loading and running PowerShellEditorServices" {
BeforeAll {
Import-Module -Force "$PSScriptRoot/../../module/PowerShellEditorServices"
Import-Module -Force "$PSScriptRoot/../../tools/PsesPsClient/out/PsesPsClient"
Import-Module -Force "$PSScriptRoot/../../tools/PsesLogAnalyzer"
$logIdx = 0
}
Context "Startup and shutdown" {
It "Can start up and shutdown" {
# Start up
$psesServer = Start-PsesServer
$client = Connect-PsesServer -PipeName $psesServer.SessionDetails.languageServicePipeName
# Shutdown
$request = Send-LspShutdownRequest -Client $client
$response = Get-LspResponse -Client $client -Id $request.Id
$response.Id | Should -BeExactly $request.Id
$response.Result | Should -BeNull
# TODO: The server seems to stay up waiting for the debug connection
# $psesServer.PsesProcess.HasExited | Should -BeTrue
try
{
$psesServer.PsesProcess.Kill()
}
finally
{
try
{
$psesServer.PsesProcess.Dispose()
}
finally
{
$client.Dispose()
}
}
ReportLogErrors -LogPath $psesServer.LogPath -FromIndex ([ref]$logIdx)
}
}
Context "LSP messages" {
BeforeAll {
# Start up
$psesServer = Start-PsesServer
$client = Connect-PsesServer -PipeName $psesServer.SessionDetails.languageServicePipeName
}
AfterAll {
try
{
$psesServer.PsesProcess.Kill()
}
finally
{
try
{
$psesServer.PsesProcess.Dispose()
}
finally
{
$client.Dispose()
}
}
}
It "Can send initialize request" {
$request = Send-LspInitializeRequest -Client $client
$response = Get-LspResponse -Client $client -Id $request.Id
$response.Id | Should -BeExactly $request.Id
ReportLogErrors -LogPath $psesServer.LogPath -FromIndex ([ref]$logIdx)
}
#
# As time goes on we add more It's here for the other messages
#
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That way the order doesn't matter and we can easily add new tests for other messages. For the future: We'll get there. Gotta figure out which messages depend on what and align the tests as much as possible to that back and forth. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I don't think there's any particularly obvious approach available at the moment. I think given that we're not planning impending development on this module that we should merge what we have since it works and is less code and then continue as is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely don't disagree with you though -- "unit" testing is very bad at sequential logic with dependent states. I think we should refactor as you say, but will defer that to another PR. |
||
It "Shuts down the process properly" { | ||
$request = Send-LspShutdownRequest -Client $client | ||
$response = Get-LspResponse -Client $client -Id $request.Id | ||
$response.Id | Should -BeExactly $request.Id | ||
$response.Result | Should -BeNull | ||
# TODO: The server seems to stay up waiting for the debug connection | ||
# $psesServer.PsesProcess.HasExited | Should -BeTrue | ||
|
||
# We close the process here rather than in an AfterAll | ||
# since errors can occur and we want to test for them. | ||
# Naturally this depends on Pester executing tests in order. | ||
|
||
# We also have to dispose of everything properly, | ||
# which means we have to use these cascading try/finally statements | ||
try | ||
{ | ||
$psesServer.PsesProcess.Kill() | ||
} | ||
finally | ||
{ | ||
try | ||
{ | ||
$psesServer.PsesProcess.Dispose() | ||
} | ||
finally | ||
{ | ||
$client.Dispose() | ||
} | ||
} | ||
|
||
ReportLogErrors -LogPath $psesServer.LogPath -FromIndex ([ref]$logIdx) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just add this to the
#Requires
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of
#requires
in general, but I think here we might want to build without testing in some circumstances