Skip to content

Commit

Permalink
fix: dispose the wsl2 command when unregistering extension
Browse files Browse the repository at this point in the history
fixes #4593
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
  • Loading branch information
benoitf committed Dec 13, 2023
1 parent 17d932e commit 856cbce
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 34 deletions.
3 changes: 2 additions & 1 deletion extensions/podman/src/extension.spec.ts
Expand Up @@ -951,7 +951,8 @@ test('ensure showNotification is not called during update', async () => {
}),
);

const podmanInstall: PodmanInstall = new PodmanInstall('');
const extensionContext = { subscriptions: [], storagePath: '' } as extensionApi.ExtensionContext;
const podmanInstall: PodmanInstall = new PodmanInstall(extensionContext);
vi.spyOn(podmanInstall, 'checkForUpdate').mockImplementation((_installedPodman: InstalledPodman) => {
return Promise.resolve({
hasUpdate: true,
Expand Down
2 changes: 1 addition & 1 deletion extensions/podman/src/extension.ts
Expand Up @@ -807,7 +807,7 @@ export async function activate(extensionContext: extensionApi.ExtensionContext):

initTelemetryLogger();

const podmanInstall = new PodmanInstall(extensionContext.storagePath);
const podmanInstall = new PodmanInstall(extensionContext);

const installedPodman = await getPodmanInstallation();
const version: string | undefined = installedPodman?.version;
Expand Down
79 changes: 51 additions & 28 deletions extensions/podman/src/podman-install.spec.ts
Expand Up @@ -25,6 +25,10 @@ import * as os from 'node:os';
const originalConsoleError = console.error;
const consoleErrorMock = vi.fn();

const extensionContext = {
subscriptions: [],
} as unknown as extensionApi.ExtensionContext;

// mock release
vi.mock('node:os', async () => {
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
Expand Down Expand Up @@ -69,6 +73,8 @@ const progress = {

beforeEach(() => {
vi.clearAllMocks();
// reset array of subscriptions
extensionContext.subscriptions.length = 0;
console.error = consoleErrorMock;
});

Expand All @@ -91,7 +97,7 @@ test('expect update on windows to show notification in case of 0 exit code', asy
vi.mock('node:fs');
vi.spyOn(fs, 'existsSync').mockReturnValue(true);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const result = await installer.update();
expect(result).toBeTruthy();
expect(extensionApi.window.showNotification).toHaveBeenCalled();
Expand All @@ -112,7 +118,7 @@ test('expect update on windows not to show notification in case of 1602 exit cod
vi.mock('node:fs');
vi.spyOn(fs, 'existsSync').mockReturnValue(true);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const result = await installer.update();
expect(result).toBeTruthy();
expect(extensionApi.window.showNotification).not.toHaveBeenCalled();
Expand All @@ -134,7 +140,7 @@ test('expect update on windows to throw error if non zero exit code', async () =
vi.mock('node:fs');
vi.spyOn(fs, 'existsSync').mockReturnValue(true);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const result = await installer.update();
expect(result).toBeFalsy();
expect(extensionApi.window.showErrorMessage).toHaveBeenCalled();
Expand All @@ -145,7 +151,7 @@ test('expect winbit preflight check return successful result if the arch is x64'
value: 'x64',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winBitCheck = preflights[0];
const result = await winBitCheck.execute();
Expand All @@ -157,7 +163,7 @@ test('expect winbit preflight check return successful result if the arch is arm6
value: 'arm64',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winBitCheck = preflights[0];
const result = await winBitCheck.execute();
Expand All @@ -169,7 +175,7 @@ test('expect winbit preflight check return failure result if the arch is not sup
value: 'x86',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winBitCheck = preflights[0];
const result = await winBitCheck.execute();
Expand All @@ -184,7 +190,7 @@ test('expect winbit preflight check return failure result if the arch is not sup
test('expect winversion preflight check return successful result if the version is greater than min valid version', async () => {
vi.spyOn(os, 'release').mockReturnValue('10.0.19043');

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winVersionCheck = preflights[1];
const result = await winVersionCheck.execute();
Expand All @@ -194,7 +200,7 @@ test('expect winversion preflight check return successful result if the version
test('expect winversion preflight check return failure result if the version is greater than 9. and less than min build version', async () => {
vi.spyOn(os, 'release').mockReturnValue('10.0.19042');

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winVersionCheck = preflights[1];
const result = await winVersionCheck.execute();
Expand All @@ -209,7 +215,7 @@ test('expect winversion preflight check return failure result if the version is
test('expect winversion preflight check return failure result if the version is less than 10.0.0', async () => {
vi.spyOn(os, 'release').mockReturnValue('9.0.19000');

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winVersionCheck = preflights[1];
const result = await winVersionCheck.execute();
Expand All @@ -225,7 +231,7 @@ test('expect winMemory preflight check return successful result if the machine h
const SYSTEM_MEM = 7 * 1024 * 1024 * 1024;
vi.spyOn(os, 'totalmem').mockReturnValue(SYSTEM_MEM);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winMemoryCheck = preflights[2];
const result = await winMemoryCheck.execute();
Expand All @@ -236,7 +242,7 @@ test('expect winMemory preflight check return failure result if the machine has
const SYSTEM_MEM = 4 * 1024 * 1024 * 1024;
vi.spyOn(os, 'totalmem').mockReturnValue(SYSTEM_MEM);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winMemoryCheck = preflights[2];
const result = await winMemoryCheck.execute();
Expand All @@ -259,7 +265,7 @@ test('expect winVirtualMachine preflight check return successful result if the v
}),
);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winVirtualMachinePlatformCheck = preflights[3];
const result = await winVirtualMachinePlatformCheck.execute();
Expand All @@ -279,7 +285,7 @@ test('expect winVirtualMachine preflight check return successful result if the v
}),
);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winVirtualMachinePlatformCheck = preflights[3];
const result = await winVirtualMachinePlatformCheck.execute();
Expand All @@ -300,7 +306,7 @@ test('expect winVirtualMachine preflight check return successful result if there
}),
);

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winVirtualMachinePlatformCheck = preflights[3];
const result = await winVirtualMachinePlatformCheck.execute();
Expand All @@ -315,7 +321,7 @@ test('expect winVirtualMachine preflight check return successful result if there
test('expect WSLVersion preflight check return fail result if wsl --version command fails its execution', async () => {
vi.spyOn(extensionApi.process, 'exec').mockRejectedValue('');

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[4];
const result = await winWSLCheck.execute();
Expand All @@ -330,7 +336,7 @@ test('expect WSLVersion preflight check return fail result if first line output
command: 'command',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[4];
const result = await winWSLCheck.execute();
Expand All @@ -345,7 +351,7 @@ test('expect WSLVersion preflight check return fail result if first line output
command: 'command',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[4];
const result = await winWSLCheck.execute();
Expand All @@ -360,7 +366,7 @@ test('expect WSLVersion preflight check return fail result if first line output
command: 'command',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[4];
const result = await winWSLCheck.execute();
Expand All @@ -377,7 +383,7 @@ test('expect WSLVersion preflight check return fail result if first line output
command: 'command',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[4];
const result = await winWSLCheck.execute();
Expand All @@ -391,7 +397,7 @@ test('expect WSLVersion preflight check return fail result if first line output
command: 'command',
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[4];
const result = await winWSLCheck.execute();
Expand Down Expand Up @@ -421,7 +427,7 @@ test('expect winWSL2 preflight check return successful result if the machine has
}
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
const result = await winWSLCheck.execute();
Expand Down Expand Up @@ -461,7 +467,7 @@ test('expect winWSL2 preflight check return failure result if the machine has WS
}
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
const result = await winWSLCheck.execute();
Expand Down Expand Up @@ -503,7 +509,7 @@ test('expect winWSL2 preflight check return successful result if the machine has
}
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
const result = await winWSLCheck.execute();
Expand Down Expand Up @@ -533,7 +539,7 @@ test('expect winWSL2 preflight check return failure result if user do not have w
}
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
const result = await winWSLCheck.execute();
Expand Down Expand Up @@ -566,7 +572,7 @@ test('expect winWSL2 preflight check return failure result if user do not have w
}
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
const result = await winWSLCheck.execute();
Expand Down Expand Up @@ -595,7 +601,7 @@ test('expect winWSL2 preflight check return failure result if it fails when chec
}
});

const installer = new WinInstaller();
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
const result = await winWSLCheck.execute();
Expand All @@ -604,11 +610,28 @@ test('expect winWSL2 preflight check return failure result if it fails when chec
expect(result.docLinks[0].title).equal('WSL2 Manual Installation Steps');
});

test('expect winWSL2 init to rgister WSLInstall command', async () => {
test('expect winWSL2 init to register WSLInstall command', async () => {
const registerCommandMock = vi.spyOn(extensionApi.commands, 'registerCommand');
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
await winWSLCheck.init();
expect(registerCommandMock).toBeCalledWith('podman.onboarding.installWSL', expect.any(Function));
});

test('expect winWSL2 command to be registered as disposable', async () => {
const registerCommandMock = vi.spyOn(extensionApi.commands, 'registerCommand');
const installer = new WinInstaller();
const disposableMock = {
dispose: vi.fn(),
};
registerCommandMock.mockReturnValue(disposableMock);
const installer = new WinInstaller(extensionContext);
const preflights = installer.getPreflightChecks();
const winWSLCheck = preflights[5];
await winWSLCheck.init();
expect(registerCommandMock).toBeCalledWith('podman.onboarding.installWSL', expect.any(Function));

// should contain a subscription with a disposable function
expect(extensionContext.subscriptions[0]).toBeDefined();
expect(extensionContext.subscriptions[0].dispose).toBeDefined();
});
20 changes: 16 additions & 4 deletions extensions/podman/src/podman-install.ts
Expand Up @@ -123,8 +123,11 @@ export class PodmanInstall {

private installers = new Map<NodeJS.Platform, Installer>();

constructor(private readonly storagePath: string) {
this.installers.set('win32', new WinInstaller());
private readonly storagePath: string;

constructor(readonly extensionContext: extensionApi.ExtensionContext) {
this.storagePath = extensionContext.storagePath;
this.installers.set('win32', new WinInstaller(extensionContext));
this.installers.set('darwin', new MacOSInstaller());
}

Expand Down Expand Up @@ -284,6 +287,10 @@ abstract class BaseInstaller implements Installer {
}

export class WinInstaller extends BaseInstaller {
constructor(private extensionContext: extensionApi.ExtensionContext) {
super();
}

getUpdatePreflightChecks(): extensionApi.InstallCheck[] {
return [];
}
Expand All @@ -295,7 +302,7 @@ export class WinInstaller extends BaseInstaller {
new WinMemoryCheck(),
new VirtualMachinePlatformCheck(),
new WSLVersionCheck(),
new WSL2Check(),
new WSL2Check(this.extensionContext),
];
}

Expand Down Expand Up @@ -484,14 +491,19 @@ class WSL2Check extends BaseCheck {
title = 'WSL2 Installed';
installWSLCommandId = 'podman.onboarding.installWSL';

constructor(private extensionContext: extensionApi.ExtensionContext) {
super();
}

async init(): Promise<void> {
extensionApi.commands.registerCommand(this.installWSLCommandId, async () => {
const wslCommand = extensionApi.commands.registerCommand(this.installWSLCommandId, async () => {
const installSucceeded = await this.installWSL();
if (installSucceeded) {
// if action succeeded, do a re-check of all podman requirements so user can be moved forward if all missing pieces have been installed
await extensionApi.commands.executeCommand('podman.onboarding.checkRequirementsCommand');
}
});
this.extensionContext.subscriptions.push(wslCommand);
}

async execute(): Promise<extensionApi.CheckResult> {
Expand Down

0 comments on commit 856cbce

Please sign in to comment.