Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 45db851

Browse files
committedFeb 28, 2024·
Add sane defaults to Start-EditorServices and fix logs
We do not need to require so many CLI flags. This continues to run the existing Emacs and Vim tests utilizing those flags as a regression scenario, and adds an additional pair of tests that launch PSES with a minimal set of flags. This also fixes the log file path to be unique per process, and to use `LogPath` as a directory instead of a file.
1 parent e8c6dea commit 45db851

File tree

8 files changed

+152
-38
lines changed

8 files changed

+152
-38
lines changed
 

‎.github/workflows/emacs-test.yml

+6-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ jobs:
4141
with:
4242
version: '28.2'
4343

44-
- name: Run ERT
44+
- name: Run ERT with full CLI
4545
run: |
4646
emacs -Q --batch -f package-refresh-contents --eval "(package-install 'eglot)"
4747
emacs -Q --batch -l test/emacs-test.el -f ert-run-tests-batch-and-exit
48+
49+
- name: Run ERT with simple CLI
50+
run: |
51+
emacs -Q --batch -f package-refresh-contents --eval "(package-install 'eglot)"
52+
emacs -Q --batch -l test/emacs-simple-test.el -f ert-run-tests-batch-and-exit

‎.github/workflows/vim-test.yml

+6-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,12 @@ jobs:
6161
repository: thinca/vim-themis
6262
path: vim-themis
6363

64-
- name: Run Themis
64+
- name: Run Themis with full CLI
6565
env:
6666
THEMIS_VIM: ${{ steps.vim.outputs.executable }}
6767
run: ./vim-themis/bin/themis ./test/vim-test.vim
68+
69+
- name: Run Themis with simple CLI
70+
env:
71+
THEMIS_VIM: ${{ steps.vim.outputs.executable }}
72+
run: ./vim-themis/bin/themis ./test/vim-simple-test.vim

‎module/PowerShellEditorServices/Start-EditorServices.ps1

+5-12
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,14 @@
2727
#>
2828
[CmdletBinding(DefaultParameterSetName="NamedPipe")]
2929
param(
30-
[Parameter(Mandatory=$true)]
3130
[ValidateNotNullOrEmpty()]
3231
[string]
3332
$HostName,
3433

35-
[Parameter(Mandatory=$true)]
3634
[ValidateNotNullOrEmpty()]
3735
[string]
3836
$HostProfileId,
3937

40-
[Parameter(Mandatory=$true)]
4138
[ValidateNotNullOrEmpty()]
4239
[string]
4340
$HostVersion,
@@ -52,7 +49,6 @@ param(
5249
[ValidateSet("Diagnostic", "Verbose", "Normal", "Warning", "Error")]
5350
$LogLevel,
5451

55-
[Parameter(Mandatory=$true)]
5652
[ValidateNotNullOrEmpty()]
5753
[string]
5854
$SessionDetailsPath,
@@ -78,20 +74,17 @@ param(
7874
[switch]
7975
$WaitForDebugger,
8076

81-
[switch]
82-
$ConfirmInstall,
83-
84-
[Parameter(ParameterSetName="Stdio", Mandatory=$true)]
77+
[Parameter(ParameterSetName="Stdio", Mandatory)]
8578
[switch]
8679
$Stdio,
8780

8881
[Parameter(ParameterSetName="NamedPipe")]
8982
[string]
90-
$LanguageServicePipeName = $null,
83+
$LanguageServicePipeName,
9184

9285
[Parameter(ParameterSetName="NamedPipe")]
9386
[string]
94-
$DebugServicePipeName = $null,
87+
$DebugServicePipeName,
9588

9689
[Parameter(ParameterSetName="NamedPipeSimplex")]
9790
[switch]
@@ -107,11 +100,11 @@ param(
107100

108101
[Parameter(ParameterSetName="NamedPipeSimplex")]
109102
[string]
110-
$DebugServiceInPipeName = $null,
103+
$DebugServiceInPipeName,
111104

112105
[Parameter(ParameterSetName="NamedPipeSimplex")]
113106
[string]
114-
$DebugServiceOutPipeName = $null
107+
$DebugServiceOutPipeName
115108
)
116109

117110
Import-Module -Name "$PSScriptRoot/PowerShellEditorServices.psd1"

‎src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs

+25-21
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ public sealed class StartEditorServicesCommand : PSCmdlet
3333

3434
private HostLogger _logger;
3535

36+
// NOTE: Ignore the suggestion to use Environment.ProcessId as it doesn't work for
37+
// .NET 4.6.2 (for Windows PowerShell), and this won't be caught in CI.
38+
private static readonly int s_currentPID = Process.GetCurrentProcess().Id;
39+
3640
public StartEditorServicesCommand()
3741
{
3842
// Sets the distribution channel to "PSES" so starts can be distinguished in PS7+ telemetry
@@ -44,30 +48,30 @@ public StartEditorServicesCommand()
4448
/// <summary>
4549
/// The name of the EditorServices host to report.
4650
/// </summary>
47-
[Parameter(Mandatory = true)]
51+
[Parameter]
4852
[ValidateNotNullOrEmpty]
49-
public string HostName { get; set; }
53+
public string HostName { get; set; } = "PSES";
5054

5155
/// <summary>
5256
/// The ID to give to the host's profile.
5357
/// </summary>
54-
[Parameter(Mandatory = true)]
58+
[Parameter]
5559
[ValidateNotNullOrEmpty]
56-
public string HostProfileId { get; set; }
60+
public string HostProfileId { get; set; } = "PSES";
5761

5862
/// <summary>
5963
/// The version to report for the host.
6064
/// </summary>
61-
[Parameter(Mandatory = true)]
65+
[Parameter]
6266
[ValidateNotNullOrEmpty]
63-
public Version HostVersion { get; set; }
67+
public Version HostVersion { get; set; } = new Version(0, 0, 0);
6468

6569
/// <summary>
6670
/// Path to the session file to create on startup or startup failure.
6771
/// </summary>
68-
[Parameter(Mandatory = true)]
72+
[Parameter]
6973
[ValidateNotNullOrEmpty]
70-
public string SessionDetailsPath { get; set; }
74+
public string SessionDetailsPath { get; set; } = "PowerShellEditorServices.json";
7175

7276
/// <summary>
7377
/// The name of the named pipe to use for the LSP transport.
@@ -120,14 +124,16 @@ public StartEditorServicesCommand()
120124
/// </summary>
121125
[Parameter]
122126
[ValidateNotNullOrEmpty]
123-
public string BundledModulesPath { get; set; }
127+
public string BundledModulesPath { get; set; } = Path.GetFullPath(Path.Combine(
128+
Path.GetDirectoryName(typeof(StartEditorServicesCommand).Assembly.Location),
129+
"..", "..", ".."));
124130

125131
/// <summary>
126-
/// The absolute path to the where the editor services log file should be created and logged to.
132+
/// The absolute path to the folder where logs will be saved.
127133
/// </summary>
128134
[Parameter]
129135
[ValidateNotNullOrEmpty]
130-
public string LogPath { get; set; }
136+
public string LogPath { get; set; } = Path.Combine(Path.GetTempPath(), "PowerShellEditorServices");
131137

132138
/// <summary>
133139
/// The minimum log level that should be emitted.
@@ -266,34 +272,32 @@ private void StartLogging()
266272
}
267273

268274
string logDirPath = GetLogDirPath();
269-
string logPath = Path.Combine(logDirPath, "StartEditorServices.log");
275+
string logPath = Path.Combine(logDirPath, $"StartEditorServices-{s_currentPID}.log");
270276

271-
// Temp debugging sessions may try to reuse this same path,
272-
// so we ensure they have a unique path
273277
if (File.Exists(logPath))
274278
{
275279
int randomInt = new Random().Next();
276-
logPath = Path.Combine(logDirPath, $"StartEditorServices-temp{randomInt.ToString("X", CultureInfo.InvariantCulture.NumberFormat)}.log");
280+
logPath = Path.Combine(logDirPath, $"StartEditorServices-{s_currentPID}-{randomInt.ToString("X", CultureInfo.InvariantCulture.NumberFormat)}.log");
277281
}
278282

279283
StreamLogger fileLogger = StreamLogger.CreateWithNewFile(logPath);
280284
_disposableResources.Add(fileLogger);
281285
IDisposable fileLoggerUnsubscriber = _logger.Subscribe(fileLogger);
282286
fileLogger.AddUnsubscriber(fileLoggerUnsubscriber);
283287
_loggerUnsubscribers.Add(fileLoggerUnsubscriber);
284-
285288
_logger.Log(PsesLogLevel.Diagnostic, "Logging started");
286289
}
287290

291+
// Sanitizes user input and ensures the directory is created.
288292
private string GetLogDirPath()
289293
{
290-
string logDir = !string.IsNullOrEmpty(LogPath)
291-
? Path.GetDirectoryName(LogPath)
292-
: Path.GetDirectoryName(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location));
294+
string logDir = LogPath;
295+
if (string.IsNullOrEmpty(logDir))
296+
{
297+
logDir = Path.Combine(Path.GetTempPath(), "PowerShellEditorServices");
298+
}
293299

294-
// Ensure logDir exists
295300
Directory.CreateDirectory(logDir);
296-
297301
return logDir;
298302
}
299303

‎src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs

+6-2
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,15 @@ internal sealed class EditorServicesServerFactory : IDisposable
4343
/// constructor? In the end it returns an instance (albeit a "singleton").
4444
/// </para>
4545
/// </remarks>
46-
/// <param name="logPath">The path of the log file to use.</param>
46+
/// <param name="logDirectoryPath">The path of the log file to use.</param>
4747
/// <param name="minimumLogLevel">The minimum log level to use.</param>
4848
/// <param name="hostLogger">The host logger?</param>
49-
public static EditorServicesServerFactory Create(string logPath, int minimumLogLevel, IObservable<(int logLevel, string message)> hostLogger)
49+
public static EditorServicesServerFactory Create(string logDirectoryPath, int minimumLogLevel, IObservable<(int logLevel, string message)> hostLogger)
5050
{
51+
// NOTE: Ignore the suggestion to use Environment.ProcessId as it doesn't work for
52+
// .NET 4.6.2 (for Windows PowerShell), and this won't be caught in CI.
53+
int currentPID = Process.GetCurrentProcess().Id;
54+
string logPath = Path.Combine(logDirectoryPath, $"PowerShellEditorServices-{currentPID}.log");
5155
Log.Logger = new LoggerConfiguration()
5256
.Enrich.FromLogContext()
5357
.WriteTo.Async(config => config.File(logPath))

‎test/emacs-simple-test.el

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
;;; emacs-test.el --- Integration testing script -*- lexical-binding: t; -*-
2+
3+
;; Copyright (c) Microsoft Corporation.
4+
;; Licensed under the MIT License.
5+
6+
;; Author: Andy Jordan <andy.jordan@microsoft.com>
7+
;; Keywords: PowerShell, LSP
8+
9+
;;; Code:
10+
11+
;; Avoid using old packages.
12+
(setq load-prefer-newer t)
13+
14+
;; Improved TLS Security.
15+
(with-eval-after-load 'gnutls
16+
(custom-set-variables
17+
'(gnutls-verify-error t)
18+
'(gnutls-min-prime-bits 3072)))
19+
20+
;; Package setup.
21+
(require 'package)
22+
(add-to-list 'package-archives
23+
'("melpa" . "https://melpa.org/packages/") t)
24+
(package-initialize)
25+
(package-refresh-contents)
26+
27+
(require 'ert)
28+
29+
(require 'flymake)
30+
31+
(unless (package-installed-p 'powershell)
32+
(package-install 'powershell))
33+
(require 'powershell)
34+
35+
(unless (package-installed-p 'eglot)
36+
(package-install 'eglot))
37+
(require 'eglot)
38+
39+
(ert-deftest powershell-editor-services ()
40+
"Eglot should connect to PowerShell Editor Services."
41+
(let* ((repo (project-root (project-current)))
42+
(start-script (expand-file-name "module/PowerShellEditorServices/Start-EditorServices.ps1" repo))
43+
(test-script (expand-file-name "test/PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1" repo))
44+
(eglot-sync-connect t))
45+
(add-to-list
46+
'eglot-server-programs
47+
`(powershell-mode
48+
. ("pwsh" "-NoLogo" "-NoProfile" "-Command" ,start-script "-Stdio")))
49+
(with-current-buffer (find-file-noselect test-script)
50+
(should (eq major-mode 'powershell-mode))
51+
(should (apply #'eglot--connect (eglot--guess-contact)))
52+
(should (eglot-current-server))
53+
(let ((lsp (eglot-current-server)))
54+
(should (string= (eglot--project-nickname lsp) "PowerShellEditorServices"))
55+
(should (member (cons 'powershell-mode "powershell") (eglot--languages lsp))))
56+
(sleep-for 5) ; TODO: Wait for "textDocument/publishDiagnostics" instead
57+
(flymake-start)
58+
(goto-char (point-min))
59+
(flymake-goto-next-error)
60+
(should (eq 'flymake-warning (face-at-point))))))
61+
62+
(provide 'emacs-test)
63+
;;; emacs-test.el ends here

‎test/vim-simple-test.vim

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
let s:suite = themis#suite('pses')
2+
let s:assert = themis#helper('assert')
3+
4+
function s:suite.before()
5+
let l:pses_path = g:repo_root . '/module'
6+
let g:LanguageClient_serverCommands = {
7+
\ 'ps1': [ 'pwsh', '-NoLogo', '-NoProfile', '-Command',
8+
\ l:pses_path . '/PowerShellEditorServices/Start-EditorServices.ps1', '-Stdio' ]
9+
\ }
10+
let g:LanguageClient_serverStderr = 'DEBUG'
11+
let g:LanguageClient_loggingFile = g:repo_root . '/LanguageClient.log'
12+
let g:LanguageClient_serverStderr = g:repo_root . '/LanguageServer.log'
13+
endfunction
14+
15+
function s:suite.has_language_client()
16+
call s:assert.includes(&runtimepath, g:repo_root . '/LanguageClient-neovim')
17+
call s:assert.cmd_exists('LanguageClientStart')
18+
call s:assert.not_empty(g:LanguageClient_serverCommands)
19+
call s:assert.true(LanguageClient#HasCommand('ps1'))
20+
endfunction
21+
22+
function s:suite.analyzes_powershell_file()
23+
view test/vim-test.ps1 " This must not use quotes!
24+
25+
let l:bufnr = bufnr('vim-test.ps1$')
26+
call s:assert.not_equal(l:bufnr, -1)
27+
let l:bufinfo = getbufinfo(l:bufnr)[0]
28+
29+
call s:assert.equal(l:bufinfo.name, g:repo_root . '/test/vim-test.ps1')
30+
call s:assert.includes(getbufline(l:bufinfo.name, 1), 'function Do-Work {}')
31+
" TODO: This shouldn't be necessary, vim-ps1 works locally but not in CI.
32+
call setbufvar(l:bufinfo.bufnr, '&filetype', 'ps1')
33+
call s:assert.equal(getbufvar(l:bufinfo.bufnr, '&filetype'), 'ps1')
34+
35+
execute 'LanguageClientStart'
36+
execute 'sleep' 5
37+
call s:assert.equal(getbufvar(l:bufinfo.name, 'LanguageClient_isServerRunning'), 1)
38+
call s:assert.equal(getbufvar(l:bufinfo.name, 'LanguageClient_projectRoot'), g:repo_root)
39+
call s:assert.equal(getbufvar(l:bufinfo.name, 'LanguageClient_statusLineDiagnosticsCounts'), {'E': 0, 'W': 1, 'H': 0, 'I': 0})
40+
endfunction

‎test/vim-test.vim

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ let s:assert = themis#helper('assert')
44
function s:suite.before()
55
let l:pses_path = g:repo_root . '/module'
66
let g:LanguageClient_serverCommands = {
7-
\ 'ps1': ['pwsh', '-NoLogo', '-NoProfile', '-Command',
7+
\ 'ps1': [ 'pwsh', '-NoLogo', '-NoProfile', '-Command',
88
\ l:pses_path . '/PowerShellEditorServices/Start-EditorServices.ps1',
99
\ '-HostName', 'vim', '-HostProfileId', 'vim', '-HostVersion', '1.0.0',
1010
\ '-BundledModulesPath', l:pses_path, '-Stdio',

0 commit comments

Comments
 (0)
Please sign in to comment.