From b835a88a0b5070a56f8783a0b5cf486a86e00544 Mon Sep 17 00:00:00 2001 From: Mark Yen Date: Thu, 29 Jun 2023 15:53:51 -0700 Subject: [PATCH] E2E: Address review comments Signed-off-by: Mark Yen --- e2e/credentials-server.e2e.spec.ts | 2 +- e2e/lockedFields.e2e.spec.ts | 3 +-- e2e/preferences.e2e.spec.ts | 3 +-- e2e/utils/TestUtils.ts | 19 +++++++++++-------- pkg/rancher-desktop/backend/mock.ts | 26 +++++++++++--------------- pkg/rancher-desktop/utils/logging.ts | 11 +++++++++-- 6 files changed, 34 insertions(+), 30 deletions(-) diff --git a/e2e/credentials-server.e2e.spec.ts b/e2e/credentials-server.e2e.spec.ts index ab15eb94c25..57e36d90409 100644 --- a/e2e/credentials-server.e2e.spec.ts +++ b/e2e/credentials-server.e2e.spec.ts @@ -224,7 +224,7 @@ describeWithCreds('Credentials server', () => { test.describe.configure({ mode: 'serial' }); test.beforeAll(async() => { - await tool('rdctl', 'factory-reset'); + await tool('rdctl', 'factory-reset', '--verbose'); createDefaultSettings({ kubernetes: { enabled: false } }); electronApp = await startRancherDesktop(__filename, { mock: false }); page = await electronApp.firstWindow(); diff --git a/e2e/lockedFields.e2e.spec.ts b/e2e/lockedFields.e2e.spec.ts index 7e8cbff3bbf..89676e5d508 100644 --- a/e2e/lockedFields.e2e.spec.ts +++ b/e2e/lockedFields.e2e.spec.ts @@ -97,9 +97,8 @@ test.describe('Locked fields', () => { page = await electronApp.firstWindow(); }); - test.afterAll(() => teardown(electronApp, __filename)); - test.afterAll(async() => { + await teardown(electronApp, __filename); await tool('rdctl', 'factory-reset', '--verbose'); reopenLogs(); }); diff --git a/e2e/preferences.e2e.spec.ts b/e2e/preferences.e2e.spec.ts index 0622516070a..32215bbc03e 100644 --- a/e2e/preferences.e2e.spec.ts +++ b/e2e/preferences.e2e.spec.ts @@ -30,9 +30,8 @@ test.describe.serial('Main App Test', () => { preferencesWindow = await electronApp.waitForEvent('window', page => /preferences/i.test(page.url())); }); - test.afterAll(() => teardown(electronApp, __filename)); - test.afterAll(async() => { + await teardown(electronApp, __filename); await tool('rdctl', 'factory-reset', '--verbose'); reopenLogs(); }); diff --git a/e2e/utils/TestUtils.ts b/e2e/utils/TestUtils.ts index aa4d0e7b107..8f8d2a004e3 100644 --- a/e2e/utils/TestUtils.ts +++ b/e2e/utils/TestUtils.ts @@ -163,27 +163,30 @@ export async function teardownApp(app: ElectronApplication) { } // Try to do platform-specific killing based on process groups if (process.platform === 'darwin' || process.platform === 'linux') { - for (const signal of ['TERM', 'TERM', 'TERM', 'KILL']) { - let pids = ''; + // Send SIGTERM to the process group, wait three seconds, then send + // SIGKILL and wait for one more second. + for (const [signal, timeout] of [['TERM', 3_000], ['KILL', 1_000]] as const) { + let pids: string[] = []; try { const args = ['-o', 'pid=', process.platform === 'darwin' ? '-g' : '--sid', `${ pid }`]; + const { stdout } = await childProcess.spawnFile('ps', args, { stdio: ['ignore', 'pipe', 'inherit'] }); - pids = (await childProcess.spawnFile('ps', args, { stdio: ['ignore', 'pipe', 'inherit'] })).stdout; + pids = stdout.trim().split(/\s+/); } catch (ex) { - console.log(`Did not find processes in process group, ignoring.`); + console.log(`Did not find processes in process group ${ pid }, ignoring.`); break; } try { - if (pids.trim()) { - console.log(`Manually killing group processes ${ pids.replace(/\r?\n/g, ' ').trim() }`); - await childProcess.spawnFile('kill', ['-s', signal].concat(...pids.split(/\s+/).filter(p => p))); + if (pids.length > 0) { + console.log(`Manually killing group processes ${ pids.join(' ') }`); + await childProcess.spawnFile('kill', ['-s', signal, ...pids]); } } catch (ex) { console.log(`Failed to process group: ${ ex } (retrying)`); } - await util.promisify(setTimeout)(1_000); + await util.promisify(setTimeout)(timeout); } } } diff --git a/pkg/rancher-desktop/backend/mock.ts b/pkg/rancher-desktop/backend/mock.ts index 0e748f1d66a..ccab40972b7 100644 --- a/pkg/rancher-desktop/backend/mock.ts +++ b/pkg/rancher-desktop/backend/mock.ts @@ -256,27 +256,23 @@ class MockContainerEngineClient implements ContainerEngineClient { return Promise.resolve(); } - readFile(imageID: string, filePath: string): Promise; - readFile(imageID: string, filePath: string, options: { encoding?: BufferEncoding | undefined; namespace?: string | undefined; }): Promise; - readFile(imageID: string, filePath: string, options?: unknown): Promise { + readFile(imageID: string, filePath: string, options?: { encoding?: BufferEncoding; namespace?: string; }): Promise { throw new Error('Method not implemented.'); } - copyFile(imageID: string, sourcePath: string, destinationDir: string): Promise; - copyFile(imageID: string, sourcePath: string, destinationDir: string, options: { namespace?: string | undefined; }): Promise; - copyFile(imageID: unknown, sourcePath: unknown, destinationDir: unknown, options?: unknown): Promise { + copyFile(imageID: string, sourcePath: string, destinationDir: string, options?: { namespace?: string; }): Promise { throw new Error('Method not implemented.'); } - getTags(imageName: string, options?: ContainerBasicOptions | undefined): Promise> { + getTags(imageName: string, options?: ContainerBasicOptions): Promise> { throw new Error('Method not implemented.'); } - run(imageID: string, options?: ContainerRunOptions | undefined): Promise { + run(imageID: string, options?: ContainerRunOptions): Promise { throw new Error('Method not implemented.'); } - stop(container: string, options?: ContainerStopOptions | undefined): Promise { + stop(container: string, options?: ContainerStopOptions): Promise { throw new Error('Method not implemented.'); } @@ -284,7 +280,7 @@ class MockContainerEngineClient implements ContainerEngineClient { throw new Error('Method not implemented.'); } - composeDown(options?: ContainerComposeOptions | undefined): Promise { + composeDown(options?: ContainerComposeOptions): Promise { throw new Error('Method not implemented.'); } @@ -296,11 +292,11 @@ class MockContainerEngineClient implements ContainerEngineClient { throw new Error('Method not implemented.'); } - runClient(args: string[], stdio?: 'ignore' | undefined, options?: ContainerRunClientOptions | undefined): Promise>; - runClient(args: string[], stdio: Log, options?: ContainerRunClientOptions | undefined): Promise>; - runClient(args: string[], stdio: 'pipe', options?: ContainerRunClientOptions | undefined): Promise<{ stdout: string; stderr: string; }>; - runClient(args: string[], stdio: 'stream', options?: ContainerRunClientOptions | undefined): ReadableProcess; - runClient(args: unknown, stdio?: unknown, options?: unknown): import('./containerClient').ReadableProcess | Promise> | Promise<{ stdout: string; stderr: string; }> { + runClient(args: string[], stdio?: 'ignore', options?: ContainerRunClientOptions): Promise>; + runClient(args: string[], stdio: Log, options?: ContainerRunClientOptions): Promise>; + runClient(args: string[], stdio: 'pipe', options?: ContainerRunClientOptions): Promise<{ stdout: string; stderr: string; }>; + runClient(args: string[], stdio: 'stream', options?: ContainerRunClientOptions): ReadableProcess; + runClient(args: string[], stdio?: unknown, options?: ContainerRunClientOptions): unknown { return Promise.resolve({ stdout: '', stderr: '' }); } } diff --git a/pkg/rancher-desktop/utils/logging.ts b/pkg/rancher-desktop/utils/logging.ts index d5dbf884ff6..e7e02715377 100644 --- a/pkg/rancher-desktop/utils/logging.ts +++ b/pkg/rancher-desktop/utils/logging.ts @@ -59,6 +59,12 @@ export class Log { protected realStream: fs.WriteStream; + /** + * Reopen the logs; this is necessary after a factory reset because the files + * would have been deleted from under us (so reopening ensures any new logs + * are readable). + * @note This is only used during E2E tests where we do a factory reset. + */ protected reopen(mode = 'w') { if (process.env.RD_TEST === 'e2e') { // If we're running E2E tests, we may need to create the log directory. @@ -208,8 +214,9 @@ export function clearLoggingDirectory(): void { export function reopenLogs() { for (const log of logs.values()) { log['reopen']('a'); - // Trigger making the stream - ((_: any) => {})(log.fdStream); + // Trigger making the stream (by passing it to `Array.of()` and ignoring the + // result). + Array.of(log.fdStream); } }