Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backend code for quitOnClose option #3843

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
4 changes: 4 additions & 0 deletions background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,10 @@ Electron.app.on('before-quit', async(event) => {
});

Electron.app.on('window-all-closed', () => {
if (cfg.window.quitOnClose) {
Electron.app.quit();
}

// On macOS, hide the dock icon.
Electron.app.dock?.hide();
});
Expand Down
59 changes: 59 additions & 0 deletions e2e/quit-on-close.e2e.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import path from 'path';

import { test, expect, _electron, ElectronApplication } from '@playwright/test';

import { createDefaultSettings, packageLogs, startRancherDesktop } from './utils/TestUtils';

/**
* Using test.describe.serial make the test execute step by step, as described on each `test()` order
* Playwright executes test in parallel by default and it will not work for our app backend loading process.
* */
test.describe.serial('quitOnClose setting', () => {
test.afterAll(async() => {
await packageLogs(__filename);
adamkpickering marked this conversation as resolved.
Show resolved Hide resolved
});

test('should quit when quitOnClose is true and window is closed', async() => {
createDefaultSettings({ window: { quitOnClose: true } });
const { electronApp } = await startRancherDesktop(__filename, false);

await expect(closeWindowsAndCheckQuit(electronApp)).resolves.toBe(true);
});

test('should not quit when quitOnClose is false and window is closed', async() => {
createDefaultSettings({ window: { quitOnClose: false } });
const { electronApp } = await startRancherDesktop(__filename, true);

await expect(closeWindowsAndCheckQuit(electronApp)).resolves.toBe(false);
const tracePath = path.join(__dirname, 'reports', `${ path.basename(__filename) }-quitOnCloseFalse.zip`);

electronApp.context().tracing.stop({ path: tracePath });
await electronApp.close();
});
});

/**
* Closes all of the windows in a running app. Returns a promise that
* resolves to true when the app has quit within a certain period of time,
* or that resolves to false when the app does not quit within that period
* of time.
* */
function closeWindowsAndCheckQuit(electronApp: ElectronApplication): Promise<boolean> {
return electronApp.evaluate(async({ app, BrowserWindow }) => {
const quitReady = new Promise<boolean>((resolve) => {
app.on('will-quit', () => resolve(true));
app.on('window-all-closed', () => {
setTimeout(() => resolve(false), 3_000);
});
});

await Promise.all(BrowserWindow.getAllWindows().map((window) => {
return new Promise<void>((resolve) => {
window.on('closed', resolve);
window.close();
});
}));

return await quitReady;
});
}
36 changes: 34 additions & 2 deletions e2e/utils/TestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import os from 'os';
import path from 'path';
import util from 'util';

import { ElectronApplication, expect } from '@playwright/test';
import { expect, _electron, ElectronApplication, Page } from '@playwright/test';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, ElectronApplication and Page can be changed to an import type, but that's mostly just moving deck chairs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's alright, I'd like to keep it as it is. The following link is convincing to me: microsoft/TypeScript#39861

import _ from 'lodash';

import { defaultSettings, Settings } from '@pkg/config/settings';
Expand Down Expand Up @@ -65,7 +65,7 @@ export function reportAsset(testPath: string, type: 'trace' | 'log' = 'trace') {
return path.join(__dirname, '..', 'reports', `${ path.basename(testPath) }-${ name }`);
}

async function packageLogs(testPath: string) {
export async function packageLogs(testPath: string) {
if (!process.env.CIRRUS_CI) {
console.log('Skipping packaging logs, not running in Cirrus CI');

Expand Down Expand Up @@ -171,3 +171,35 @@ export async function retry<T>(proc: () => Promise<T>, options?: { delay?: numbe
}
}
}

/**
* Run Rancher Desktop; return promise that resolves to commonly-used
* playwright objects when it has started.
* @param testPath The path to the test file.
* @param tracing Whether to start tracing.
*/
export async function startRancherDesktop(testPath: string, tracing: boolean): Promise<{electronApp: ElectronApplication, page: Page}> {
const electronApp = await _electron.launch({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we call await electronApp.context.tracing.start({ screenshots: true, snapshots: true }); somewhere in this function?

args: [
path.join(__dirname, '../../'),
'--disable-gpu',
'--whitelisted-ips=',
// See pkg/rancher-desktop/utils/commandLine.ts before changing the next item as the final option.
'--disable-dev-shm-usage',
'--no-modal-dialogs',
],
env: {
...process.env,
RD_LOGS_DIR: reportAsset(testPath, 'log'),
RD_MOCK_BACKEND: '1',
},
});

if (tracing) {
electronApp.context().tracing.start({ screenshots: true, snapshots: true });
}

const page = await electronApp.firstWindow();

return { electronApp, page };
}