From 17fff23008172077719de225f018afb5f4df8880 Mon Sep 17 00:00:00 2001 From: Charlie Drage Date: Fri, 11 Aug 2023 11:44:19 -0400 Subject: [PATCH] fix: only restart if a machine is running ### What does this PR do? No matter what, it would try to restart regardless if a machine was running or not. This is because the if statement was: `if (isRunning !== '')` which will always return true even if isRunning was undefined. The if statement has now been changes to `if (isRunning)` meaning it will only run if isRunning is defined (not ''). ### Screenshot/screencast of this PR N/A. The prompt will not appear when trying to restart it if there is no machine running. ### What issues does this PR fix or reference? Closes https://github.com/containers/podman-desktop/issues/3321 ### How to test this PR? Click on Docker Compatibility with a Podman Machine stopped. You should not get the prompt to restart the podman machine. Signed-off-by: Charlie Drage --- .../podman/src/compatibility-mode.spec.ts | 62 +++++++++++++++---- extensions/podman/src/compatibility-mode.ts | 5 +- extensions/podman/src/extension.ts | 2 +- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/extensions/podman/src/compatibility-mode.spec.ts b/extensions/podman/src/compatibility-mode.spec.ts index 8429a56bb22d0..b2cda7b9f7824 100644 --- a/extensions/podman/src/compatibility-mode.spec.ts +++ b/extensions/podman/src/compatibility-mode.spec.ts @@ -23,6 +23,7 @@ import { spawn } from 'node:child_process'; import { getSocketCompatibility, DarwinSocketCompatibility, LinuxSocketCompatibility } from './compatibility-mode'; import * as extensionApi from '@podman-desktop/api'; import type { Readable } from 'node:stream'; +import * as extension from './extension'; vi.mock('@podman-desktop/api', () => { return { @@ -34,7 +35,6 @@ vi.mock('@podman-desktop/api', () => { }); // macOS tests - vi.mock('runSudoMacHelperCommand', () => { return vi.fn(); }); @@ -44,16 +44,6 @@ afterEach(() => { vi.restoreAllMocks(); }); -// mock isDefaultMachineRunning from extension to always return true -// this is to prevent execPromise to be ran within it and cause errors -vi.mock('./extension', () => { - return { - findRunningMachine: () => { - return 'default'; - }, - }; -}); - test('darwin: compatibility mode binary not found failure', async () => { // Mock platform to be darwin Object.defineProperty(process, 'platform', { @@ -108,6 +98,11 @@ test('darwin: DarwinSocketCompatibility class, test promptRestart ran within run return Promise.resolve(); }); + const spyFindRunningMachine = vi.spyOn(extension, 'findRunningMachine'); + spyFindRunningMachine.mockImplementation(() => { + return Promise.resolve('default'); + }); + // Mock that enable ran successfully const spyEnable = vi.spyOn(socketCompatClass, 'runCommand'); spyEnable.mockImplementation(() => { @@ -215,3 +210,48 @@ test('windows: compatibility mode fail', async () => { // Expect getSocketCompatibility to return error since Linux is not supported yet expect(() => getSocketCompatibility()).toThrowError(); }); + +test('darwin: test promptRestart IS NOT ran when findRunningMachine returns undefined for enable AND disable', async () => { + // Mock platform to be darwin + Object.defineProperty(process, 'platform', { + value: 'darwin', + }); + + const socketCompatClass = new DarwinSocketCompatibility(); + + // Mock execPromise was ran successfully + vi.mock('execPromise', () => { + return Promise.resolve(); + }); + + // Mock that enable ran successfully + const spyEnable = vi.spyOn(socketCompatClass, 'runCommand'); + spyEnable.mockImplementation(() => { + return Promise.resolve(); + }); + + // Mock that disable ran successfully + const spyDisable = vi.spyOn(socketCompatClass, 'runCommand'); + spyDisable.mockImplementation(() => { + return Promise.resolve(); + }); + + // Mock that findRunningMachine returns undefined + vi.mock('./extension', () => { + return { + findRunningMachine: () => { + return Promise.resolve(undefined); + }, + }; + }); + + const spyPromptRestart = vi.spyOn(socketCompatClass, 'promptRestart'); + + // Enable shouldn't call promptRestart + await socketCompatClass.enable(); + expect(spyPromptRestart).not.toHaveBeenCalled(); + + // Disable shouldn't call promptRestart + await socketCompatClass.disable(); + expect(spyPromptRestart).not.toHaveBeenCalled(); +}); diff --git a/extensions/podman/src/compatibility-mode.ts b/extensions/podman/src/compatibility-mode.ts index 9ff341d19dbf4..8c55dbb653706 100644 --- a/extensions/podman/src/compatibility-mode.ts +++ b/extensions/podman/src/compatibility-mode.ts @@ -138,7 +138,8 @@ export class DarwinSocketCompatibility extends SocketCompatibility { // Prompt the user to restart the podman machine if it's running const isRunning = await findRunningMachine(); - if (isRunning !== '') { + console.log('enable: ', isRunning); + if (isRunning) { await this.promptRestart(isRunning); } @@ -150,7 +151,7 @@ export class DarwinSocketCompatibility extends SocketCompatibility { // Prompt the user to restart the podman machine if it's running const isRunning = await findRunningMachine(); - if (isRunning !== '') { + if (isRunning) { await this.promptRestart(isRunning); } diff --git a/extensions/podman/src/extension.ts b/extensions/podman/src/extension.ts index f9f0cb76248d7..d806cce627d88 100644 --- a/extensions/podman/src/extension.ts +++ b/extensions/podman/src/extension.ts @@ -979,7 +979,7 @@ export async function activate(extensionContext: extensionApi.ExtensionContext): await podmanConfiguration.init(); } -// Function that checks to see if the default machine is running and return a boolean +// Function that checks to see if the default machine is running and return a string export async function findRunningMachine(): Promise { let runningMachine: string;