Skip to content

Commit

Permalink
Upgrade language server version (#89)
Browse files Browse the repository at this point in the history
* Upgrade language server version

To go along with the changes in the language server 0.4.0:
https://github.com/smithy-lang/smithy-language-server/blob/main/CHANGELOG.md#040-2024-07-30,
this commit also:
- updates coursier to use java 21 to run the server
- adds config options diagnostics.minimumSeverity, onlyReloadOnSave
- removes config option for logToFile

* Fix tests relying on .smithy.lsp.log

Most of the test suites read the old .smithy.lsp.log file that the
language server could be configured to write out. Since that was
removed, we need a different way to do test assertions. I tried to
see if we could look at the trace of messages between server <->
vscode client, but wasn't able to find a way. So I simplified the
assertions for these test suites, which should be ok because we
probably don't want to rely on logs anyways for testing, and more
robust testing is done on the server side (our extension doesn't
have that much functionality to test).

I also had to add `sources` to each of the smithy-build.json files
used for tests, so the language server could properly load those
test projects.

* Make couriser download jdk 21

By default, couriser will use AdoptOpenJDK 8 to run the server, so we
needed to tell it to use jdk 21. Coursier will download the JDK if
necessary. It uses an index to figure out where to download it from:
https://get-coursier.io/docs/2.0.6/cli-java#jvm-index. The version of
coursier we are using in the extension has an old version of the index,
so we needed to provide an updated index for coursier to pull from.
Also, we needed to tell coursier specifically which jdk to use (not just
21), because by default it tries to use AdoptOpenJDK, when there's no
AdoptOpenJDK 21 (it is under the name Adoptium) now.

This is a temporary solution, meant to avoid downloading a second version
of coursier which had an updated index and could properly find the right
jdk to download. In the future, we will ship the language server as a
standalone executable, and can remove coursier altogether.

I also updated the tests to stop using timeouts, so we don't have to worry
about an inconsistent download time in CI.
  • Loading branch information
milesziemer authored Aug 1, 2024
1 parent cece632 commit b381d23
Show file tree
Hide file tree
Showing 14 changed files with 96 additions and 66 deletions.
20 changes: 15 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 20 additions & 16 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@
"Programming Languages",
"Snippets"
],
"activationEvents": [
"onLanguage:smithy",
"onCommand:smithy.runSelector"
],
"main": "./out/src/extension",
"preview": true,
"contributes": {
Expand Down Expand Up @@ -106,25 +102,33 @@
"default": "verbose",
"description": "Traces the communication between VS Code and the language server."
},
"smithyLsp.logToFile": {
"scope": "window",
"type": "string",
"enum": [
"enabled",
"disabled"
],
"default": "disabled",
"description": "Writes additional logging to .smithy.lsp.log file in workspace."
},
"smithyLsp.version": {
"scope": "window",
"type": "string",
"default": "0.2.4",
"description": "Version of the Smithy LSP (see https://github.com/smithy-lang/smithy-language-server)"
"default": "0.4.0",
"description": "Version of the Smithy Language Server (see https://github.com/smithy-lang/smithy-language-server)."
},
"smithyLsp.rootPath": {
"scope": "resource",
"type": "string"
},
"smithyLsp.diagnostics.minimumSeverity": {
"scope": "window",
"type": "string",
"enum": [
"NOTE",
"WARNING",
"DANGER",
"ERROR"
],
"default": "WARNING",
"description": "Minimum severity of Smithy validation events to display in the editor."
},
"smithyLsp.onlyReloadOnSave": {
"scope": "window",
"type": "boolean",
"default": false,
"description": "Whether to only re-load the Smithy model on save. Use this if the server feels slow as you type."
}
}
}
Expand Down
21 changes: 20 additions & 1 deletion src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ import {
} from "vscode-languageclient/node";
import { getCoursierExecutable } from "./coursier/coursier";

// Couriser uses an index to determine where to download jvms from: https://get-coursier.io/docs/2.0.6/cli-java#jvm-index
// Newer versions of coursier use this index, which is more up to date than the one
// used by the coursier version used by the extension.
// This is a temporary solution to avoid adding logic that determines the version of
// coursier on the local machine. In the near future, we will vend the language server
// as a standalone executable, and will no longer need couriser to manage the jvm version.
const COURSIER_JVM_INDEX = "https://raw.githubusercontent.com/coursier/jvm-index/master/index.json";

let client: LanguageClient;

export function activate(context: vscode.ExtensionContext) {
Expand Down Expand Up @@ -68,6 +76,15 @@ export function activate(context: vscode.ExtensionContext) {
let launchargs = [
"launch",
"software.amazon.smithy:smithy-language-server:" + version,
// Configure couriser to use java 21
"--jvm",
// By default, coursier uses AdoptOpenJDK: https://get-coursier.io/docs/2.0.6/cli-java
// We could just say '21' here, and let coursier default to adopt jdk
// 21, but later versions of the jdk are released under the name adoptium.
"corretto:21",
// The location to download the jvm from is provided by the jvm index.
"--jvm-index",
COURSIER_JVM_INDEX,
"-r",
"m2local",
"-M",
Expand Down Expand Up @@ -179,11 +196,13 @@ function getClientOptions(): LanguageClientOptions {
workspaceFolder,

initializationOptions: {
logToFile: vscode.workspace.getConfiguration("smithyLsp").get("logToFile", "disabled"),
"diagnostics.minimumSeverity": vscode.workspace.getConfiguration("smithyLsp").get("diagnostics.minimumSeverity"),
onlyReloadOnSave: vscode.workspace.getConfiguration("smithyLsp").get("onlyReloadOnSave"),
},

// Don't switch to output window when the server returns output.
revealOutputChannelOn: RevealOutputChannelOn.Never,
progressOnInitialization: true,
};
}

Expand Down
1 change: 1 addition & 0 deletions test-fixtures/suite1/smithy-build.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"version": "1.0",
"sources": ["./main.smithy"],
"maven": {
"dependencies": ["software.amazon.smithy:smithy-aws-traits:1.40.0"],
"repositories": [{ "url": "https://repo1.maven.org/maven2/" }]
Expand Down
1 change: 1 addition & 0 deletions test-fixtures/suite2/smithy-build.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"version": "1.0",
"sources": ["./main.smithy"],
"maven": {
"dependencies": ["software.amazon.smithy:smithy-aws-traits:1.40.0"],
"repositories": [{ "url": "https://repo1.maven.org/maven2/" }]
Expand Down
1 change: 1 addition & 0 deletions test-fixtures/suite3/smithy-build.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"version": "1.0",
"sources": ["./main.smithy"],
"maven": {
"dependencies": ["software.amazon.smithy:smithy-aws-traits:1.40.0"],
"repositories": [{ "url": "https://repo1.maven.org/maven2/" }]
Expand Down
1 change: 1 addition & 0 deletions test-fixtures/suite4/smithy/smithy-build.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"version": "1.0",
"sources": ["./main.smithy"],
"maven": {
"dependencies": ["software.amazon.smithy:smithy-aws-traits:1.40.0", "software.amazon.smithy:smithy-waiters:1.40.0"],
"repositories": [{ "url": "https://repo1.maven.org/maven2/" }]
Expand Down
1 change: 1 addition & 0 deletions test-fixtures/suite5/smithy/smithy-build.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"version": "1.0",
"sources": ["./main.smithy"],
"maven": {
"dependencies": ["software.amazon.smithy:smithy-aws-traits:1.40.0", "software.amazon.smithy:smithy-waiters:1.40.0"],
"repositories": [{ "url": "https://repo1.maven.org/maven2/" }]
Expand Down
8 changes: 1 addition & 7 deletions tests/helper.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TextDocument, TextEditor, Uri, workspace } from "vscode";
import { TextDocument, TextEditor, Uri } from "vscode";
import { resolve } from "path";
import { glob } from "glob";
import * as Mocha from "mocha";
Expand All @@ -19,12 +19,6 @@ export async function waitForServerStartup() {
await new Promise((resolve) => setTimeout(resolve, 9000));
}

export async function getLangServerLogs(p: string): Promise<string> {
const smithyLogUri = getDocUri(p + "/.smithy.lsp.log");
const smithyLog = await workspace.openTextDocument(smithyLogUri);
return smithyLog.getText();
}

export function runTests(testsRoot: string, cb: (error: any, failures?: number) => void): void {
const mocha = new Mocha({
ui: "tdd",
Expand Down
17 changes: 7 additions & 10 deletions tests/suite1/extension.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import * as assert from "assert";
import * as vscode from "vscode";
import { getDocUri, getLangServerLogs, waitForServerStartup } from "./../helper";
import { getDocUri, waitForServerStartup } from "./../helper";

suite("Extension tests", function () {
this.timeout(0);

suite("Extension tests", () => {
test("Should start extension and Language Server", async () => {
const smithyMainUri = getDocUri("suite1/main.smithy");
const doc = await vscode.workspace.openTextDocument(smithyMainUri);
Expand All @@ -17,19 +19,14 @@ suite("Extension tests", () => {

assert.match(modelFileText, /namespace example.weather/);

// Grab Language Server logs
const logText = await getLangServerLogs("suite1");

assert.notEqual(doc, undefined);
assert.notEqual(editor, undefined);
assert.equal(ext.isActive, true);
assert.match(logText, /Downloaded external jars.*smithy-aws-traits-1\.40\.0\.jar/);
assert.match(logText, /Discovered smithy files.*\/main.smithy]/);
assert.equal(diagnostics.length, 0);
}).timeout(10000);
assert.deepStrictEqual(diagnostics, []);
});

test("Should register language", async () => {
const languages = await vscode.languages.getLanguages();
assert.equal(languages.includes("smithy"), true);
}).timeout(1000);
});
});
9 changes: 6 additions & 3 deletions tests/suite2/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import * as assert from "assert";
import * as vscode from "vscode";
import { getDocUri, waitForServerStartup } from "./../helper";

suite("broken model tests", () => {
suite("broken model tests", function () {
this.timeout(0);

test("Should provide diagnostics", async () => {
const smithyMainUri = getDocUri("suite2/main.smithy");
const doc = await vscode.workspace.openTextDocument(smithyMainUri);
Expand All @@ -12,6 +14,7 @@ suite("broken model tests", () => {

assert.match(diagnostics[0].message, /Cannot apply `smithy.api#deprecated` to an immutable prelude shape/);
assert.equal(diagnostics[0].range.start.line, 4);
assert.equal(diagnostics[0].range.start.character, 0);
}).timeout(10000);
assert.equal(diagnostics[0].range.start.character, 24);
return Promise.resolve();
});
});
16 changes: 9 additions & 7 deletions tests/suite3/extension.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import * as assert from "assert";
import * as vscode from "vscode";
import { getDocUri, getLangServerLogs, waitForServerStartup } from "./../helper";
import { getDocUri, waitForServerStartup } from "./../helper";
import * as sinon from "sinon";

suite("Selector tests", () => {
suite("Selector tests", function () {
this.timeout(0);

test("Can run selectors", async () => {
const smithyMainUri = getDocUri("suite3/main.smithy");
const doc = await vscode.workspace.openTextDocument(smithyMainUri);
Expand All @@ -13,8 +14,9 @@ suite("Selector tests", () => {
const showInputBox = sinon.stub(vscode.window, "showInputBox");
showInputBox.resolves("operation [id|namespace=example.weather]");
await vscode.commands.executeCommand("smithy.runSelector");
const logText = await getLangServerLogs("suite3");

assert.match(logText, /Selector command found 4 matching shapes/);
}).timeout(10000);
// we don't have a way to check the output. as long as this command
// can run it should be fine - more robust tests are done on the server
// side.
return Promise.resolve();
});
});
14 changes: 7 additions & 7 deletions tests/suite4/extension.test.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import * as assert from "assert";
import * as vscode from "vscode";
import { getDocUri, getLangServerLogs, waitForServerStartup } from "../helper";
import { getDocUri, waitForServerStartup } from "../helper";

suite("User-specific root", function () {
this.timeout(0);

suite("User-specific root", () => {
test("Should download jars even when not in workspace root", async () => {
const smithyMainUri = getDocUri("suite4/smithy/main.smithy");
const doc = await vscode.workspace.openTextDocument(smithyMainUri);
await vscode.window.showTextDocument(doc);
await waitForServerStartup();
const diagnostics = vscode.languages.getDiagnostics(smithyMainUri);
const logText = await getLangServerLogs("suite4/smithy");

assert.match(logText, /Downloaded external jars.*smithy-aws-traits-1\.40\.0\.jar/);
assert.match(logText, /Downloaded external jars.*smithy-waiters-1\.40\.0\.jar/);
assert.doesNotMatch(logText, /Unable to resolve trait/);
// We would have diagnostics for unknown traits if the jars weren't downloaded
assert.equal(diagnostics.length, 0);
}).timeout(10000);
return Promise.resolve();
});
});
16 changes: 6 additions & 10 deletions tests/suite5/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import * as assert from "assert";
import * as vscode from "vscode";
import { getDocUri, waitForServerStartup } from "../helper";

suite("formatting tests", () => {
suite("formatting tests", function () {
this.timeout(0);

test("Should register Smithy formatter", async () => {
const smithyMainUri = getDocUri("suite5/smithy/main.smithy");
const doc = await vscode.workspace.openTextDocument(smithyMainUri);
Expand All @@ -12,13 +14,7 @@ suite("formatting tests", () => {
"vscode.executeFormatDocumentProvider",
smithyMainUri
);
const edit: vscode.TextEdit = edits[0];
const expectedRange = new vscode.Range(new vscode.Position(12, 0), new vscode.Position(12, 0));
const range = edit.range;
const newText = edit.newText;

assert.equal(edits.length, 1);
assert.equal(range.isEqual(expectedRange), true);
assert.equal(newText, " ");
}).timeout(10000);
assert.strictEqual(edits.length > 0, true, "expected edits from formatter, but got none");
return Promise.resolve();
});
});

0 comments on commit b381d23

Please sign in to comment.