Skip to content

Commit

Permalink
fix: use process.env when running fiddle
Browse files Browse the repository at this point in the history
  • Loading branch information
dsanders11 committed Sep 28, 2023
1 parent 1e40d19 commit baefcac
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 25 deletions.
1 change: 1 addition & 0 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ export interface PackageJsonOptions {

export interface StartFiddleParams {
localPath: string | undefined;
enableElectronLogging: boolean;
isValidBuild: boolean; // If the localPath is a valid Electron build
version: string; // The user selected version
dir: string;
Expand Down
21 changes: 20 additions & 1 deletion src/main/fiddle-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,26 @@ export async function startFiddle(
webContents: WebContents,
params: StartFiddleParams,
): Promise<void> {
const { dir, env, isValidBuild, localPath, options, version } = params;
const {
dir,
enableElectronLogging,
isValidBuild,
localPath,
options,
version,
} = params;
const env = { ...process.env, ...params.env };

if (enableElectronLogging) {
env.ELECTRON_ENABLE_LOGGING = 'true';
env.ELECTRON_DEBUG_NOTIFICATIONS = 'true';
env.ELECTRON_ENABLE_STACK_DUMPING = 'true';
} else {
delete env.ELECTRON_ENABLE_LOGGING;
delete env.ELECTRON_DEBUG_NOTIFICATIONS;
delete env.ELECTRON_ENABLE_STACK_DUMPING;
}

const child = await runner.spawn(
isValidBuild && localPath ? Installer.getExecPath(localPath) : version,
dir,
Expand Down
17 changes: 4 additions & 13 deletions src/renderer/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,19 +331,9 @@ export class Runner {
}

private buildChildEnvVars(): { [x: string]: string | undefined } {
const { isEnablingElectronLogging, environmentVariables } = this.appState;

const env = { ...process.env };

if (isEnablingElectronLogging) {
env.ELECTRON_ENABLE_LOGGING = 'true';
env.ELECTRON_DEBUG_NOTIFICATIONS = 'true';
env.ELECTRON_ENABLE_STACK_DUMPING = 'true';
} else {
delete env.ELECTRON_ENABLE_LOGGING;
delete env.ELECTRON_DEBUG_NOTIFICATIONS;
delete env.ELECTRON_ENABLE_STACK_DUMPING;
}
const { environmentVariables } = this.appState;

const env: Record<string, string> = {};

for (const v of environmentVariables) {
const [key, value] = v.split('=');
Expand Down Expand Up @@ -379,6 +369,7 @@ export class Runner {
try {
await window.ElectronFiddle.startFiddle({
...params,
enableElectronLogging: this.appState.isEnablingElectronLogging,
options,
env,
});
Expand Down
62 changes: 62 additions & 0 deletions tests/main/fiddle-core-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* @jest-environment node
*/

import { ElectronVersions, Runner } from '@electron/fiddle-core';
import { WebContents } from 'electron';
import { mocked } from 'jest-mock';

import { setupFiddleCore, startFiddle } from '../../src/main/fiddle-core';
import { ChildProcessMock } from '../mocks/child-process';
import { ElectronVersionsMock, FiddleRunnerMock } from '../mocks/fiddle-core';
import { WebContentsMock } from '../mocks/web-contents';

jest.mock('@electron/fiddle-core', () => {
const { FiddleRunnerMock, InstallerMock } = require('../mocks/fiddle-core');

return {
Installer: InstallerMock,
Runner: FiddleRunnerMock,
};
});

describe('fiddle-core', () => {
let runner: FiddleRunnerMock;

beforeEach(() => {
runner = new FiddleRunnerMock();
mocked(Runner.create).mockResolvedValue(runner as unknown as Runner);
setupFiddleCore(new ElectronVersionsMock() as unknown as ElectronVersions);
});

describe('startFiddle', () => {
it('runs with logging when enabled', async () => {
const child = new ChildProcessMock();
mocked(runner.spawn).mockResolvedValue(child);

const version = '18.0.0';

await startFiddle(new WebContentsMock() as unknown as WebContents, {
dir: '/path/to/fiddle',
enableElectronLogging: true,
env: {},
isValidBuild: true,
localPath: undefined,
options: [],
version,
});

expect(runner.spawn).toHaveBeenCalledWith(
version,
expect.anything(),
expect.objectContaining({
env: expect.objectContaining({
ELECTRON_DEBUG_NOTIFICATIONS: 'true',
ELECTRON_ENABLE_LOGGING: 'true',
ELECTRON_ENABLE_STACK_DUMPING: 'true',
}),
}),
);
});
});
});
10 changes: 4 additions & 6 deletions tests/mocks/fiddle-core.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import { Installer } from '@electron/fiddle-core';
import { EventEmitter } from 'node:events';

import { ChildProcessMock } from './child-process';
import { InstallState } from '../../src/interfaces';

export class InstallerMock extends Installer {
export class InstallerMock extends EventEmitter {
public state = () => InstallState.installed;
}

export class FiddleRunnerMock {
public child = new ChildProcessMock();
public spawn = (): ChildProcessMock => {
return this.child;
};
public spawn: () => Promise<ChildProcessMock> = jest.fn();
public static create = jest.fn();
}

export class ElectronVersionsMock {
Expand Down
10 changes: 5 additions & 5 deletions tests/renderer/runner-spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ describe('Runner component', () => {

it('runs with logging when enabled', async () => {
store.isEnablingElectronLogging = true;
const spy = jest.spyOn(window.ElectronFiddle, 'startFiddle');
spy.mockImplementationOnce(async (params) => {
expect(params.env).toHaveProperty('ELECTRON_ENABLE_LOGGING');
expect(params.env).toHaveProperty('ELECTRON_ENABLE_STACK_DUMPING');
});

// wait for run() to get running
const runPromise = instance.run();
Expand All @@ -79,6 +74,11 @@ describe('Runner component', () => {
expect(store.isRunning).toBe(false);
expect(fileManager.saveToTemp).toHaveBeenCalled();
expect(window.ElectronFiddle.addModules).toHaveBeenCalled();
expect(window.ElectronFiddle.startFiddle).toBeCalledWith(
expect.objectContaining({
enableElectronLogging: true,
}),
);
});

it('emits output with exitCode', async () => {
Expand Down

0 comments on commit baefcac

Please sign in to comment.