Skip to content

Commit

Permalink
Add sane defaults to Start-EditorServices and fix logs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
andyleejordan committed Feb 28, 2024
1 parent f77d222 commit fe7fe5a
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 38 deletions.
7 changes: 6 additions & 1 deletion .github/workflows/emacs-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ jobs:
with:
version: '28.2'

- name: Run ERT
- name: Run ERT with full CLI
run: |
emacs -Q --batch -f package-refresh-contents --eval "(package-install 'eglot)"
emacs -Q --batch -l test/emacs-test.el -f ert-run-tests-batch-and-exit
- name: Run ERT with simple CLI
run: |
emacs -Q --batch -f package-refresh-contents --eval "(package-install 'eglot)"
emacs -Q --batch -l test/emacs-simple-test.el -f ert-run-tests-batch-and-exit
7 changes: 6 additions & 1 deletion .github/workflows/vim-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ jobs:
repository: thinca/vim-themis
path: vim-themis

- name: Run Themis
- name: Run Themis with full CLI
env:
THEMIS_VIM: ${{ steps.vim.outputs.executable }}
run: ./vim-themis/bin/themis ./test/vim-test.vim

- name: Run Themis with simple CLI
env:
THEMIS_VIM: ${{ steps.vim.outputs.executable }}
run: ./vim-themis/bin/themis ./test/vim-simple-test.vim
17 changes: 5 additions & 12 deletions module/PowerShellEditorServices/Start-EditorServices.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,14 @@
#>
[CmdletBinding(DefaultParameterSetName="NamedPipe")]
param(
[Parameter(Mandatory=$true)]
[ValidateNotNullOrEmpty()]
[string]
$HostName,

[Parameter(Mandatory=$true)]
[ValidateNotNullOrEmpty()]
[string]
$HostProfileId,

[Parameter(Mandatory=$true)]
[ValidateNotNullOrEmpty()]
[string]
$HostVersion,
Expand All @@ -52,7 +49,6 @@ param(
[ValidateSet("Diagnostic", "Verbose", "Normal", "Warning", "Error")]
$LogLevel,

[Parameter(Mandatory=$true)]
[ValidateNotNullOrEmpty()]
[string]
$SessionDetailsPath,
Expand All @@ -78,20 +74,17 @@ param(
[switch]
$WaitForDebugger,

[switch]
$ConfirmInstall,

[Parameter(ParameterSetName="Stdio", Mandatory=$true)]
[Parameter(ParameterSetName="Stdio", Mandatory)]
[switch]
$Stdio,

[Parameter(ParameterSetName="NamedPipe")]
[string]
$LanguageServicePipeName = $null,
$LanguageServicePipeName,

[Parameter(ParameterSetName="NamedPipe")]
[string]
$DebugServicePipeName = $null,
$DebugServicePipeName,

[Parameter(ParameterSetName="NamedPipeSimplex")]
[switch]
Expand All @@ -107,11 +100,11 @@ param(

[Parameter(ParameterSetName="NamedPipeSimplex")]
[string]
$DebugServiceInPipeName = $null,
$DebugServiceInPipeName,

[Parameter(ParameterSetName="NamedPipeSimplex")]
[string]
$DebugServiceOutPipeName = $null
$DebugServiceOutPipeName
)

Import-Module -Name "$PSScriptRoot/PowerShellEditorServices.psd1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ public sealed class StartEditorServicesCommand : PSCmdlet

private HostLogger _logger;

// NOTE: Ignore the suggestion to use Environment.ProcessId as it doesn't work for
// .NET 4.6.2 (for Windows PowerShell), and this won't be caught in CI.
private static readonly int s_currentPID = Process.GetCurrentProcess().Id;

public StartEditorServicesCommand()
{
// Sets the distribution channel to "PSES" so starts can be distinguished in PS7+ telemetry
Expand All @@ -44,30 +48,30 @@ public StartEditorServicesCommand()
/// <summary>
/// The name of the EditorServices host to report.
/// </summary>
[Parameter(Mandatory = true)]
[Parameter]
[ValidateNotNullOrEmpty]
public string HostName { get; set; }
public string HostName { get; set; } = "PSES";

/// <summary>
/// The ID to give to the host's profile.
/// </summary>
[Parameter(Mandatory = true)]
[Parameter]
[ValidateNotNullOrEmpty]
public string HostProfileId { get; set; }
public string HostProfileId { get; set; } = "PSES";

/// <summary>
/// The version to report for the host.
/// </summary>
[Parameter(Mandatory = true)]
[Parameter]
[ValidateNotNullOrEmpty]
public Version HostVersion { get; set; }
public Version HostVersion { get; set; } = new Version(0, 0, 0);

/// <summary>
/// Path to the session file to create on startup or startup failure.
/// </summary>
[Parameter(Mandatory = true)]
[Parameter]
[ValidateNotNullOrEmpty]
public string SessionDetailsPath { get; set; }
public string SessionDetailsPath { get; set; } = "PowerShellEditorServices.json";

/// <summary>
/// The name of the named pipe to use for the LSP transport.
Expand Down Expand Up @@ -120,14 +124,16 @@ public StartEditorServicesCommand()
/// </summary>
[Parameter]
[ValidateNotNullOrEmpty]
public string BundledModulesPath { get; set; }
public string BundledModulesPath { get; set; } = Path.GetFullPath(Path.Combine(
Path.GetDirectoryName(typeof(StartEditorServicesCommand).Assembly.Location),
"..", "..", ".."));

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

/// <summary>
/// The minimum log level that should be emitted.
Expand Down Expand Up @@ -266,34 +272,32 @@ private void StartLogging()
}

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

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

StreamLogger fileLogger = StreamLogger.CreateWithNewFile(logPath);
_disposableResources.Add(fileLogger);
IDisposable fileLoggerUnsubscriber = _logger.Subscribe(fileLogger);
fileLogger.AddUnsubscriber(fileLoggerUnsubscriber);
_loggerUnsubscribers.Add(fileLoggerUnsubscriber);

_logger.Log(PsesLogLevel.Diagnostic, "Logging started");
}

// Sanitizes user input and ensures the directory is created.
private string GetLogDirPath()
{
string logDir = !string.IsNullOrEmpty(LogPath)
? Path.GetDirectoryName(LogPath)
: Path.GetDirectoryName(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location));
string logDir = LogPath;
if (string.IsNullOrEmpty(logDir))
{
logDir = Path.Combine(Path.GetTempPath(), "PowerShellEditorServices");
}

// Ensure logDir exists
Directory.CreateDirectory(logDir);

return logDir;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,15 @@ internal sealed class EditorServicesServerFactory : IDisposable
/// constructor? In the end it returns an instance (albeit a "singleton").
/// </para>
/// </remarks>
/// <param name="logPath">The path of the log file to use.</param>
/// <param name="logDirectoryPath">The path of the log file to use.</param>
/// <param name="minimumLogLevel">The minimum log level to use.</param>
/// <param name="hostLogger">The host logger?</param>
public static EditorServicesServerFactory Create(string logPath, int minimumLogLevel, IObservable<(int logLevel, string message)> hostLogger)
public static EditorServicesServerFactory Create(string logDirectoryPath, int minimumLogLevel, IObservable<(int logLevel, string message)> hostLogger)
{
// NOTE: Ignore the suggestion to use Environment.ProcessId as it doesn't work for
// .NET 4.6.2 (for Windows PowerShell), and this won't be caught in CI.
int currentPID = Process.GetCurrentProcess().Id;
string logPath = Path.Combine(logDirectoryPath, $"PowerShellEditorServices-{currentPID}.log");
Log.Logger = new LoggerConfiguration()
.Enrich.FromLogContext()
.WriteTo.Async(config => config.File(logPath))
Expand Down
63 changes: 63 additions & 0 deletions test/emacs-simple-test.el
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
;;; emacs-test.el --- Integration testing script -*- lexical-binding: t; -*-

;; Copyright (c) Microsoft Corporation.
;; Licensed under the MIT License.

;; Author: Andy Jordan <andy.jordan@microsoft.com>
;; Keywords: PowerShell, LSP

;;; Code:

;; Avoid using old packages.
(setq load-prefer-newer t)

;; Improved TLS Security.
(with-eval-after-load 'gnutls
(custom-set-variables
'(gnutls-verify-error t)
'(gnutls-min-prime-bits 3072)))

;; Package setup.
(require 'package)
(add-to-list 'package-archives
'("melpa" . "https://melpa.org/packages/") t)
(package-initialize)
(package-refresh-contents)

(require 'ert)

(require 'flymake)

(unless (package-installed-p 'powershell)
(package-install 'powershell))
(require 'powershell)

(unless (package-installed-p 'eglot)
(package-install 'eglot))
(require 'eglot)

(ert-deftest powershell-editor-services ()
"Eglot should connect to PowerShell Editor Services."
(let* ((repo (project-root (project-current)))
(start-script (expand-file-name "module/PowerShellEditorServices/Start-EditorServices.ps1" repo))
(test-script (expand-file-name "test/PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1" repo))
(eglot-sync-connect t))
(add-to-list
'eglot-server-programs
`(powershell-mode
. ("pwsh" "-NoLogo" "-NoProfile" "-Command" ,start-script "-Stdio")))
(with-current-buffer (find-file-noselect test-script)
(should (eq major-mode 'powershell-mode))
(should (apply #'eglot--connect (eglot--guess-contact)))
(should (eglot-current-server))
(let ((lsp (eglot-current-server)))
(should (string= (eglot--project-nickname lsp) "PowerShellEditorServices"))
(should (member (cons 'powershell-mode "powershell") (eglot--languages lsp))))
(sleep-for 5) ; TODO: Wait for "textDocument/publishDiagnostics" instead
(flymake-start)
(goto-char (point-min))
(flymake-goto-next-error)
(should (eq 'flymake-warning (face-at-point))))))

(provide 'emacs-test)
;;; emacs-test.el ends here
40 changes: 40 additions & 0 deletions test/vim-simple-test.vim
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
let s:suite = themis#suite('pses')
let s:assert = themis#helper('assert')

function s:suite.before()
let l:pses_path = g:repo_root . '/module'
let g:LanguageClient_serverCommands = {
\ 'ps1': [ 'pwsh', '-NoLogo', '-NoProfile', '-Command',
\ l:pses_path . '/PowerShellEditorServices/Start-EditorServices.ps1', '-Stdio' ]
\ }
let g:LanguageClient_serverStderr = 'DEBUG'
let g:LanguageClient_loggingFile = g:repo_root . '/LanguageClient.log'
let g:LanguageClient_serverStderr = g:repo_root . '/LanguageServer.log'
endfunction

function s:suite.has_language_client()
call s:assert.includes(&runtimepath, g:repo_root . '/LanguageClient-neovim')
call s:assert.cmd_exists('LanguageClientStart')
call s:assert.not_empty(g:LanguageClient_serverCommands)
call s:assert.true(LanguageClient#HasCommand('ps1'))
endfunction

function s:suite.analyzes_powershell_file()
view test/vim-test.ps1 " This must not use quotes!

let l:bufnr = bufnr('vim-test.ps1$')
call s:assert.not_equal(l:bufnr, -1)
let l:bufinfo = getbufinfo(l:bufnr)[0]

call s:assert.equal(l:bufinfo.name, g:repo_root . '/test/vim-test.ps1')
call s:assert.includes(getbufline(l:bufinfo.name, 1), 'function Do-Work {}')
" TODO: This shouldn't be necessary, vim-ps1 works locally but not in CI.
call setbufvar(l:bufinfo.bufnr, '&filetype', 'ps1')
call s:assert.equal(getbufvar(l:bufinfo.bufnr, '&filetype'), 'ps1')

execute 'LanguageClientStart'
execute 'sleep' 5
call s:assert.equal(getbufvar(l:bufinfo.name, 'LanguageClient_isServerRunning'), 1)
call s:assert.equal(getbufvar(l:bufinfo.name, 'LanguageClient_projectRoot'), g:repo_root)
call s:assert.equal(getbufvar(l:bufinfo.name, 'LanguageClient_statusLineDiagnosticsCounts'), {'E': 0, 'W': 1, 'H': 0, 'I': 0})
endfunction
2 changes: 1 addition & 1 deletion test/vim-test.vim
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ let s:assert = themis#helper('assert')
function s:suite.before()
let l:pses_path = g:repo_root . '/module'
let g:LanguageClient_serverCommands = {
\ 'ps1': ['pwsh', '-NoLogo', '-NoProfile', '-Command',
\ 'ps1': [ 'pwsh', '-NoLogo', '-NoProfile', '-Command',
\ l:pses_path . '/PowerShellEditorServices/Start-EditorServices.ps1',
\ '-HostName', 'vim', '-HostProfileId', 'vim', '-HostVersion', '1.0.0',
\ '-BundledModulesPath', l:pses_path, '-Stdio',
Expand Down

0 comments on commit fe7fe5a

Please sign in to comment.