Add LSP Client and long running tests in tools by chrisqm-dev · Pull Request #511 · aws-cloudformation/cloudformation-languageserver

chrisqm-dev · 2026-03-31T21:53:38Z

Description of changes:

Add lsp client methods to start up and interact with the cfn language server
Can be used for telemetry generator as well
Add long-running tests
- This tests the standalone.js and ensures it is usable for extended periods of time
- Tests the following:
  - Completion on document updates
  - Hover on document updates
  - Region switching, and therefore public schema retrieval
  - Ensures performance doesn't degrade over time

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

chrisqm-dev · 2026-03-31T21:56:24Z

+        const output = data.toString().trim();
+
+        // Readiness detection
+        if (output.includes('cfn-lint version')) {


Currently, the only way to determine if lint, guard, and the schemas are ready is through the logs. #507 Introduces a new LSP request which will return the guard, lint, and schema statuses. This addition will make testing more robust and speed up test time, but this PR is functional without this change

satyakigh · 2026-04-01T16:41:29Z

Our folder and file names are usually camelCased unless its a bin/cli command file

satyakigh · 2026-04-01T16:51:56Z

+export type StandaloneTestMetrics = {
+    operationsAttempted: number;
+    operationsFailed: number;
+    averageDuration: number | null;


Can't these just be an array of numbers and the user decides what to do with them

Because this test could run for a very long time and there are thousands of operations per minute, the duration array could get very large. We should keep the min, max, and average attributes to avoid having to calculate from the array. Added the durations array in case there's any other operations we use in the future, such as median or p99.

satyakigh · 2026-04-01T16:52:34Z

+    }
+}
+
+export function generateFinalReport(testStartTime: number): void {


What will this report be used for?

This prints out to the github actions logs upon completing successfully. I wanted to replicate a test coverage output report similar to what you see in vitest. We can't use vitest since it will not work with node 18

satyakigh · 2026-04-01T16:53:45Z

+    }
+}
+
+void main();


This will swallow the exception and succeed the run

Added logic so that the exception is not swallowed in the event an exception (such as hover validation failure) occurs, and it fails fast. In the catch, we will explicitly throw the error

satyakigh · 2026-04-01T16:55:44Z

Lots of references to "long-running" the caller of the main file defines the duration

chrisqm-dev · 2026-04-02T15:25:02Z

Our folder and file names are usually camelCased unless its a bin/cli command file

Fixed folder and file names so that folders are camelCased and files are PascalCased, which matches the styling in the rest of the package

chrisqm-dev · 2026-04-02T16:40:07Z

Lots of references to "long-running" the caller of the main file defines the duration

Changed to "Stability" tests instead of calling it LongRunning

rongyamazon · 2026-04-28T18:34:18Z

+    async testAllScenarios(uri: string): Promise<void> {
+        // Test 1: Top-level completion using full document update
+        const basicTemplate = `AWSTemplateFormatVersion: '2010-09-09'
+`;


nit: remove extra line

the '`' is placed on a new line so that the completion provider returns all the top level resources, since the completion provider will only return these values on a new line

satyakigh · 2026-04-30T17:11:15Z

 *.node
 .DS_Store
 tools/*
-!tools/*.ts


Just need !tools/**/*.ts

Removed !tools/**/*.ts

satyakigh · 2026-04-30T17:14:13Z

+        const output = data.toString().trim();
+
+        // Log filtering - keep for debugging
+        const suppressLevels = this.config.suppressLogLevels ?? ['INFO', 'DEBUG'];


This line can be moved out of the function, this only needs to execute once

Moved suppressLevels outside of the function

satyakigh · 2026-04-30T17:15:18Z

+    protected isShutdown = false;
+    protected workspaceConfig: Record<string, unknown>[] = [{}];
+
+    constructor(protected config: LspClientConfig) {


Changed config to private

satyakigh · 2026-04-30T17:16:37Z

+    path: string;
+}
+
+function parseSimpleArgs(): Partial<Config> {


Use yargs similar to the other tools

yargs cannot be used in node 18:

Error: yargs parser supports a minimum Node.js version of 20. Read our version support policy: https://github.com/yargs/yargs-parser#supported-nodejs-versions

satyakigh · 2026-04-30T17:17:23Z

@@ -0,0 +1,72 @@
+export interface Config {


This file should just return a Config object, instead of exporting the parsers

Fixed so that Config.ts exports a config object instead of parseConfig()

satyakigh · 2026-04-30T17:19:07Z

+    content: string;
+}
+
+export const TEMPLATE_CONFIGS: TemplateConfig[] = [


Templates? Test Templates?
There is no config here

Renamed const to TEST_TEMPLATES

satyakigh · 2026-04-30T17:21:30Z

+    },
+};
+
+export function getTesterConfig(operationType: OperationType): TesterConfig {


If getTesterConfig is required, we don't need to export TESTER_CONFIG, but easier would just be to export TESTER_CONFIG and use it directly TESTER_CONFIG[operationType]

calling getTesterConfig or TESTER_CONFIG[operationType] once is trivial

Removed getter function to instead use the const directly

satyakigh · 2026-04-30T17:21:55Z

@@ -0,0 +1,27 @@
+import { recordOperation } from '../Monitoring';


Renamed file to "TesterUtils.ts"

satyakigh · 2026-04-30T17:23:04Z

+        run: node cfn-lsp-server-standalone.js --version
+
+      - name: Install dependencies
+        run: npm ci


This probably won't work for node 18

Confirmed this works for node 18 by running npm ci locally with node 18

satyakigh · 2026-04-30T17:23:42Z

+  contents: read
+
+jobs:
+  check-beta-release:


this handles the main branch as well? Alpha?

Added a workflow for alpha to work. We need a release for this test to work as it requires a pre-built standalone file, which is not created on every push to main.

kddejong

A few comments below, requesting changes for the report timing bug.

kddejong · 2026-05-01T17:46:23Z

+    try {
+        await orchestrator.initialize();
+        await orchestrator.runTests();
+        generateFinalReport(Date.now());


generateFinalReport(Date.now()) is called after the tests finish, so the reported runtime will always be ~0 minutes. Capture the start time before runTests() and pass that in. (Or just use the startTime that initializeMonitoring() already tracks internally.)

Fixed to use startTime value appropriately

kddejong · 2026-05-01T17:46:23Z

+export interface LspConnection {
+    initialize(): Promise<void>;
+    sendRequest(method: string, params: any): Promise<any>;
+    sendNotification(method: string, params: any): Promise<void>;


There's a lot of any throughout the client and testers — is there a reason we're not using the types from vscode-languageserver-protocol? Things like Hover, CompletionList, CompletionParams, etc. would give you compile-time safety on the validators for free.

Added typing for the specific lsp requests.

For sendRequest, sendNotification, and onRequest,any will still be necessary since it can be used for any request object.

kddejong · 2026-05-01T17:46:23Z

+        metric.averageDuration === null
+            ? duration
+            : (metric.averageDuration * (metric.operations - 1) + duration) / metric.operations;
+    metric.minDuration = metric.minDuration === null ? duration : Math.min(metric.minDuration, duration);


durations is pushed to on every operation but never read anywhere. What's it for?

This was originally going to be used to generate metrics other than average (for example, median duration which requires the durations list). Those metrics were not implemented since average was sufficient. Removed the durations list, we can add it back if we later determine more metrics are needed

kddejong · 2026-05-01T17:46:23Z

+
+        await this.client.updateDocument(uri, 2, s3Template);
+
+        await retryOperationWithPerformance(


Nit: the document versions (2, 3, 4, 5, 6) are hardcoded across the testers and orchestrator. Works fine now but could get fragile if more testers are added. A shared counter would help down the road.

Added nextDocumentVersion() and resetDocumentVersion() helpers

kddejong

Ship it 🚀

chrisqm-dev requested a review from a team as a code owner March 31, 2026 21:53

chrisqm-dev commented Mar 31, 2026

View reviewed changes

satyakigh reviewed Apr 1, 2026

View reviewed changes

chrisqm-dev force-pushed the long-running branch from eed185f to 2ddb466 Compare April 2, 2026 18:31

chrisqm-dev marked this pull request as draft April 7, 2026 20:54

chrisqm-dev force-pushed the long-running branch from 562aac8 to 30b5b8e Compare April 22, 2026 20:31

chrisqm-dev marked this pull request as ready for review April 22, 2026 20:36

chrisqm-dev force-pushed the long-running branch 2 times, most recently from bdd25b8 to 9375221 Compare April 28, 2026 18:09

rongyamazon previously approved these changes Apr 28, 2026

View reviewed changes

chrisqm-dev dismissed rongyamazon’s stale review via 183a4c0 April 28, 2026 18:59

satyakigh reviewed Apr 30, 2026

View reviewed changes

chrisqm-dev force-pushed the long-running branch 2 times, most recently from 3b40c92 to f3210fd Compare May 1, 2026 16:33

kddejong requested changes May 1, 2026

View reviewed changes

chrisqm-dev force-pushed the long-running branch from ac40307 to 86d8f6b Compare May 1, 2026 19:09

kddejong approved these changes May 1, 2026

View reviewed changes

chrisqm-dev added 9 commits May 1, 2026 17:23

Add LSP Client and long running tests in tools

accfd30

Address comments

022c5ed

Modify LspConnection inteface

1ee98aa

Use system status handler

c8bd516

Linting

d926544

Address comments

d28db8e

Rename long running workflow

dd6862f

Address typings and duration

3ebad83

Add typings for changeConfiguration

4d1acc5

chrisqm-dev force-pushed the long-running branch from 86d8f6b to 4d1acc5 Compare May 1, 2026 21:23

		@@ -0,0 +1,27 @@
		import { recordOperation } from '../Monitoring';


		await this.client.updateDocument(uri, 2, s3Template);

		await retryOperationWithPerformance(

Conversation

chrisqm-dev commented Mar 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

satyakigh commented Apr 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

satyakigh commented Apr 1, 2026

Uh oh!

chrisqm-dev commented Apr 2, 2026

Uh oh!

chrisqm-dev commented Apr 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisqm-dev Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kddejong left a comment

Choose a reason for hiding this comment

chrisqm-dev Apr 30, 2026 •

edited

Loading