Skip to content

Commit

Permalink
feat: DHE server config (#103)
Browse files Browse the repository at this point in the history
Added support for configuring DHE servers + associated tests. Nothing is
consuming this yet, but trying to keep PRs smaller.

supports #79
  • Loading branch information
bmingles authored Aug 15, 2024
1 parent 83050dc commit 133527f
Show file tree
Hide file tree
Showing 13 changed files with 230 additions and 64 deletions.
20 changes: 20 additions & 0 deletions e2e/global.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import * as vscode from 'vscode';

declare global {
namespace WebdriverIO {
interface Browser {
/** Extend default types for better type safety of callback args. */
executeWorkbench: <
TArgs extends unknown[],
TResult extends unknown,
TCallback extends (
vs: typeof vscode,
...args: TArgs
) => Promise<TResult>,
>(
callback: TCallback,
...args: TArgs
) => Promise<TResult>;
}
}
}
41 changes: 41 additions & 0 deletions e2e/specs/config.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { expect } from '@wdio/globals';
import { getConfig, resetConfig, setConfigSectionSettings } from '../testUtils';

afterEach(async () => {
await resetConfig();
});

describe('Extension config testing', () => {
it('should default to the correct settings', async () => {
const config = await getConfig();

expect(config).toStrictEqual({
'core-servers': ['http://localhost:10000/'],
'enterprise-servers': [],
});
});

(
[
['Empty configs', [], []],
[
'Populated configs',
['core-a', 'core-b'],
['enterprise-a', 'enterprise-b'],
],
] as const
).forEach(([label, coreServers, enterpriseServers]) => {
it(`should return custom settings: ${label}`, async () => {
await setConfigSectionSettings('core-servers', coreServers);
await setConfigSectionSettings('enterprise-servers', enterpriseServers);

const config = await getConfig();

expect(config).toStrictEqual({
'core-servers': coreServers,
'enterprise-servers': enterpriseServers,
});
});
});
});
14 changes: 9 additions & 5 deletions e2e/specs/test.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
hasConnectionStatusBarItem,
openEditors,
PYTHON_AND_GROOVY_SERVER_CONFIG,
setCoreServerSettings,
resetConfig,
setConfigSectionSettings,
} from '../testUtils';

// There are some tests that can be used for reference in:
Expand All @@ -22,12 +23,15 @@ describe('VS Code Extension Testing', () => {

describe('Connection status bar item', () => {
beforeEach(async () => {
await setCoreServerSettings(PYTHON_AND_GROOVY_SERVER_CONFIG);
await setConfigSectionSettings(
'core-servers',
PYTHON_AND_GROOVY_SERVER_CONFIG
);
await openEditors(['test.txt', 'test.groovy', 'test.py']);
});

afterEach(async () => {
await setCoreServerSettings(undefined);
await resetConfig();
await closeAllEditors();
});

Expand All @@ -45,8 +49,8 @@ describe('Connection status bar item', () => {
await workbench.getEditorView().openEditor(supportedTitle);
expect(await hasConnectionStatusBarItem()).toBeTruthy();

// Set to empty string to clear all server configs
await setCoreServerSettings([]);
// Set to empty array to clear all server configs
await setConfigSectionSettings('core-servers', []);
expect(await hasConnectionStatusBarItem()).toBeFalsy();
});
});
Expand Down
72 changes: 54 additions & 18 deletions e2e/testUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
import * as vscode from 'vscode';

// Note that calls to `browser.executeWorkbench` cannot reference any variables
// or functions from the outside scope. They only have access to variables
// passed in as additional parameters.
// See https://www.npmjs.com/package/wdio-vscode-service#accessing-vscode-apis

// EXTENSION_ID and ConfigSectionKey are based on `src/common/constants.ts`.
// We can't currently import source code from the extension into the e2e tests
// due to isolated tsconfigs. Should be fine for now since the duplication is
// small and tests should fail if things get out of sync. If this duplication
// grows, will need to figure out how to reconfigure to support importing from
// the source code.
const EXTENSION_ID = 'vscode-deephaven' as const;
type ConfigSectionKey = 'core-servers' | 'enterprise-servers';

export const PYTHON_AND_GROOVY_SERVER_CONFIG = [
'http://localhost:10000',
{
Expand Down Expand Up @@ -35,10 +49,7 @@ export async function hasConnectionStatusBarItem(): Promise<boolean> {
* @param editorTitles The titles of the editors to open.
*/
export async function openEditors(editorTitles: string[]): Promise<void> {
// Note that calls to `browser.executeWorkbench` cannot reference any variables
// or functions from the outside scope. They only have access to variables
// passed in as additional parameters.
// See https://www.npmjs.com/package/wdio-vscode-service#accessing-vscode-apis
// See note about `executeWorkbench` at top of this file.
await browser.executeWorkbench(
async (vs: typeof vscode, editorTitles: string[]): Promise<void> => {
const filePathsToOpen = editorTitles.map(
Expand All @@ -62,24 +73,49 @@ export async function closeAllEditors(): Promise<void> {
}

/**
* Set the core server settings in the extension configuration.
* @param config The core server settings to set. Setting to `undefined` will
* result in the default configuration vs an `[]` will actually clear the config
* completely.
* Get the configuration settings for the extension.
* @returns The configuration settings for the extension.
*/
export async function getConfig(): Promise<vscode.WorkspaceConfiguration> {
// See note about `executeWorkbench` at top of this file.
return browser.executeWorkbench(async (vs: typeof vscode, extensionIdIn) => {
return vs.workspace.getConfiguration(extensionIdIn);
}, EXTENSION_ID);
}

/**
* Reset all configuration settings to their default values.
*/
export async function resetConfig(): Promise<void> {
await setConfigSectionSettings('core-servers', undefined);
await setConfigSectionSettings('enterprise-servers', undefined);
}

/**
* Set the section settings in the extension configuration.
* @param sectionKey The section key to set.
* @param sectionValue The settings to set. Setting to `undefined` will
* result in the default configuration vs defined value will actually clear the
* config completely.
*/
export async function setCoreServerSettings(
config: readonly unknown[] | undefined
export async function setConfigSectionSettings(
sectionKey: ConfigSectionKey,
sectionValue: unknown | undefined
): Promise<void> {
// Note that calls to `browser.executeWorkbench` cannot reference any variables
// or functions from the outside scope. They only have access to variables
// passed in as additional parameters.
// See https://www.npmjs.com/package/wdio-vscode-service#accessing-vscode-apis
// See note about `executeWorkbench` at top of this file.
await browser.executeWorkbench(
async (vs: typeof vscode, config): Promise<void> => {
async (
vs: typeof vscode,
extensionIdIn: typeof EXTENSION_ID,
sectionKeyIn: ConfigSectionKey,
sectionValueIn: unknown | undefined
): Promise<void> => {
await vs.workspace
.getConfiguration('vscode-deephaven')
.update('core-servers', config ?? undefined);
.getConfiguration(extensionIdIn)
.update(sectionKeyIn, sectionValueIn ?? undefined);
},
config
EXTENSION_ID,
sectionKey,
sectionValue
);
}
8 changes: 7 additions & 1 deletion e2e/wdio.conf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ export const config: Options.Testrunner = {
// and 30 processes will get spawned. The property handles how many capabilities
// from the same test should run tests.
//
maxInstances: 10,
// TODO: Unfortunately, `wdio-vscode-service` doesn't seem to have support for isolating
// workspaces to a test suite. This means that they all share the same workspace,
// and things like updating config settings as part of a test will affect other
// tests. This PR was created to request a feature to support this:
// https://github.com/webdriverio-community/wdio-vscode-service/issues/132
// In the meantime, we'll disable concurrency.
maxInstances: 1,
//
// If you have trouble getting all important capabilities together, check out the
// Sauce Labs platform configurator - a great tool to configure your capabilities:
Expand Down
8 changes: 8 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,14 @@
"http://localhost:10000/"
],
"description": "Deephaven Core servers"
},
"vscode-deephaven.enterprise-servers": {
"type": "array",
"items": {
"type": "string"
},
"default": [],
"description": "Deephaven Enterprise servers"
}
}
},
Expand Down
17 changes: 11 additions & 6 deletions src/common/constants.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import * as path from 'node:path';

export const CONFIG_KEY = 'vscode-deephaven';
export const CONFIG_CORE_SERVERS = 'core-servers';
export const EXTENSION_ID = 'vscode-deephaven' as const;

export const CONFIG_KEY = {
root: EXTENSION_ID,
coreServers: 'core-servers',
enterpriseServers: 'enterprise-servers',
} as const;

export const DEFAULT_CONSOLE_TYPE = 'python' as const;
// export const DHFS_SCHEME = 'dhfs';
export const DOWNLOAD_LOGS_CMD = `${CONFIG_KEY}.downloadLogs`;
export const RUN_CODE_COMMAND = `${CONFIG_KEY}.runCode`;
export const RUN_SELECTION_COMMAND = `${CONFIG_KEY}.runSelection`;
export const SELECT_CONNECTION_COMMAND = `${CONFIG_KEY}.selectConnection`;
export const DOWNLOAD_LOGS_CMD = `${EXTENSION_ID}.downloadLogs`;
export const RUN_CODE_COMMAND = `${EXTENSION_ID}.runCode`;
export const RUN_SELECTION_COMMAND = `${EXTENSION_ID}.runSelection`;
export const SELECT_CONNECTION_COMMAND = `${EXTENSION_ID}.selectConnection`;

export const STATUS_BAR_DISCONNECTED_TEXT = 'Deephaven: Disconnected';
export const STATUS_BAR_DISCONNECT_TEXT = 'Deephaven: Disconnect';
Expand Down
18 changes: 12 additions & 6 deletions src/common/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,24 @@ export type ConnectionAndSession<TConnection, TSession> = {

export type ConsoleType = 'groovy' | 'python';

export interface ConnectionConfig {
url: string;
consoleType: ConsoleType;
}

export type ConnectionConfigStored =
export type CoreConnectionConfigStored =
| string
| {
url: string;
consoleType?: ConsoleType;
};

export interface CoreConnectionConfig {
url: string;
consoleType: ConsoleType;
}

export type EnterpriseConnectionConfigStored = string;

export interface EnterpriseConnectionConfig {
url: string;
}

export interface Disposable {
dispose(): Promise<void>;
}
29 changes: 21 additions & 8 deletions src/services/Config.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as vscode from 'vscode';
import { beforeEach, describe, expect, it, Mock, vi } from 'vitest';
import { Config } from './Config';
import { CONFIG_CORE_SERVERS } from '../common';
import { CONFIG_KEY } from '../common';

// See __mocks__/vscode.ts for the mock implementation
vi.mock('vscode');
Expand All @@ -19,12 +19,12 @@ beforeEach(() => {
).mockReturnValue(configMap as unknown as vscode.WorkspaceConfiguration);
});

describe('Config', () => {
const urlA = 'http://someUrl';
const urlB = 'http://someOtherUrl';
const urlC = 'http://someAdditionalUrl';
const urlInvalid = 'invalidUrl';
const urlA = 'http://someUrl';
const urlB = 'http://someOtherUrl';
const urlC = 'http://someAdditionalUrl';
const urlInvalid = 'invalidUrl';

describe('getCoreServers', () => {
it.each([
['Empty config', [], []],
[
Expand All @@ -36,7 +36,7 @@ describe('Config', () => {
],
],
[
'Default url',
'Default consoleType',
[{ url: urlA }, { url: urlB }, { url: urlInvalid }],
[
{ url: urlA, consoleType: 'python' },
Expand All @@ -57,10 +57,23 @@ describe('Config', () => {
],
],
])('should return core servers: %s', (_label, given, expected) => {
configMap.set(CONFIG_CORE_SERVERS, given);
configMap.set(CONFIG_KEY.coreServers, given);

const config = Config.getCoreServers();

expect(config).toEqual(expected);
});
});

describe('getEnterpriseServers', () => {
it.each([
['Empty config', [], []],
['String config', [urlA, urlB, urlInvalid], [{ url: urlA }, { url: urlB }]],
])('should return enterprise servers: %s', (_label, given, expected) => {
configMap.set(CONFIG_KEY.enterpriseServers, given);

const config = Config.getEnterpriseServers();

expect(config).toEqual(expected);
});
});
Loading

0 comments on commit 133527f

Please sign in to comment.