Skip to content

Commit

Permalink
fix: update podman api and replace local runCliCommand with process.e…
Browse files Browse the repository at this point in the history 
…xec (#44)

Signed-off-by: lstocchi <lstocchi@redhat.com>
  • Loading branch information
@lstocchi
lstocchi authored May 22, 2024
1 parent 60eae96 commit 90c3701
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 237 deletions.
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"publisher": "podman-desktop",
"license": "Apache-2.0",
"engines": {
"podman-desktop": "^1.0.0"
"podman-desktop": ">=1.10.0"
},
"main": "./dist/extension.js",
"contributes": {
Expand Down Expand Up @@ -80,7 +80,7 @@
},
"devDependencies": {
"7zip-min": "^1.4.4",
"@podman-desktop/api": "^1.6.4",
"@podman-desktop/api": "^1.10.0",
"@typescript-eslint/eslint-plugin": "^6.19.1",
"@typescript-eslint/parser": "^6.19.1",
"@vitest/coverage-v8": "^1.2.1",
Expand Down
21 changes: 10 additions & 11 deletions src/create-cluster.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import { beforeEach, expect, test, vi } from 'vitest';
import type { Mock } from 'vitest';
import { createCluster } from './create-cluster';
import { runCliCommand } from './util';
import type { TelemetryLogger } from '@podman-desktop/api';
import * as extensionApi from '@podman-desktop/api';

Expand All @@ -29,12 +28,14 @@ vi.mock('@podman-desktop/api', async () => {
kubernetes: {
createResources: vi.fn(),
},
process: {
exec: vi.fn(),
},
};
});

vi.mock('./util', async () => {
return {
runCliCommand: vi.fn(),
getMinikubePath: vi.fn(),
};
});
Expand All @@ -52,7 +53,7 @@ const telemetryLoggerMock = {

test('expect error is cli returns non zero exit code', async () => {
try {
(runCliCommand as Mock).mockReturnValue({ exitCode: -1, error: 'error' });
(extensionApi.process.exec as Mock).mockReturnValue({ exitCode: -1, error: 'error' });
await createCluster({}, undefined, '', telemetryLoggerMock, undefined);
} catch (err) {
expect(err).to.be.a('Error');
Expand All @@ -62,7 +63,7 @@ test('expect error is cli returns non zero exit code', async () => {
});

test('expect cluster to be created', async () => {
(runCliCommand as Mock).mockReturnValue({ exitCode: 0 });
(extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 });
await createCluster({}, undefined, '', telemetryLoggerMock, undefined);
expect(telemetryLogUsageMock).toHaveBeenNthCalledWith(
1,
Expand All @@ -75,7 +76,7 @@ test('expect cluster to be created', async () => {

test('expect error if Kubernetes reports error', async () => {
try {
(runCliCommand as Mock).mockReturnValue({ exitCode: 0 });
(extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 });
const logger = {
log: vi.fn(),
error: vi.fn(),
Expand All @@ -95,7 +96,7 @@ test('expect error if Kubernetes reports error', async () => {
});

test('expect cluster to be created with custom base image', async () => {
(runCliCommand as Mock).mockReturnValue({ exitCode: 0 });
(extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 });
await createCluster(
{ 'minikube.cluster.creation.base-image': 'myCustomImage' },
undefined,
Expand All @@ -110,7 +111,7 @@ test('expect cluster to be created with custom base image', async () => {
);
expect(telemetryLogErrorMock).not.toBeCalled();
expect(extensionApi.kubernetes.createResources).not.toBeCalled();
expect(runCliCommand).toBeCalledWith(
expect(extensionApi.process.exec).toBeCalledWith(
'',
[
'start',
Expand All @@ -124,12 +125,11 @@ test('expect cluster to be created with custom base image', async () => {
'myCustomImage',
],
expect.anything(),
undefined,
);
});

test('expect cluster to be created with custom mount', async () => {
(runCliCommand as Mock).mockReturnValue({ exitCode: 0 });
(extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 });
await createCluster(
{ 'minikube.cluster.creation.mount-string': '/foo:/bar' },
undefined,
Expand All @@ -144,7 +144,7 @@ test('expect cluster to be created with custom mount', async () => {
);
expect(telemetryLogErrorMock).not.toBeCalled();
expect(extensionApi.kubernetes.createResources).not.toBeCalled();
expect(runCliCommand).toBeCalledWith(
expect(extensionApi.process.exec).toBeCalledWith(
'',
[
'start',
Expand All @@ -159,6 +159,5 @@ test('expect cluster to be created with custom mount', async () => {
'/foo:/bar',
],
expect.anything(),
undefined,
);
});
5 changes: 3 additions & 2 deletions src/create-cluster.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
*
* SPDX-License-Identifier: Apache-2.0
***********************************************************************/
import { getMinikubePath, runCliCommand } from './util';
import { getMinikubePath } from './util';

import type { Logger, TelemetryLogger, CancellationToken } from '@podman-desktop/api';
import { process as processApi } from '@podman-desktop/api';

export async function createCluster(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -52,7 +53,7 @@ export async function createCluster(

// now execute the command to create the cluster
try {
await runCliCommand(minikubeCli, startArgs, { env, logger }, token);
await processApi.exec(minikubeCli, startArgs, { env, logger, token });
telemetryLogger.logUsage('createCluster', { driver, runtime });
} catch (error) {
const errorMessage = error instanceof Error ? error.message : error;
Expand Down
4 changes: 2 additions & 2 deletions src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
***********************************************************************/

import * as extensionApi from '@podman-desktop/api';
import { detectMinikube, getMinikubePath, runCliCommand } from './util';
import { detectMinikube, getMinikubePath } from './util';
import { MinikubeInstaller } from './minikube-installer';
import type { CancellationToken, Logger } from '@podman-desktop/api';
import { window } from '@podman-desktop/api';
Expand Down Expand Up @@ -123,7 +123,7 @@ async function updateClusters(provider: extensionApi.Provider, containers: exten
delete: async (logger): Promise<void> => {
const env = Object.assign({}, process.env);
env.PATH = getMinikubePath();
await runCliCommand(minikubeCli, ['delete', '--profile', cluster.name], { env, logger });
await extensionApi.process.exec(minikubeCli, ['delete', '--profile', cluster.name], { env, logger });
},
};
// create a new connection
Expand Down
12 changes: 7 additions & 5 deletions src/image-handler.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ import type { Mock } from 'vitest';
import { ImageHandler } from './image-handler';
import * as extensionApi from '@podman-desktop/api';
import * as fs from 'node:fs';
import { getMinikubePath, runCliCommand } from './util';
import { getMinikubePath } from './util';

let imageHandler: ImageHandler;
vi.mock('@podman-desktop/api', async () => {
return {
containerEngine: {
saveImage: vi.fn(),
},
process: {
exec: vi.fn(),
},
window: {
showNotification: vi.fn(),
showInformationMessage: vi.fn(),
Expand All @@ -38,7 +41,6 @@ vi.mock('@podman-desktop/api', async () => {

vi.mock('./util', async () => {
return {
runCliCommand: vi.fn().mockReturnValue({ exitCode: 0 }),
getMinikubePath: vi.fn(),
};
});
Expand Down Expand Up @@ -119,9 +121,9 @@ test('expect cli is called with right PATH', async () => {
);
expect(getMinikubePath).toBeCalled();

expect(runCliCommand).toBeCalledTimes(1);
// grab the env parameter of the first call to runCliCommand
const props = (runCliCommand as Mock).mock.calls[0][2];
expect(extensionApi.process.exec).toBeCalledTimes(1);
// grab the env parameter of the first call to process.Exec
const props = (extensionApi.process.exec as Mock).mock.calls[0][2];
expect(props).to.have.property('env');
const env = props.env;
expect(env).to.have.property('PATH');
Expand Down
4 changes: 2 additions & 2 deletions src/image-handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import type { MinikubeCluster } from './extension';
import * as extensionApi from '@podman-desktop/api';
import { tmpName } from 'tmp-promise';
import { getMinikubePath, runCliCommand } from './util';
import { getMinikubePath } from './util';
import * as fs from 'node:fs';

type ImageInfo = { engineId: string; name?: string; tag?: string };
Expand Down Expand Up @@ -71,7 +71,7 @@ export class ImageHandler {
await extensionApi.containerEngine.saveImage(image.engineId, name, filename);

// Run the minikube image load command to push the image to the cluster
await runCliCommand(minikubeCli, ['-p', selectedCluster.label, 'image', 'load', filename], {
await extensionApi.process.exec(minikubeCli, ['-p', selectedCluster.label, 'image', 'load', filename], {
env: env,
});

Expand Down
4 changes: 0 additions & 4 deletions src/minikube-installer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ const telemetryLoggerMock = {
logError: telemetryLogErrorMock,
} as unknown as extensionApi.TelemetryLogger;

vi.mock('runCliCommand', async () => {
return vi.fn();
});

beforeEach(() => {
installer = new MinikubeInstaller('.', telemetryLoggerMock);
vi.clearAllMocks();
Expand Down
122 changes: 11 additions & 111 deletions src/util.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@

import * as extensionApi from '@podman-desktop/api';
import { afterEach, beforeEach, expect, test, vi } from 'vitest';
import { detectMinikube, getMinikubePath, installBinaryToSystem, runCliCommand } from './util';
import * as childProcess from 'node:child_process';
import { detectMinikube, getMinikubePath, installBinaryToSystem } from './util';
import type { MinikubeInstaller } from './minikube-installer';
import * as fs from 'node:fs';
import * as path from 'node:path';
Expand Down Expand Up @@ -86,126 +85,27 @@ test('getMinikubePath on macOS with existing PATH', async () => {
expect(computedPath).toEqual(`${existingPATH}:/usr/local/bin:/opt/homebrew/bin:/opt/local/bin:/opt/podman/bin`);
});

test.each([
['macOS', true, false],
['windows', false, true],
])('detectMinikube on %s', async (operatingSystem, isMac, isWindows) => {
vi.mocked(extensionApi.env).isMac = isMac;
vi.mocked(extensionApi.env).isWindows = isWindows;

// spy on runCliCommand
const spawnSpy = vi.spyOn(childProcess, 'spawn');

const onEventMock = vi.fn();

onEventMock.mockImplementation((event: string, callback: (data: string) => void) => {
// delay execution
if (event === 'close') {
setTimeout(() => {
callback(0 as unknown as string);
}, 500);
}
});

spawnSpy.mockReturnValue({
on: onEventMock,
stdout: { setEncoding: vi.fn(), on: vi.fn() },
stderr: { setEncoding: vi.fn(), on: vi.fn() },
} as unknown as childProcess.ChildProcessWithoutNullStreams);

test('detectMinikube', async () => {
const fakeMinikubeInstaller = {
getAssetInfo: vi.fn(),
} as unknown as MinikubeInstaller;

const execMock = vi.spyOn(extensionApi.process, 'exec').mockResolvedValue({
command: '',
stderr: '',
stdout: '',
});

const result = await detectMinikube('', fakeMinikubeInstaller);
expect(result).toEqual('minikube');

// expect not called getAssetInfo
expect(fakeMinikubeInstaller.getAssetInfo).not.toBeCalled();

expect(spawnSpy).toBeCalled();
expect(execMock).toBeCalled();
// expect right parameters
if (isMac) {
expect(spawnSpy.mock.calls[0][0]).toEqual('minikube');
} else if (isWindows) {
expect(spawnSpy.mock.calls[0][0]).toEqual('"minikube"');
}
expect(spawnSpy.mock.calls[0][1]).toEqual(['version']);
});

test('runCliCommand/killProcess on macOS', async () => {
vi.mocked(extensionApi.env).isMac = true;
vi.mocked(extensionApi.env).isWindows = false;

// spy on runCliCommand
const spawnSpy = vi.spyOn(childProcess, 'spawn');

const killMock = vi.fn();

spawnSpy.mockReturnValue({
kill: killMock,
on: vi.fn(),
stdout: { setEncoding: vi.fn(), on: vi.fn() },
stderr: { setEncoding: vi.fn(), on: vi.fn() },
} as unknown as childProcess.ChildProcessWithoutNullStreams);

const fakeToken = {
onCancellationRequested: vi.fn(),
} as unknown as extensionApi.CancellationToken;

vi.mocked(fakeToken.onCancellationRequested).mockImplementation((callback: any): extensionApi.Disposable => {
// abort execution after 500ms
setTimeout(() => {
callback();
}, 500);

return extensionApi.Disposable.from({ dispose: vi.fn() });
});

await expect(runCliCommand('fooCommand', [], undefined, fakeToken)).rejects.toThrow('Execution cancelled');

expect(spawnSpy.mock.calls[0][0]).toEqual('fooCommand');

expect(killMock).toBeCalled();
});

test('runCliCommand/killProcess on Windows', async () => {
vi.mocked(extensionApi.env).isMac = false;
vi.mocked(extensionApi.env).isWindows = true;

// spy on runCliCommand
const spawnSpy = vi.spyOn(childProcess, 'spawn');

const killMock = vi.fn();

spawnSpy.mockReturnValue({
kill: killMock,
pid: 'pid123',
on: vi.fn(),
stdout: { setEncoding: vi.fn(), on: vi.fn() },
stderr: { setEncoding: vi.fn(), on: vi.fn() },
} as unknown as childProcess.ChildProcessWithoutNullStreams);

const fakeToken = {
onCancellationRequested: vi.fn(),
} as unknown as extensionApi.CancellationToken;

vi.mocked(fakeToken.onCancellationRequested).mockImplementation((callback: any): extensionApi.Disposable => {
// abort execution after 500ms
setTimeout(() => {
callback();
}, 500);

return extensionApi.Disposable.from({ dispose: vi.fn() });
});

await expect(runCliCommand('fooCommand', [], undefined, fakeToken)).rejects.toThrow('Execution cancelled');

expect(spawnSpy.mock.calls[0][0]).toEqual('"fooCommand"');
// on windows we don't use killProcess but run taskkill
expect(killMock).not.toBeCalled();

expect(spawnSpy.mock.calls[1]).toEqual(['taskkill', ['/pid', 'pid123', '/f', '/t']]);
expect(execMock.mock.calls[0][0]).toEqual('minikube');
expect(execMock.mock.calls[0][1]).toEqual(['version']);
});

test('error: expect installBinaryToSystem to fail with a non existing binary', async () => {
Expand Down
Loading

0 comments on commit 90c3701

Please sign in to comment.