Skip to content

Commit

Permalink
fix: only restart if a machine is running
Browse files Browse the repository at this point in the history
### 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

<!-- Please include a screenshot or a screencast
explaining what is doing 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?

<!-- Include any related issues from Podman Desktop
repository (or from another issue tracker). -->

Closes #3321

### How to test this PR?

<!-- Please explain steps to reproduce -->

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 <charlie@charliedrage.com>
  • Loading branch information
cdrage committed Aug 11, 2023
1 parent f75dc3f commit 17fff23
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 14 deletions.
62 changes: 51 additions & 11 deletions extensions/podman/src/compatibility-mode.spec.ts
Expand Up @@ -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 {
Expand All @@ -34,7 +35,6 @@ vi.mock('@podman-desktop/api', () => {
});

// macOS tests

vi.mock('runSudoMacHelperCommand', () => {
return vi.fn();
});
Expand All @@ -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', {
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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();
});
5 changes: 3 additions & 2 deletions extensions/podman/src/compatibility-mode.ts
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion extensions/podman/src/extension.ts
Expand Up @@ -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<string> {
let runningMachine: string;

Expand Down

0 comments on commit 17fff23

Please sign in to comment.