Skip to content

Commit

Permalink
test/gopls: change test environment setup to use single file edit
Browse files Browse the repository at this point in the history
When we wrote the first gopls integration test, gopls required
vscode to open the workspace folder while vscode code does not
support dynamic workspace folder registration in test mode. As a
result, we ended up having a fixed workspace folder as a test fixture,
starting the test code instance with the folder, and copying
necessary files to the folder. There were so many moving parts
and this created race conditions and caused slow test run.

Since v0.4.x, gopls starts to support single file edit and automatically
constructs an internal workspace by walking the directory tree
around the open file. This CL utilizes this new capability.
In each suite, we start testing by starting a new gopls, opening
a single file, and waiting for the gopls to finish the initial package
loading.

This CL introduces Env.onPatternInTrace, which watches the
fake trace output channel, and emits an event when a registered
pattern is observed. This is a hack to wait for the gopls's internal
state to reach to a desirable state - ideally, we want to intercept
all the progress report messages like gopls' regression tests
once it is supported from the language client middleware.
microsoft/vscode-languageserver-node#671

We also identified subtle issues in the existing test setup.

Issue 1: when the code for testing starts (using `vscode-test`'s
`runTests`) we pass the extension development path. It seems like
the vscode instance uses the `main` program specified in `package.json`
and activates it even without us asking. As a result, when we run
tests and call 'activate' again, multiple hover/completion providers
are registered, and vscode returns results from legacy and gopls-based
providers. For example, the completion middleware test was observing
entries from gopls, and goCompletionItemProvider that uses gocode.

We address this issue here by introducing the VSCODE_GO_IN_TEST
environment variable. If it is set, activate will return immediately.
So, tests can control when to register what, and how.

We need this setting in both `launch.json` and `runTest.ts` that's
invoked in CI (`npm run test`)

Issue 2: when the code for testing needs to call `activate`, we
got the extension instance by using `vscode.extensions.getExtension`
and called its `activate`. This was because there is no easy way
to supply sufficiently complete vscode's ExtensionContext.
It turned out, the extension instance vscode.extensions.getExtension
returns is the one built with the `main` program specified in
our `package.json` - that is the webpack'ed one in `dist/goMain.js`.
On the other hand, our debugging depends on pre-webpack versions
in `out/*`. This caused confusion and made debugging near impossible
(breakpoints we set on pre-webpack versions will not be hit because
we are running a different version of extension)!

We don't know if there is a way to teach `vscode-test` to use pre-webpack
version. Maybe this is our misconfiguration in our launch.json and
package.json. For now, we work around this issue by not calling
`activate`. Instead, in this gopls test, we call `buildLanguageClient`
directly. This required some refactoring work in goLanguageServer.ts.

Issue 3: sinon is cool, but stubbing vscode API for channel creation
is too much. While we refactor buildLanguageClient, we made changes
to let the caller supply the output channels.

Issue 4: as `vscode-test` starts the test vscode instance, it also
activates the registered snippets and it interferes with our gopls
completion middleware tests. In test, now we explicitly filter out
the snippet entries.

Issue 5: for some reason, the first entry in the completion middleware
test that expects 'Print' item, the filter text is not set. It can be
a bug, or working as intended (the first item has label === filterText).
Gopls is providing the expected entry. Workaround this issue by inspecting
the label field too.

Updates #655
Updates #832

Change-Id: Ic7088fd551329d1c8f78078ccb24a5f529eec72a
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/266418
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
  • Loading branch information
hyangah committed Nov 10, 2020
1 parent 8e1dadd commit e81c30f
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 128 deletions.
9 changes: 8 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,14 @@
"--timeout",
"999999"
],
"env": {
"VSCODE_GO_IN_TEST": "1" // Disable code that shouldn't be used in test
},
"stopOnEntry": false,
"sourceMaps": true,
"smartStep": true,
"outFiles": [
"${workspaceFolder}/out/**/*.js",
"${workspaceFolder}/out/test/**/*.js"
],
"preLaunchTask": "npm: watch"
Expand All @@ -83,11 +87,14 @@
"--extensionDevelopmentPath=${workspaceFolder}",
"--extensionTestsPath=${workspaceFolder}/out/test/gopls/index",
"--timeout", "999999",
"${workspaceFolder}/test/gopls/testfixtures/src/workspace" // gopls requires a workspace to work with.
],
"env": {
"VSCODE_GO_IN_TEST": "1" // Disable code that shouldn't be used in test
},
"stopOnEntry": false,
"sourceMaps": true,
"outFiles": [
"${workspaceFolder}/out/**/*.js",
"${workspaceFolder}/out/test/**/*.js"
],
"preLaunchTask": "npm: watch",
Expand Down
2 changes: 1 addition & 1 deletion src/goCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function check(fileUri: vscode.Uri, goConfig: vscode.WorkspaceConfigurati

// If a user has enabled diagnostics via a language server,
// then we disable running build or vet to avoid duplicate errors and warnings.
const lspConfig = buildLanguageServerConfig();
const lspConfig = buildLanguageServerConfig(goConfig);
const disableBuildAndVet = lspConfig.enabled && lspConfig.features.diagnostics;

let testPromise: Thenable<boolean>;
Expand Down
6 changes: 5 additions & 1 deletion src/goInstallTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,11 @@ export async function offerToInstallTools() {
});
}

const goConfig = getGoConfig();
if (!goConfig['useLanguageServer']) {
return;
}

const usingSourceGraph = getToolFromToolPath(getLanguageServerToolPath()) === 'go-langserver';
if (usingSourceGraph && goVersion.gt('1.10')) {
const promptMsg =
Expand All @@ -476,7 +481,6 @@ export async function offerToInstallTools() {
if (selected === installLabel) {
await installTools([getTool('gopls')], goVersion);
} else if (selected === disableLabel) {
const goConfig = getGoConfig();
const inspectLanguageServerSetting = goConfig.inspect('useLanguageServer');
if (inspectLanguageServerSetting.globalValue === true) {
goConfig.update('useLanguageServer', false, vscode.ConfigurationTarget.Global);
Expand Down
58 changes: 36 additions & 22 deletions src/goLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import { getFromGlobalState, updateGlobalState } from './stateUtils';
import { getBinPath, getCurrentGoPath, getGoConfig, getGoplsConfig, getWorkspaceFolderPath } from './util';
import { getToolFromToolPath } from './utils/pathUtils';

interface LanguageServerConfig {
export interface LanguageServerConfig {
serverName: string;
path: string;
version: string;
Expand Down Expand Up @@ -102,7 +102,7 @@ let lastUserAction: Date = new Date();
// startLanguageServerWithFallback starts the language server, if enabled,
// or falls back to the default language providers.
export async function startLanguageServerWithFallback(ctx: vscode.ExtensionContext, activation: boolean) {
const cfg = buildLanguageServerConfig();
const cfg = buildLanguageServerConfig(getGoConfig());

// If the language server is gopls, we enable a few additional features.
// These include prompting for updates and surveys.
Expand Down Expand Up @@ -134,7 +134,7 @@ function scheduleGoplsSuggestions(tool: Tool) {
const update = async () => {
setTimeout(update, timeDay);

const cfg = buildLanguageServerConfig();
const cfg = buildLanguageServerConfig(getGoConfig());
if (!cfg.enabled) {
return;
}
Expand All @@ -146,7 +146,7 @@ function scheduleGoplsSuggestions(tool: Tool) {
const survey = async () => {
setTimeout(survey, timeDay);

const cfg = buildLanguageServerConfig();
const cfg = buildLanguageServerConfig(getGoConfig());
if (!goplsSurveyOn || !cfg.enabled) {
return;
}
Expand Down Expand Up @@ -176,7 +176,7 @@ async function startLanguageServer(ctx: vscode.ExtensionContext, config: Languag
// Track the latest config used to start the language server,
// and rebuild the language client.
latestConfig = config;
languageClient = await buildLanguageClient(config);
languageClient = await buildLanguageClient(buildLanguageClientOption(config));
crashCount = 0;
}

Expand Down Expand Up @@ -206,21 +206,38 @@ async function startLanguageServer(ctx: vscode.ExtensionContext, config: Languag
return true;
}

async function buildLanguageClient(cfg: LanguageServerConfig): Promise<LanguageClient> {
// Reuse the same output channel for each instance of the server.
if (cfg.enabled) {
if (!serverOutputChannel) {
serverOutputChannel = vscode.window.createOutputChannel(cfg.serverName + ' (server)');
}
if (!serverTraceChannel) {
serverTraceChannel = vscode.window.createOutputChannel(cfg.serverName);
export interface BuildLanguageClientOption extends LanguageServerConfig {
outputChannel?: vscode.OutputChannel;
traceOutputChannel?: vscode.OutputChannel;
}

// buildLanguageClientOption returns the default, extra configuration
// used in building a new LanguageClient instance. Options specified
// in LanguageServerConfig
function buildLanguageClientOption(cfg: LanguageServerConfig): BuildLanguageClientOption {
// Reuse the same output channel for each instance of the server.
if (cfg.enabled) {
if (!serverOutputChannel) {
serverOutputChannel = vscode.window.createOutputChannel(cfg.serverName + ' (server)');
}
if (!serverTraceChannel) {
serverTraceChannel = vscode.window.createOutputChannel(cfg.serverName);
}
}
}
return Object.assign({
outputChannel: serverOutputChannel,
traceOutputChannel: serverTraceChannel
}, cfg);
}

// buildLanguageClient returns a language client built using the given language server config.
// The returned language client need to be started before use.
export async function buildLanguageClient(cfg: BuildLanguageClientOption): Promise<LanguageClient> {
let goplsWorkspaceConfig = getGoplsConfig();
goplsWorkspaceConfig = await adjustGoplsWorkspaceConfiguration(cfg, goplsWorkspaceConfig);
const c = new LanguageClient(
'go', // id
cfg.serverName, // name
cfg.serverName, // name e.g. gopls
{
command: cfg.path,
args: ['-mode=stdio', ...cfg.flags],
Expand All @@ -235,8 +252,8 @@ async function buildLanguageClient(cfg: LanguageServerConfig): Promise<LanguageC
(uri.scheme ? uri : uri.with({ scheme: 'file' })).toString(),
protocol2Code: (uri: string) => vscode.Uri.parse(uri)
},
outputChannel: serverOutputChannel,
traceOutputChannel: serverTraceChannel,
outputChannel: cfg.outputChannel,
traceOutputChannel: cfg.traceOutputChannel,
revealOutputChannelOn: RevealOutputChannelOn.Never,
initializationFailedHandler: (error: WebRequest.ResponseError<InitializeError>): boolean => {
vscode.window.showErrorMessage(
Expand Down Expand Up @@ -550,8 +567,8 @@ export function watchLanguageServerConfiguration(e: vscode.ConfigurationChangeEv
}
}

export function buildLanguageServerConfig(): LanguageServerConfig {
const goConfig = getGoConfig();
export function buildLanguageServerConfig(goConfig: vscode.WorkspaceConfiguration): LanguageServerConfig {

const cfg: LanguageServerConfig = {
serverName: '',
path: '',
Expand Down Expand Up @@ -604,9 +621,6 @@ Please try reinstalling it.`);
*/
export function getLanguageServerToolPath(): string {
const goConfig = getGoConfig();
if (!goConfig['useLanguageServer']) {
return;
}
// Check that all workspace folders are configured with the same GOPATH.
if (!allFoldersHaveSameGopath()) {
vscode.window.showInformationMessage(
Expand Down
3 changes: 3 additions & 0 deletions src/goMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ export let vetDiagnosticCollection: vscode.DiagnosticCollection;
export let restartLanguageServer = () => { return; };

export function activate(ctx: vscode.ExtensionContext) {
if (process.env['VSCODE_GO_IN_TEST'] === '1') { // Make sure this does not run when running in test.
return;
}
const cfg = getGoConfig();
setLogConfig(cfg['logging']);

Expand Down
6 changes: 3 additions & 3 deletions src/goStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { formatGoVersion, GoEnvironmentOption, terminalCreationListener } from '
import { buildLanguageServerConfig, getLocalGoplsVersion, serverOutputChannel } from './goLanguageServer';
import { isGoFile } from './goMode';
import { getModFolderPath, isModSupported } from './goModules';
import { getGoVersion } from './util';
import { getGoConfig, getGoVersion } from './util';

export let outputChannel = vscode.window.createOutputChannel('Go');

Expand Down Expand Up @@ -47,7 +47,7 @@ export async function expandGoStatusBar() {
];

// Get the gopls configuration
const cfg = buildLanguageServerConfig();
const cfg = buildLanguageServerConfig(getGoConfig());
if (cfg.serverName === 'gopls') {
const goplsVersion = await getLocalGoplsVersion(cfg);
options.push({label: `${languageServerIcon}Open 'gopls' trace`, description: `${goplsVersion}`});
Expand Down Expand Up @@ -101,7 +101,7 @@ export async function initGoStatusBar() {
// Add an icon to indicate that the 'gopls' server is running.
// Assume if it is configured it is already running, since the
// icon will be updated on an attempt to start.
const cfg = buildLanguageServerConfig();
const cfg = buildLanguageServerConfig(getGoConfig());
updateLanguageServerIconGoStatusBar(true, cfg.serverName);

showGoStatusBar();
Expand Down
Loading

0 comments on commit e81c30f

Please sign in to comment.