Show save/execution error dialogs on desktop #264
This commit introduces system-native error dialogs on desktop application for code save or execution failures, addressing user confusion described in issue #264. This commit adds informative feedback when script execution or saving fails. Changes: - Implement support for system-native error dialogs. - Refactor `CodeRunner` and `Dialog` interfaces and their implementations to improve error handling and provide better type safety. - Introduce structured error handling, allowing UI to display detailed error messages. - Replace error throwing with an error object interface for controlled handling. This ensures that errors are propagated to the renderer process without being limited by Electron's error object serialization limitations as detailed in electron/electron#24427. - Add logging for dialog actions to aid in troubleshooting. - Rename `fileName` to `defaultFilename` in `saveFile` functions to clarify its purpose. - Centralize message assertion in `LoggerStub` for consistency. - Introduce `expectTrue` in tests for clearer boolean assertions. - Standardize `filename` usage across the codebase. - Enhance existing test names and organization for clarity. - Update related documentation.
This commit is contained in:
undergroundwires
@@ -2,142 +2,178 @@ import { describe, it, expect } from 'vitest';
|
||||
import { ScriptFileCodeRunner } from '@/infrastructure/CodeRunner/ScriptFileCodeRunner';
|
||||
import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub';
|
||||
import { Logger } from '@/application/Common/Log/Logger';
|
||||
import { ScriptFileName } from '@/application/CodeRunner/ScriptFileName';
|
||||
import { ScriptFilename } from '@/application/CodeRunner/ScriptFilename';
|
||||
import { ScriptFileExecutor } from '@/infrastructure/CodeRunner/Execution/ScriptFileExecutor';
|
||||
import { ScriptFileExecutorStub } from '@tests/unit/shared/Stubs/ScriptFileExecutorStub';
|
||||
import { ScriptFileCreator } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator';
|
||||
import { ScriptFileCreatorStub } from '@tests/unit/shared/Stubs/ScriptFileCreatorStub';
|
||||
import { expectExists } from '@tests/shared/Assertions/ExpectExists';
|
||||
import { expectThrowsAsync } from '@tests/shared/Assertions/ExpectThrowsAsync';
|
||||
import { CodeRunErrorType } from '@/application/CodeRunner/CodeRunner';
|
||||
|
||||
describe('ScriptFileCodeRunner', () => {
|
||||
describe('runCode', () => {
|
||||
it('executes script file correctly', async () => {
|
||||
// arrange
|
||||
const expectedFilePath = 'expected script path';
|
||||
const fileExecutor = new ScriptFileExecutorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(new ScriptFileCreatorStub().withCreatedFilePath(expectedFilePath))
|
||||
.withFileExecutor(fileExecutor);
|
||||
describe('creating file', () => {
|
||||
it('uses provided code', async () => {
|
||||
// arrange
|
||||
const expectedCode = 'expected code';
|
||||
const fileCreator = new ScriptFileCreatorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(fileCreator)
|
||||
.withCode(expectedCode);
|
||||
|
||||
// act
|
||||
await context.runCode();
|
||||
// act
|
||||
await context.runCode();
|
||||
|
||||
// assert
|
||||
const executeCalls = fileExecutor.callHistory.filter((call) => call.methodName === 'executeScriptFile');
|
||||
expect(executeCalls.length).to.equal(1);
|
||||
const [actualPath] = executeCalls[0].args;
|
||||
expect(actualPath).to.equal(expectedFilePath);
|
||||
// assert
|
||||
const createCalls = fileCreator.callHistory.filter((call) => call.methodName === 'createScriptFile');
|
||||
expect(createCalls.length).to.equal(1);
|
||||
const [actualCode] = createCalls[0].args;
|
||||
expect(actualCode).to.equal(expectedCode);
|
||||
});
|
||||
it('uses provided extension', async () => {
|
||||
// arrange
|
||||
const expectedFileExtension = 'expected-file-extension';
|
||||
const fileCreator = new ScriptFileCreatorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(fileCreator)
|
||||
.withFileExtension(expectedFileExtension);
|
||||
|
||||
// act
|
||||
await context.runCode();
|
||||
|
||||
// assert
|
||||
const createCalls = fileCreator.callHistory.filter((call) => call.methodName === 'createScriptFile');
|
||||
expect(createCalls.length).to.equal(1);
|
||||
const [,scriptFileNameParts] = createCalls[0].args;
|
||||
expectExists(scriptFileNameParts, JSON.stringify(`Call args: ${JSON.stringify(createCalls[0].args)}`));
|
||||
expect(scriptFileNameParts.scriptFileExtension).to.equal(expectedFileExtension);
|
||||
});
|
||||
it('uses default script name', async () => {
|
||||
// arrange
|
||||
const expectedScriptName = ScriptFilename;
|
||||
const fileCreator = new ScriptFileCreatorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(fileCreator);
|
||||
|
||||
// act
|
||||
await context.runCode();
|
||||
|
||||
// assert
|
||||
const createCalls = fileCreator.callHistory.filter((call) => call.methodName === 'createScriptFile');
|
||||
expect(createCalls.length).to.equal(1);
|
||||
const [,scriptFileNameParts] = createCalls[0].args;
|
||||
expectExists(scriptFileNameParts, JSON.stringify(`Call args: ${JSON.stringify(createCalls[0].args)}`));
|
||||
expect(scriptFileNameParts.scriptName).to.equal(expectedScriptName);
|
||||
});
|
||||
});
|
||||
it('creates script file with provided code', async () => {
|
||||
// arrange
|
||||
const expectedCode = 'expected code';
|
||||
const fileCreator = new ScriptFileCreatorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(fileCreator)
|
||||
.withCode(expectedCode);
|
||||
describe('executing file', () => {
|
||||
it('executes at correct path', async () => {
|
||||
// arrange
|
||||
const expectedFilePath = 'expected script path';
|
||||
const fileExecutor = new ScriptFileExecutorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(new ScriptFileCreatorStub().withCreatedFilePath(expectedFilePath))
|
||||
.withFileExecutor(fileExecutor);
|
||||
|
||||
// act
|
||||
await context.runCode();
|
||||
// act
|
||||
await context.runCode();
|
||||
|
||||
// assert
|
||||
const createCalls = fileCreator.callHistory.filter((call) => call.methodName === 'createScriptFile');
|
||||
expect(createCalls.length).to.equal(1);
|
||||
const [actualCode] = createCalls[0].args;
|
||||
expect(actualCode).to.equal(expectedCode);
|
||||
// assert
|
||||
const executeCalls = fileExecutor.callHistory.filter((call) => call.methodName === 'executeScriptFile');
|
||||
expect(executeCalls.length).to.equal(1);
|
||||
const [actualPath] = executeCalls[0].args;
|
||||
expect(actualPath).to.equal(expectedFilePath);
|
||||
});
|
||||
});
|
||||
it('creates script file with provided extension', async () => {
|
||||
// arrange
|
||||
const expectedFileExtension = 'expected-file-extension';
|
||||
const fileCreator = new ScriptFileCreatorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(fileCreator)
|
||||
.withFileExtension(expectedFileExtension);
|
||||
describe('successful run', () => {
|
||||
it('indicates success', async () => {
|
||||
// arrange
|
||||
const expectedSuccessResult = true;
|
||||
const context = new CodeRunnerTestSetup();
|
||||
|
||||
// act
|
||||
await context.runCode();
|
||||
// act
|
||||
const { success: actualSuccessValue } = await context.runCode();
|
||||
|
||||
// assert
|
||||
const createCalls = fileCreator.callHistory.filter((call) => call.methodName === 'createScriptFile');
|
||||
expect(createCalls.length).to.equal(1);
|
||||
const [,scriptFileNameParts] = createCalls[0].args;
|
||||
expectExists(scriptFileNameParts, JSON.stringify(`Call args: ${JSON.stringify(createCalls[0].args)}`));
|
||||
expect(scriptFileNameParts.scriptFileExtension).to.equal(expectedFileExtension);
|
||||
});
|
||||
it('creates script file with provided name', async () => {
|
||||
// arrange
|
||||
const expectedScriptName = ScriptFileName;
|
||||
const fileCreator = new ScriptFileCreatorStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withFileCreator(fileCreator);
|
||||
// assert
|
||||
expect(actualSuccessValue).to.equal(expectedSuccessResult);
|
||||
});
|
||||
it('logs success message', async () => {
|
||||
// arrange
|
||||
const expectedMessagePart = 'Successfully ran script';
|
||||
const logger = new LoggerStub();
|
||||
const context = new CodeRunnerTestSetup()
|
||||
.withLogger(logger);
|
||||
|
||||
// act
|
||||
await context.runCode();
|
||||
// act
|
||||
await context.runCode();
|
||||
|
||||
// assert
|
||||
const createCalls = fileCreator.callHistory.filter((call) => call.methodName === 'createScriptFile');
|
||||
expect(createCalls.length).to.equal(1);
|
||||
const [,scriptFileNameParts] = createCalls[0].args;
|
||||
expectExists(scriptFileNameParts, JSON.stringify(`Call args: ${JSON.stringify(createCalls[0].args)}`));
|
||||
expect(scriptFileNameParts.scriptName).to.equal(expectedScriptName);
|
||||
// assert
|
||||
logger.assertLogsContainMessagePart('info', expectedMessagePart);
|
||||
});
|
||||
});
|
||||
describe('error handling', () => {
|
||||
const testScenarios: ReadonlyArray<{
|
||||
readonly description: string;
|
||||
readonly injectedException: Error;
|
||||
readonly faultyContext: CodeRunnerTestSetup;
|
||||
readonly expectedErrorType: CodeRunErrorType;
|
||||
readonly expectedErrorMessage: string;
|
||||
buildFaultyContext(
|
||||
setup: CodeRunnerTestSetup,
|
||||
errorMessage: string,
|
||||
errorType: CodeRunErrorType,
|
||||
): CodeRunnerTestSetup;
|
||||
}> = [
|
||||
(() => {
|
||||
const error = new Error('Test Error: Script file execution intentionally failed for testing purposes.');
|
||||
const executor = new ScriptFileExecutorStub();
|
||||
executor.executeScriptFile = () => {
|
||||
throw error;
|
||||
};
|
||||
return {
|
||||
description: 'fails to execute script file',
|
||||
injectedException: error,
|
||||
faultyContext: new CodeRunnerTestSetup().withFileExecutor(executor),
|
||||
};
|
||||
})(),
|
||||
(() => {
|
||||
const error = new Error('Test Error: Script file creation intentionally failed for testing purposes.');
|
||||
const creator = new ScriptFileCreatorStub();
|
||||
creator.createScriptFile = () => {
|
||||
throw error;
|
||||
};
|
||||
return {
|
||||
description: 'fails to create script file',
|
||||
injectedException: error,
|
||||
faultyContext: new CodeRunnerTestSetup().withFileCreator(creator),
|
||||
};
|
||||
})(),
|
||||
{
|
||||
description: 'execution failure',
|
||||
expectedErrorType: 'FileExecutionError',
|
||||
expectedErrorMessage: 'execution error',
|
||||
buildFaultyContext: (setup, errorMessage, errorType) => {
|
||||
const executor = new ScriptFileExecutorStub();
|
||||
executor.executeScriptFile = () => Promise.resolve({
|
||||
success: false,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: errorType,
|
||||
},
|
||||
});
|
||||
return setup.withFileExecutor(executor);
|
||||
},
|
||||
},
|
||||
{
|
||||
description: 'creation failure',
|
||||
expectedErrorType: 'FileWriteError',
|
||||
expectedErrorMessage: 'creation error',
|
||||
buildFaultyContext: (setup, errorMessage, errorType) => {
|
||||
const creator = new ScriptFileCreatorStub();
|
||||
creator.createScriptFile = () => Promise.resolve({
|
||||
success: false,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: errorType,
|
||||
},
|
||||
});
|
||||
return setup.withFileCreator(creator);
|
||||
},
|
||||
},
|
||||
];
|
||||
describe('logs errors', () => {
|
||||
testScenarios.forEach(({ description, faultyContext }) => {
|
||||
it(`logs error when ${description}`, async () => {
|
||||
// arrange
|
||||
const logger = new LoggerStub();
|
||||
faultyContext.withLogger(logger);
|
||||
// act
|
||||
try {
|
||||
await faultyContext.runCode();
|
||||
} catch {
|
||||
// Swallow
|
||||
}
|
||||
// assert
|
||||
const errorCall = logger.callHistory.find((c) => c.methodName === 'error');
|
||||
expectExists(errorCall);
|
||||
});
|
||||
});
|
||||
});
|
||||
describe('rethrows errors', () => {
|
||||
testScenarios.forEach(({ description, injectedException, faultyContext }) => {
|
||||
it(`rethrows error when ${description}`, async () => {
|
||||
// act
|
||||
const act = () => faultyContext.runCode();
|
||||
// assert
|
||||
await expectThrowsAsync(act, injectedException.message);
|
||||
});
|
||||
testScenarios.forEach(({
|
||||
description, expectedErrorType, expectedErrorMessage, buildFaultyContext,
|
||||
}) => {
|
||||
it(`handles ${description}`, async () => {
|
||||
// arrange
|
||||
const context = buildFaultyContext(
|
||||
new CodeRunnerTestSetup(),
|
||||
expectedErrorMessage,
|
||||
expectedErrorType,
|
||||
);
|
||||
|
||||
// act
|
||||
const { success, error } = await context.runCode();
|
||||
|
||||
// assert
|
||||
expect(success).to.equal(false);
|
||||
expectExists(error);
|
||||
expect(error.message).to.include(expectedErrorMessage);
|
||||
expect(error.type).to.equal(expectedErrorType);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -155,13 +191,14 @@ class CodeRunnerTestSetup {
|
||||
|
||||
private logger: Logger = new LoggerStub();
|
||||
|
||||
public async runCode(): Promise<void> {
|
||||
public runCode() {
|
||||
const runner = new ScriptFileCodeRunner(
|
||||
this.fileExecutor,
|
||||
this.fileCreator,
|
||||
this.logger,
|
||||
);
|
||||
await runner.runCode(this.code, this.fileExtension);
|
||||
return runner
|
||||
.runCode(this.code, this.fileExtension);
|
||||
}
|
||||
|
||||
public withFileExecutor(fileExecutor: ScriptFileExecutor): this {
|
||||
|
||||
Reference in New Issue
Block a user