Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 84 additions & 22 deletions extensions/openshell/src/manager/openshell-cli-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { existsSync, readFileSync } from 'node:fs';
import { join } from 'node:path';

import { cli, configuration, process as extensionProcess } from '@openkaiden/api';
import { beforeEach, describe, expect, test, vi } from 'vitest';
import { assert, beforeEach, describe, expect, test, vi } from 'vitest';

import { OpenshellCliManager } from './openshell-cli-manager';

Expand Down Expand Up @@ -82,17 +82,16 @@ describe('OpenshellCliManager', () => {
expect(registeredNames).toContain('openshell-gateway');

const gwCall = createCalls.find(call => call[0].name === 'openshell-gateway');
expect(gwCall).toBeDefined();
expect(gwCall![0].installationSource).toBe('extension');
expect(gwCall![0].path).toBeUndefined();
expect(gwCall![0].version).toBeUndefined();
assert(gwCall);
expect(gwCall[0].installationSource).toBe('extension');
expect(gwCall[0].path).toBeUndefined();
expect(gwCall[0].version).toBeUndefined();
});

describe('binary discovery priority', () => {
test('prefers bundled resource over system PATH', async () => {
const bundledPath = join('/resources', 'openshell', 'openshell');

Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true });
const platformArch = `${process.platform}-${process.arch}`;
const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell');

vi.mocked(existsSync).mockImplementation((p: PathLike) => {
return String(p) === bundledPath;
Expand All @@ -118,13 +117,9 @@ describe('OpenshellCliManager', () => {
expect(extensionProcess.exec).not.toHaveBeenCalledWith('openshell', expect.anything());
// only the bundled binary should have been version-checked
expect(extensionProcess.exec).toHaveBeenCalledWith(bundledPath, ['--version']);

Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true });
});

test('falls back to system PATH when no bundled resource exists', async () => {
Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true });

// no binary exists on disk
vi.mocked(existsSync).mockReturnValue(false);

Expand All @@ -143,15 +138,12 @@ describe('OpenshellCliManager', () => {
await manager.init();

expect(manager.getRegisteredPath()).toBe('/usr/local/bin/openshell');

Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true });
});

test('prefers extension storage over bundled resource when resolution is storage,bundled,system', async () => {
const storageBinPath = join(STORAGE_PATH, 'bin', 'openshell');
const bundledPath = join('/resources', 'openshell', 'openshell');

Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true });
const platformArch = `${process.platform}-${process.arch}`;
const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell');

vi.mocked(configuration.getConfiguration).mockReturnValue({
get: vi.fn().mockImplementation((key: string) => {
Expand Down Expand Up @@ -183,14 +175,11 @@ describe('OpenshellCliManager', () => {
expect(manager.getRegisteredPath()).toBe(storageBinPath);
// bundled binary should not have been checked
expect(extensionProcess.exec).not.toHaveBeenCalledWith(bundledPath, expect.anything());

Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true });
});

test('prefers system PATH over bundled resource when resolution is system,bundled,storage', async () => {
const bundledPath = join('/resources', 'openshell', 'openshell');

Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true });
const platformArch = `${process.platform}-${process.arch}`;
const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell');

vi.mocked(configuration.getConfiguration).mockReturnValue({
get: vi.fn().mockImplementation((key: string) => {
Expand Down Expand Up @@ -221,8 +210,81 @@ describe('OpenshellCliManager', () => {
expect(manager.getRegisteredPath()).toBe('/usr/local/bin/openshell');
// bundled binary should NOT have been version-checked because system PATH was found first
expect(extensionProcess.exec).not.toHaveBeenCalledWith(bundledPath, expect.anything());
});

test('uses process.resourcesPath with original subdir in production mode', async () => {
vi.stubEnv('PROD', true);
const bundledPath = join('/resources', 'openshell', 'openshell');

Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true });

vi.mocked(existsSync).mockImplementation((p: PathLike) => {
return String(p) === bundledPath;
});

vi.mocked(extensionProcess.exec).mockImplementation(async (cmd: string) => {
if (cmd === bundledPath) {
return { stdout: 'openshell 0.2.0', stderr: '', command: cmd };
}
throw new Error(`unexpected exec: ${cmd}`);
});

const manager = createManager();
await manager.init();

expect(manager.getRegisteredPath()).toBe(bundledPath);

Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true });
vi.unstubAllEnvs();
});
Comment thread
benoitf marked this conversation as resolved.

test('uses assets folder with platform-arch subdir in development mode', async () => {
const platformArch = `${process.platform}-${process.arch}`;
const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell');

vi.mocked(existsSync).mockImplementation((p: PathLike) => {
return String(p) === bundledPath;
});

vi.mocked(extensionProcess.exec).mockImplementation(async (cmd: string) => {
if (cmd === bundledPath) {
return { stdout: 'openshell 0.2.0', stderr: '', command: cmd };
}
throw new Error(`unexpected exec: ${cmd}`);
});

const manager = createManager();
await manager.init();

expect(manager.getRegisteredPath()).toBe(bundledPath);
expect(extensionProcess.exec).toHaveBeenCalledWith(bundledPath, ['--version']);
});

test('uses assets/image-builder subdir for image builder in development mode', async () => {
const platformArch = `${process.platform}-${process.arch}`;
const ibBundledPath = join(EXTENSION_URI, 'assets', 'image-builder', platformArch, 'openshell-image-builder');

vi.mocked(existsSync).mockImplementation((p: PathLike) => {
return String(p) === ibBundledPath;
});

vi.mocked(extensionProcess.exec).mockImplementation(async (cmd: string) => {
if (cmd === ibBundledPath) {
return { stdout: 'openshell-image-builder 0.9.0', stderr: '', command: cmd };
}
throw new Error(`unexpected exec: ${cmd}`);
});

const manager = createManager();
await manager.init();

expect(extensionProcess.exec).toHaveBeenCalledWith(ibBundledPath, ['--version']);

const createCalls = vi.mocked(cli.createCliTool).mock.calls;
const ibCall = createCalls.find(call => call[0].name === 'openshell-image-builder');
assert(ibCall);
expect(ibCall[0].path).toBe(ibBundledPath);
expect(ibCall[0].version).toBe('0.9.0');
});

test('prefers custom config path over all others', async () => {
Expand Down
22 changes: 19 additions & 3 deletions extensions/openshell/src/manager/openshell-cli-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export class OpenshellCliManager implements Disposable {
'openshell-image-builder',
'imageBuilder.binary.path',
'openshell-image-builder',
'image-builder',
);
const ibRegistration: BinaryDiscoveryResult = ibResult ?? {
installationSource: 'extension',
Expand Down Expand Up @@ -139,6 +140,7 @@ export class OpenshellCliManager implements Disposable {
binaryBaseName: string,
configKey: string,
resourceSubdir: string,
assetsSubdir?: string,
): Promise<BinaryDiscoveryResult | undefined> {
const binDir = join(this.extensionContext.storagePath, 'bin');
const binaryName = extensionApi.env.isWindows ? `${binaryBaseName}.exe` : binaryBaseName;
Expand Down Expand Up @@ -167,7 +169,7 @@ export class OpenshellCliManager implements Disposable {
result = await this.discoverFromExtensionStorage(binaryBaseName, localBinaryPath);
break;
case 'bundled':
result = await this.discoverFromBundledResources(binaryBaseName, binaryName, resourceSubdir);
result = await this.discoverFromBundledResources(binaryBaseName, binaryName, resourceSubdir, assetsSubdir);
break;
case 'system':
result = await this.discoverFromSystemPath(binaryBaseName);
Expand Down Expand Up @@ -198,10 +200,24 @@ export class OpenshellCliManager implements Disposable {
binaryBaseName: string,
binaryName: string,
resourceSubdir: string,
assetsSubdir?: string,
): Promise<BinaryDiscoveryResult | undefined> {
const resourcesPath = (process as NodeJS.Process & { resourcesPath?: string }).resourcesPath;
let resourcesPath: string | undefined;
let bundledResourceSubdir: string;
if (import.meta.env.PROD) {
resourcesPath = (process as NodeJS.Process & { resourcesPath?: string }).resourcesPath;
bundledResourceSubdir = resourceSubdir;
} else {
Comment thread
benoitf marked this conversation as resolved.
// In development mode, use the path used by download scripts in assets folder
const parts: string[] = [this.extensionContext.extensionUri.fsPath, 'assets'];
if (assetsSubdir) {
parts.push(assetsSubdir);
}
resourcesPath = join(...parts);
bundledResourceSubdir = `${process.platform}-${process.arch}`;
}
if (resourcesPath) {
const bundledBinaryPath = join(resourcesPath, resourceSubdir, binaryName);
const bundledBinaryPath = join(resourcesPath, bundledResourceSubdir, binaryName);
console.log(`[${binaryBaseName}] checking bundled resources at ${bundledBinaryPath}`);
if (existsSync(bundledBinaryPath)) {
const version = await this.getVersion(bundledBinaryPath);
Expand Down
2 changes: 1 addition & 1 deletion extensions/openshell/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@
"/@/*": ["./src/*"]
}
},
"include": ["src"]
"include": ["src", "../../types/*.d.ts"]
}
Loading