Skip to content

Commit d28db8e

Browse files
committed
Address comments
1 parent d926544 commit d28db8e

12 files changed

Lines changed: 61 additions & 79 deletions

File tree

.github/workflows/post-beta-long-running.yml

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name: Post-Beta Long Running Tests
1+
name: Post-Release Long Running Tests
22
run-name: Long Running Tests for ${{ github.event.release.tag_name }}
33

44
on:
@@ -9,30 +9,38 @@ permissions:
99
contents: read
1010

1111
jobs:
12-
check-beta-release:
12+
check-release:
1313
runs-on: ubuntu-latest
1414
outputs:
15-
is-beta: ${{ steps.check.outputs.is-beta }}
15+
should-run: ${{ steps.check.outputs.should-run }}
16+
duration: ${{ steps.check.outputs.duration }}
17+
timeout: ${{ steps.check.outputs.timeout }}
1618
steps:
17-
- name: Check if beta release
19+
- name: Check release type
1820
id: check
1921
run: |
2022
TAG="${{ github.event.release.tag_name }}"
2123
if [[ "$TAG" =~ -beta$ ]]; then
22-
echo "is-beta=true" >> $GITHUB_OUTPUT
24+
echo "should-run=true" >> $GITHUB_OUTPUT
25+
echo "duration=4h" >> $GITHUB_OUTPUT
26+
echo "timeout=360" >> $GITHUB_OUTPUT
27+
elif [[ "$TAG" =~ -alpha$ ]]; then
28+
echo "should-run=true" >> $GITHUB_OUTPUT
29+
echo "duration=20m" >> $GITHUB_OUTPUT
30+
echo "timeout=60" >> $GITHUB_OUTPUT
2331
else
24-
echo "is-beta=false" >> $GITHUB_OUTPUT
32+
echo "should-run=false" >> $GITHUB_OUTPUT
2533
fi
2634
2735
long-running-tests:
28-
needs: check-beta-release
29-
if: needs.check-beta-release.outputs.is-beta == 'true'
36+
needs: check-release
37+
if: needs.check-release.outputs.should-run == 'true'
3038
strategy:
3139
matrix:
3240
os: [ubuntu-latest, windows-latest, macos-latest]
3341
node-version: [18, 20, 22]
3442
runs-on: ${{ matrix.os }}
35-
timeout-minutes: 360
43+
timeout-minutes: ${{ fromJSON(needs.check-release.outputs.timeout) }}
3644
steps:
3745
- uses: actions/checkout@v5
3846
with:
@@ -44,6 +52,7 @@ jobs:
4452
node-version: ${{ matrix.node-version }}
4553

4654
- name: Download release standalone
55+
shell: bash
4756
env:
4857
GH_TOKEN: ${{ github.token }}
4958
run: |
@@ -57,11 +66,10 @@ jobs:
5766
gh release download "$TAG" --pattern "$PATTERN"
5867
5968
- name: Extract standalone bundle
69+
shell: bash
6070
run: |
61-
if [[ "${{ matrix.os }}" == "windows-latest" ]]; then
62-
unzip *.zip
63-
else
64-
unzip *.zip
71+
unzip *.zip
72+
if [[ "${{ matrix.os }}" != "windows-latest" ]]; then
6573
chmod +x cfn-lsp-server-standalone.js
6674
fi
6775
@@ -73,15 +81,15 @@ jobs:
7381

7482
- name: Run long-running stability tests
7583
env:
76-
STABILITY_TEST_DURATION: 4h
84+
STABILITY_TEST_DURATION: ${{ needs.check-release.outputs.duration }}
7785
STANDALONE_PATH: ./cfn-lsp-server-standalone.js
7886
run: npm run test:stability
7987

8088
- name: Upload test results
8189
if: always()
8290
uses: actions/upload-artifact@v4
8391
with:
84-
name: long-running-test-results-${{ matrix.os }}
92+
name: long-running-test-results-${{ matrix.os }}-node${{ matrix.node-version }}
8593
path: |
8694
test-results.json
8795
test-logs.txt

.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
*.node
1010
.DS_Store
1111
tools/*
12-
!tools/*/
1312
!tools/**/*.ts
1413
**/.aws-cfn-storage
1514
/oss-attribution

tools/lspClient/LspClient.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@ export class LspClient implements LspConnection {
2929
protected isShutdown = false;
3030
protected workspaceConfig: Record<string, unknown>[] = [{}];
3131

32-
constructor(protected config: LspClientConfig) {
32+
private readonly suppressLevels: string[];
33+
34+
constructor(private readonly config: LspClientConfig) {
3335
this.createdAt = performance.now();
3436
this.encryptionKey = randomBytes(32);
37+
this.suppressLevels = this.config.suppressLogLevels ?? ['INFO', 'DEBUG'];
3538
}
3639

3740
async initialize(): Promise<void> {
@@ -100,9 +103,7 @@ export class LspClient implements LspConnection {
100103
private readonly onServerOutput = (data: Buffer) => {
101104
const output = data.toString().trim();
102105

103-
// Log filtering - keep for debugging
104-
const suppressLevels = this.config.suppressLogLevels ?? ['INFO', 'DEBUG'];
105-
const shouldSuppress = suppressLevels.some((level) => output.includes(`${level}:`));
106+
const shouldSuppress = this.suppressLevels.some((level) => output.includes(`${level}:`));
106107

107108
if (!shouldSuppress) {
108109
console.error(`[LSP Server]: ${output}`);

tools/stability/Config.ts

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,6 @@ function parseSimpleArgs(): Partial<Config> {
3030
return args;
3131
}
3232

33-
export function parseConfig(): Config {
34-
// Start with environment variables (npm script support)
35-
const envConfig = {
36-
duration: process.env.STABILITY_TEST_DURATION ?? '4h',
37-
maxRetries: Number.parseInt(process.env.MAX_RETRIES ?? '3'),
38-
responseTimeout: Number.parseInt(process.env.RESPONSE_TIMEOUT ?? '5000'),
39-
path: process.env.STANDALONE_PATH ?? './cfn-lsp-server-standalone.js',
40-
};
41-
42-
// Override with command line arguments if provided
43-
const cliArgs = parseSimpleArgs();
44-
45-
return {
46-
...envConfig,
47-
...cliArgs,
48-
};
49-
}
50-
5133
export function parseDuration(duration: string): number {
5234
const match = duration.match(/^(\d+)([hms])$/);
5335
if (!match) throw new Error(`Invalid duration format: ${duration}`);
@@ -70,3 +52,11 @@ export function parseDuration(duration: string): number {
7052
}
7153
}
7254
}
55+
56+
export const config: Config = {
57+
duration: process.env.STABILITY_TEST_DURATION ?? '4h',
58+
maxRetries: Number.parseInt(process.env.MAX_RETRIES ?? '3'),
59+
responseTimeout: Number.parseInt(process.env.RESPONSE_TIMEOUT ?? '5000'),
60+
path: process.env.STANDALONE_PATH ?? './cfn-lsp-server-standalone.js',
61+
...parseSimpleArgs(),
62+
};

tools/stability/Monitoring.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { OperationType, getTesterConfig } from './testers/TesterTypes';
1+
import { OperationType, TESTER_CONFIG } from './testers/TesterTypes';
22

33
export type TestMetrics = {
44
operations: number;
@@ -83,7 +83,7 @@ export function generateFinalReport(testStartTime: number): void {
8383

8484
export function checkPerformanceDegradation(): void {
8585
for (const [operationType, metric] of Object.entries(metrics) as [OperationType, TestMetrics][]) {
86-
const config = getTesterConfig(operationType);
86+
const config = TESTER_CONFIG[operationType];
8787

8888
if (metric.averageDuration !== null && metric.averageDuration > config.avgDurationLimitMs) {
8989
throw new Error(

tools/stability/Templates.ts

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,8 @@
1-
import { Templates } from '../../tst/utils/TemplateUtils';
1+
import { TemplateConfig, Templates } from '../../tst/utils/TemplateUtils';
22

3-
export interface TemplateConfig {
4-
name: string;
5-
content: string;
6-
}
7-
8-
export const TEMPLATE_CONFIGS: TemplateConfig[] = [
9-
{
10-
name: 'sample.yaml',
11-
content: Templates.sample.yaml.contents,
12-
},
13-
{
14-
name: 'simple.yaml',
15-
content: Templates.simple.yaml.contents,
16-
},
17-
{
18-
name: 'comprehensive.yaml',
19-
content: Templates.comprehensive.yaml.contents,
20-
},
21-
{
22-
name: 'condition-usage.yaml',
23-
content: Templates.conditionUsage.yaml.contents,
24-
},
3+
export const TEST_TEMPLATES: TemplateConfig[] = [
4+
Templates.sample.yaml,
5+
Templates.simple.yaml,
6+
Templates.comprehensive.yaml,
7+
Templates.conditionUsage.yaml,
258
];

tools/stability/TestOrchestrator.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
11
import { LspClient } from '../lspClient/LspClient';
2-
import { parseConfig, parseDuration } from './Config';
2+
import { config, parseDuration } from './Config';
33
import { initializeMonitoring, logProgress, checkPerformanceDegradation } from './Monitoring';
44
import { HoverTester } from './testers/HoverTester';
55
import { CompletionTester } from './testers/CompletionTester';
6-
import { TEMPLATE_CONFIGS } from './Templates';
6+
import { TEST_TEMPLATES } from './Templates';
77
import { AwsRegion } from '../../src/utils/Region';
88
import { WaitFor } from '../../tst/utils/Utils';
99
import { existsSync } from 'fs';
1010

1111
export class TestOrchestrator {
1212
private client!: LspClient;
13-
private readonly config = parseConfig();
13+
private readonly config = config;
1414
private startTime!: number;
1515
private endTime!: number;
1616
private hoverTester!: HoverTester;
1717
private completionTester!: CompletionTester;
1818

19-
private readonly templates = TEMPLATE_CONFIGS;
19+
private readonly templates = TEST_TEMPLATES;
2020

2121
private readonly testRegions = Object.values(AwsRegion).filter(
2222
(region) => region !== AwsRegion.ME_SOUTH_1 && region !== AwsRegion.ME_CENTRAL_1,
@@ -138,15 +138,15 @@ export class TestOrchestrator {
138138

139139
// Test all templates for this region
140140
for (const template of this.templates) {
141-
const uri = `file:///test/${template.name}`;
141+
const uri = `file:///test/${template.fileName}`;
142142

143143
try {
144-
await this.client.openDocument(uri, template.content);
144+
await this.client.openDocument(uri, template.contents);
145145

146146
await this.validateLsp(uri);
147147

148148
// Revert document to original state after tests
149-
await this.client.updateDocument(uri, 6, template.content);
149+
await this.client.updateDocument(uri, 6, template.contents);
150150
} finally {
151151
try {
152152
await this.client.closeDocument(uri);

tools/stability/testers/CompletionTester.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { LspClient } from '../../lspClient/LspClient';
22
import { OperationTester, OperationType } from './TesterTypes';
3-
import { retryOperationWithPerformance } from './TesterCommon';
3+
import { retryOperationWithPerformance } from './TesterUtils';
44

55
export class CompletionTester implements OperationTester {
66
constructor(private readonly client: LspClient) {}

tools/stability/testers/HoverTester.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { LspClient } from '../../lspClient/LspClient';
22
import { OperationTester, OperationType } from './TesterTypes';
3-
import { retryOperationWithPerformance } from './TesterCommon';
3+
import { retryOperationWithPerformance } from './TesterUtils';
44

55
export class HoverTester implements OperationTester {
66
constructor(private readonly client: LspClient) {}

tools/stability/testers/TesterTypes.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,3 @@ export const TESTER_CONFIG: Record<OperationType, TesterConfig> = {
2525
maxDurationLimitMs: 5000,
2626
},
2727
};
28-
29-
export function getTesterConfig(operationType: OperationType): TesterConfig {
30-
return TESTER_CONFIG[operationType];
31-
}

0 commit comments

Comments
 (0)