From e09db0f1bd73503204d8e5375a9cfe693f174a57 Mon Sep 17 00:00:00 2001 From: undergroundwires Date: Sun, 14 Jan 2024 22:35:53 +0100 Subject: [PATCH] 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. --- README.md | 2 +- docs/desktop-vs-web-features.md | 14 +- src/application/CodeRunner/CodeRunner.ts | 32 +- src/application/CodeRunner/ScriptFileName.ts | 1 - src/application/CodeRunner/ScriptFilename.ts | 1 + .../Directory/PersistentDirectoryProvider.ts | 124 ++++++-- .../Directory/ScriptDirectoryProvider.ts | 22 +- .../Creation/Filename/FilenameGenerator.ts | 4 +- .../Filename/TimestampedFilenameGenerator.ts | 16 +- .../ScriptFileCreationOrchestrator.ts | 107 ++++++- .../CodeRunner/Creation/ScriptFileCreator.ts | 26 +- .../Execution/ScriptFileExecutor.ts | 21 +- .../VisibleTerminalScriptFileExecutor.ts | 67 +++- .../CodeRunner/ScriptFileCodeRunner.ts | 48 ++- .../Dialog/Browser/BrowserDialog.ts | 21 +- .../Dialog/Browser/BrowserSaveFileDialog.ts | 6 +- .../Dialog/Browser/FileSaverDialog.ts | 15 +- .../Dialog/Electron/ElectronDialog.ts | 24 +- .../Dialog/Electron/ElectronSaveFileDialog.ts | 6 +- .../Electron/NodeElectronSaveFileDialog.ts | 150 ++++++--- .../Dialog/LoggingDialogDecorator.ts | 36 +++ src/presentation/common/Dialog.ts | 29 +- .../Code/CodeButtons/CodeRunButton.vue | 33 +- .../Code/CodeButtons/Save/CodeSaveButton.vue | 43 ++- .../Hooks/Dialog/ClientDialogFactory.ts | 34 +- .../Shared/Hooks/Dialog/UseDialog.ts | 4 +- .../IpcBridging/IpcChannelDefinitions.ts | 2 +- .../CodeRunner/ScriptFileCodeRunner.spec.ts | 14 +- tests/shared/Assertions/ExpectTrue.ts | 18 ++ .../PersistentDirectoryProvider.spec.ts | 144 ++++++--- .../TimestampedFilenameGenerator.spec.ts | 4 +- .../ScriptFileCreationOrchestrator.spec.ts | 206 +++++++++--- .../VisibleTerminalScriptFileExecutor.spec.ts | 162 +++++++--- .../CodeRunner/ScriptFileCodeRunner.spec.ts | 265 +++++++++------- .../Dialog/Browser/BrowserDialog.spec.ts | 101 +++++- .../Dialog/Browser/FileSaverDialog.spec.ts | 4 +- .../Dialog/Electron/ElectronDialog.spec.ts | 100 +++++- .../NodeElectronSaveFileDialog.spec.ts | 296 +++++++++++++----- .../Dialog/LoggingDialogDecorator.spec.ts | 163 ++++++++++ .../Modules/AppInitializationLogger.spec.ts | 6 +- .../Hooks/Dialog/ClientDialogFactory.spec.ts | 104 ++++-- tests/unit/shared/Stubs/CodeRunnerStub.ts | 8 +- tests/unit/shared/Stubs/DialogStub.ts | 24 +- .../shared/Stubs/FilenameGeneratorStub.ts | 4 +- tests/unit/shared/Stubs/LoggerStub.ts | 25 ++ .../Stubs/ScriptDirectoryProviderStub.ts | 9 +- .../shared/Stubs/ScriptFileCreatorStub.ts | 11 +- .../shared/Stubs/ScriptFileExecutorStub.ts | 8 +- 48 files changed, 1986 insertions(+), 578 deletions(-) delete mode 100644 src/application/CodeRunner/ScriptFileName.ts create mode 100644 src/application/CodeRunner/ScriptFilename.ts create mode 100644 src/infrastructure/Dialog/LoggingDialogDecorator.ts create mode 100644 tests/shared/Assertions/ExpectTrue.ts create mode 100644 tests/unit/infrastructure/Dialog/LoggingDialogDecorator.spec.ts diff --git a/README.md b/README.md index 12ce51ea..01c0fd73 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ For a detailed comparison of features between the desktop and web versions of pr - **Transparent**. Have full visibility into what the tweaks do as you enable them. - **Reversible**. Revert if something feels wrong. - **Accessible**. No need to run any compiled software on your computer with web version. -- **Secure**: Security is a top priority at privacy.sexy with comprehensive safeguards in place. [Learn more](./SECURITY.md). +- **Secure**: Security is a top priority at privacy.sexy with [comprehensive safeguards](./SECURITY.md#application-security) in place. - **Open**. What you see as code in this repository is what you get. The application itself, its infrastructure and deployments are open-source and automated thanks to [bump-everywhere](https://github.com/undergroundwires/bump-everywhere). - **Tested**. A lot of tests. Automated and manual. Community-testing and verification. Stability improvements comes before new features. - **Extensible**. Effortlessly [extend scripts](./CONTRIBUTING.md#extend-scripts) with a custom designed [templating language](./docs/templating.md). diff --git a/docs/desktop-vs-web-features.md b/docs/desktop-vs-web-features.md index b8698adb..a5492834 100644 --- a/docs/desktop-vs-web-features.md +++ b/docs/desktop-vs-web-features.md @@ -9,6 +9,8 @@ This table highlights differences between the desktop and web versions of `priva | [Auto-updates](#auto-updates) | 🟢 Available | 🟢 Available | | [Logging](#logging) | 🟢 Available | 🔴 Not available | | [Script execution](#script-execution) | 🟢 Available | 🔴 Not available | +| [Error handling](#error-handling) | 🟢 Advanced | 🟡 Limited | +| [Native dialogs](#error-handling) | 🟢 Available | 🔴 Not available | ## Feature descriptions @@ -53,7 +55,7 @@ Log file locations vary by operating system: The desktop version of privacy.sexy enables direct script execution, providing a seamless and integrated experience. This direct execution capability isn't available in the web version due to inherent browser restrictions. -**Logging and storage:** +**Script execution history:** For enhanced auditability and easier troubleshooting, the desktop version keeps a record of executed scripts in designated directories. These locations vary based on the operating system: @@ -62,7 +64,13 @@ These locations vary based on the operating system: - Linux: `$HOME/.config/privacy.sexy/runs` - Windows: `%APPDATA%\privacy.sexy\runs` -**Native file system dialogs:** +### Error handling -The desktop version uses native system file save dialogs, offering more features and reliability compared to the browser's file system dialogs. +The desktop version of privacy.sexy features advanced error handling capabilities. +It employs robust and reliable execution strategies, including self-healing mechanisms, and provides guidance and troubleshooting information to resolve issues effectively. +In contrast, the web version has more basic error handling due to browser limitations and the nature of web applications. + +### Native dialogs + +The desktop version uses native dialogs, offering more features and reliability compared to the browser's file system dialogs. These native dialogs provide a more integrated and user-friendly experience, aligning with the operating system's standard interface and functionalities. diff --git a/src/application/CodeRunner/CodeRunner.ts b/src/application/CodeRunner/CodeRunner.ts index 176eda1a..40e41f7d 100644 --- a/src/application/CodeRunner/CodeRunner.ts +++ b/src/application/CodeRunner/CodeRunner.ts @@ -2,5 +2,35 @@ export interface CodeRunner { runCode( code: string, fileExtension: string, - ): Promise; + ): Promise; +} + +export type CodeRunErrorType = + | 'FileWriteError' + | 'FilePathGenerationError' + | 'UnsupportedOperatingSystem' + | 'FileExecutionError' + | 'DirectoryCreationError' + | 'UnexpectedError'; + +export type CodeRunOutcome = SuccessfulCodeRun | FailedCodeRun; + +interface CodeRunStatus { + readonly success: boolean; + readonly error?: CodeRunError; +} + +interface SuccessfulCodeRun extends CodeRunStatus { + readonly success: true; + readonly error?: undefined; +} + +export interface FailedCodeRun extends CodeRunStatus { + readonly success: false; + readonly error: CodeRunError; +} + +export interface CodeRunError { + readonly type: CodeRunErrorType; + readonly message: string; } diff --git a/src/application/CodeRunner/ScriptFileName.ts b/src/application/CodeRunner/ScriptFileName.ts deleted file mode 100644 index b108db12..00000000 --- a/src/application/CodeRunner/ScriptFileName.ts +++ /dev/null @@ -1 +0,0 @@ -export const ScriptFileName = 'privacy-script' as const; diff --git a/src/application/CodeRunner/ScriptFilename.ts b/src/application/CodeRunner/ScriptFilename.ts new file mode 100644 index 00000000..2e56d983 --- /dev/null +++ b/src/application/CodeRunner/ScriptFilename.ts @@ -0,0 +1 @@ +export const ScriptFilename = 'privacy-script' as const; diff --git a/src/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts b/src/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts index bf788ee3..023015c8 100644 --- a/src/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts +++ b/src/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts @@ -1,40 +1,116 @@ import { Logger } from '@/application/Common/Log/Logger'; import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; +import { CodeRunError, CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; import { SystemOperations } from '../../System/SystemOperations'; import { NodeElectronSystemOperations } from '../../System/NodeElectronSystemOperations'; -import { ScriptDirectoryProvider } from './ScriptDirectoryProvider'; +import { ScriptDirectoryOutcome, ScriptDirectoryProvider } from './ScriptDirectoryProvider'; export const ExecutionSubdirectory = 'runs'; +/** + * Provides a dedicated directory for script execution. + * Benefits of using a persistent directory: + * - Antivirus Exclusions: Easier antivirus configuration. + * - Auditability: Stores script execution history for troubleshooting. + * - Reliability: Avoids issues with directory clean-ups during execution, + * seen in Windows Pro Azure VMs when stored on Windows temporary directory. + */ export class PersistentDirectoryProvider implements ScriptDirectoryProvider { constructor( private readonly system: SystemOperations = new NodeElectronSystemOperations(), private readonly logger: Logger = ElectronLogger, ) { } - async provideScriptDirectory(): Promise { - const scriptsDirectory = this.system.location.combinePaths( - /* - Switched from temporary to persistent directory for script storage for improved reliability. - - Temporary directories in some environments, such certain Windows Pro Azure VMs, showed - issues where scripts were interrupted due to directory cleanup during script execution. - This was observed with system temp directories (e.g., `%LOCALAPPDATA%\Temp`). - - Persistent directories offer better stability during long executions and aid in auditability - and troubleshooting. - */ - this.system.operatingSystem.getUserDataDirectory(), - ExecutionSubdirectory, - ); - this.logger.info(`Attempting to create script directory at path: ${scriptsDirectory}`); - try { - await this.system.fileSystem.createDirectory(scriptsDirectory, true); - this.logger.info(`Script directory successfully created at: ${scriptsDirectory}`); - } catch (error) { - this.logger.error(`Error creating script directory at ${scriptsDirectory}: ${error.message}`, error); - throw error; + public async provideScriptDirectory(): Promise { + const { + success: isPathConstructed, + error: pathConstructionError, + directoryPath, + } = this.constructScriptDirectoryPath(); + if (!isPathConstructed) { + return { + success: false, + error: pathConstructionError, + }; } - return scriptsDirectory; + const { + success: isDirectoryCreated, + error: directoryCreationError, + } = await this.createDirectory(directoryPath); + if (!isDirectoryCreated) { + return { + success: false, + error: directoryCreationError, + }; + } + return { + success: true, + directoryAbsolutePath: directoryPath, + }; + } + + private async createDirectory(directoryPath: string): Promise { + try { + this.logger.info(`Attempting to create script directory at path: ${directoryPath}`); + await this.system.fileSystem.createDirectory(directoryPath, true); + this.logger.info(`Script directory successfully created at: ${directoryPath}`); + return { + success: true, + }; + } catch (error) { + return { + success: false, + error: this.handleException(error, 'DirectoryCreationError'), + }; + } + } + + private constructScriptDirectoryPath(): DirectoryPathConstructionOutcome { + try { + const parentDirectory = this.system.operatingSystem.getUserDataDirectory(); + const scriptDirectory = this.system.location.combinePaths( + parentDirectory, + ExecutionSubdirectory, + ); + return { + success: true, + directoryPath: scriptDirectory, + }; + } catch (error) { + return { + success: false, + error: this.handleException(error, 'DirectoryCreationError'), + }; + } + } + + private handleException( + exception: Error, + errorType: CodeRunErrorType, + ): CodeRunError { + const errorMessage = 'Error during script directory creation'; + this.logger.error(errorType, errorMessage, exception); + return { + type: errorType, + message: `${errorMessage}: ${exception.message}`, + }; } } + +type DirectoryPathConstructionOutcome = { + readonly success: false; + readonly error: CodeRunError; + readonly directoryPath?: undefined; +} | { + readonly success: true; + readonly directoryPath: string; + readonly error?: undefined; +}; + +type DirectoryPathCreationOutcome = { + readonly success: false; + readonly error: CodeRunError; +} | { + readonly success: true; + readonly error?: undefined; +}; diff --git a/src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts b/src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts index e28573da..5a09f305 100644 --- a/src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts +++ b/src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts @@ -1,3 +1,23 @@ +import { CodeRunError } from '@/application/CodeRunner/CodeRunner'; + export interface ScriptDirectoryProvider { - provideScriptDirectory(): Promise; + provideScriptDirectory(): Promise; +} + +export type ScriptDirectoryOutcome = SuccessfulDirectoryCreation | FailedDirectoryCreation; + +interface ScriptDirectoryCreationStatus { + readonly success: boolean; + readonly directoryAbsolutePath?: string; + readonly error?: CodeRunError; +} + +interface SuccessfulDirectoryCreation extends ScriptDirectoryCreationStatus { + readonly success: true; + readonly directoryAbsolutePath: string; +} + +interface FailedDirectoryCreation extends ScriptDirectoryCreationStatus { + readonly success: false; + readonly error: CodeRunError; } diff --git a/src/infrastructure/CodeRunner/Creation/Filename/FilenameGenerator.ts b/src/infrastructure/CodeRunner/Creation/Filename/FilenameGenerator.ts index 6c5615ca..b03f173c 100644 --- a/src/infrastructure/CodeRunner/Creation/Filename/FilenameGenerator.ts +++ b/src/infrastructure/CodeRunner/Creation/Filename/FilenameGenerator.ts @@ -1,5 +1,5 @@ -import { ScriptFileNameParts } from '../ScriptFileCreator'; +import { ScriptFilenameParts } from '../ScriptFileCreator'; export interface FilenameGenerator { - generateFilename(scriptFileNameParts: ScriptFileNameParts): string; + generateFilename(scriptFilenameParts: ScriptFilenameParts): string; } diff --git a/src/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.ts b/src/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.ts index 81d3c237..c7626c31 100644 --- a/src/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.ts +++ b/src/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.ts @@ -1,14 +1,14 @@ -import { ScriptFileNameParts } from '../ScriptFileCreator'; +import { ScriptFilenameParts } from '../ScriptFileCreator'; import { FilenameGenerator } from './FilenameGenerator'; export class TimestampedFilenameGenerator implements FilenameGenerator { public generateFilename( - scriptFileNameParts: ScriptFileNameParts, + scriptFilenameParts: ScriptFilenameParts, date = new Date(), ): string { - validateScriptFileNameParts(scriptFileNameParts); - const baseFileName = `${createTimeStampForFile(date)}-${scriptFileNameParts.scriptName}`; - return scriptFileNameParts.scriptFileExtension ? `${baseFileName}.${scriptFileNameParts.scriptFileExtension}` : baseFileName; + validateScriptFilenameParts(scriptFilenameParts); + const baseFilename = `${createTimeStampForFile(date)}-${scriptFilenameParts.scriptName}`; + return scriptFilenameParts.scriptFileExtension ? `${baseFilename}.${scriptFilenameParts.scriptFileExtension}` : baseFilename; } } @@ -21,11 +21,11 @@ function createTimeStampForFile(date: Date): string { .replace(/\..+/, ''); } -function validateScriptFileNameParts(scriptFileNameParts: ScriptFileNameParts) { - if (!scriptFileNameParts.scriptName) { +function validateScriptFilenameParts(scriptFilenameParts: ScriptFilenameParts) { + if (!scriptFilenameParts.scriptName) { throw new Error('Script name is required but not provided.'); } - if (scriptFileNameParts.scriptFileExtension?.startsWith('.')) { + if (scriptFilenameParts.scriptFileExtension?.startsWith('.')) { throw new Error('File extension should not start with a dot.'); } } diff --git a/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts b/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts index 8e485fa9..d8556ee5 100644 --- a/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts +++ b/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts @@ -1,9 +1,10 @@ import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; import { Logger } from '@/application/Common/Log/Logger'; +import { CodeRunError, CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; import { SystemOperations } from '../System/SystemOperations'; import { NodeElectronSystemOperations } from '../System/NodeElectronSystemOperations'; import { FilenameGenerator } from './Filename/FilenameGenerator'; -import { ScriptFileNameParts, ScriptFileCreator } from './ScriptFileCreator'; +import { ScriptFilenameParts, ScriptFileCreator, ScriptFileCreationOutcome } from './ScriptFileCreator'; import { TimestampedFilenameGenerator } from './Filename/TimestampedFilenameGenerator'; import { ScriptDirectoryProvider } from './Directory/ScriptDirectoryProvider'; import { PersistentDirectoryProvider } from './Directory/PersistentDirectoryProvider'; @@ -18,23 +19,99 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator { public async createScriptFile( contents: string, - scriptFileNameParts: ScriptFileNameParts, - ): Promise { - const filePath = await this.provideFilePath(scriptFileNameParts); - await this.createFile(filePath, contents); - return filePath; + scriptFilenameParts: ScriptFilenameParts, + ): Promise { + const { + success: isDirectoryCreated, error: directoryCreationError, directoryAbsolutePath, + } = await this.directoryProvider.provideScriptDirectory(); + if (!isDirectoryCreated) { + return createFailure(directoryCreationError); + } + const { + success: isFilePathConstructed, error: filePathGenerationError, filePath, + } = this.constructFilePath(scriptFilenameParts, directoryAbsolutePath); + if (!isFilePathConstructed) { + return createFailure(filePathGenerationError); + } + const { + success: isFileCreated, error: fileCreationError, + } = await this.writeFile(filePath, contents); + if (!isFileCreated) { + return createFailure(fileCreationError); + } + return { + success: true, + scriptFileAbsolutePath: filePath, + }; } - private async provideFilePath(scriptFileNameParts: ScriptFileNameParts): Promise { - const filename = this.filenameGenerator.generateFilename(scriptFileNameParts); - const directoryPath = await this.directoryProvider.provideScriptDirectory(); - const filePath = this.system.location.combinePaths(directoryPath, filename); - return filePath; + private constructFilePath( + scriptFilenameParts: ScriptFilenameParts, + directoryPath: string, + ): FilePathConstructionOutcome { + try { + const filename = this.filenameGenerator.generateFilename(scriptFilenameParts); + const filePath = this.system.location.combinePaths(directoryPath, filename); + return { success: true, filePath }; + } catch (error) { + return { + success: false, + error: this.handleException(error, 'FilePathGenerationError'), + }; + } } - private async createFile(filePath: string, contents: string): Promise { - this.logger.info(`Creating file at ${filePath}, size: ${contents.length} characters`); - await this.system.fileSystem.writeToFile(filePath, contents); - this.logger.info(`File created successfully at ${filePath}`); + private async writeFile( + filePath: string, + contents: string, + ): Promise { + try { + this.logger.info(`Creating file at ${filePath}, size: ${contents.length} characters`); + await this.system.fileSystem.writeToFile(filePath, contents); + this.logger.info(`File created successfully at ${filePath}`); + return { success: true }; + } catch (error) { + return { + success: false, + error: this.handleException(error, 'FileWriteError'), + }; + } + } + + private handleException( + exception: Error, + errorType: CodeRunErrorType, + ): CodeRunError { + const errorMessage = 'Error during script file operation'; + this.logger.error(errorType, errorMessage, exception); + return { + type: errorType, + message: `${errorMessage}: ${exception.message}`, + }; } } + +function createFailure(error: CodeRunError): ScriptFileCreationOutcome { + return { + success: false, + error, + }; +} + +type FileWriteOutcome = { + readonly success: true; + readonly error?: undefined; +} | { + readonly success: false; + readonly error: CodeRunError; +}; + +type FilePathConstructionOutcome = { + readonly success: true; + readonly filePath: string; + readonly error?: undefined; +} | { + readonly success: false; + readonly filePath?: undefined; + readonly error: CodeRunError; +}; diff --git a/src/infrastructure/CodeRunner/Creation/ScriptFileCreator.ts b/src/infrastructure/CodeRunner/Creation/ScriptFileCreator.ts index e4cd2493..3ab60422 100644 --- a/src/infrastructure/CodeRunner/Creation/ScriptFileCreator.ts +++ b/src/infrastructure/CodeRunner/Creation/ScriptFileCreator.ts @@ -1,11 +1,31 @@ +import { CodeRunError } from '@/application/CodeRunner/CodeRunner'; + export interface ScriptFileCreator { createScriptFile( contents: string, - scriptFileNameParts: ScriptFileNameParts, - ): Promise; + scriptFilenameParts: ScriptFilenameParts, + ): Promise; } -export interface ScriptFileNameParts { +export interface ScriptFilenameParts { readonly scriptName: string; readonly scriptFileExtension: string | undefined; } + +export type ScriptFileCreationOutcome = SuccessfulScriptCreation | FailedScriptCreation; + +interface ScriptFileCreationStatus { + readonly success: boolean; + readonly error?: CodeRunError; + readonly scriptFileAbsolutePath?: string; +} + +interface SuccessfulScriptCreation extends ScriptFileCreationStatus { + readonly success: true; + readonly scriptFileAbsolutePath: string; +} + +interface FailedScriptCreation extends ScriptFileCreationStatus { + readonly success: false; + readonly error: CodeRunError; +} diff --git a/src/infrastructure/CodeRunner/Execution/ScriptFileExecutor.ts b/src/infrastructure/CodeRunner/Execution/ScriptFileExecutor.ts index a4dde93b..b2de5f01 100644 --- a/src/infrastructure/CodeRunner/Execution/ScriptFileExecutor.ts +++ b/src/infrastructure/CodeRunner/Execution/ScriptFileExecutor.ts @@ -1,3 +1,22 @@ +import { CodeRunError } from '@/application/CodeRunner/CodeRunner'; + export interface ScriptFileExecutor { - executeScriptFile(filePath: string): Promise; + executeScriptFile(filePath: string): Promise; +} + +export type ScriptFileExecutionOutcome = SuccessfulScriptFileExecution | FailedScriptFileExecution; + +interface ScriptFileExecutionStatus { + readonly success: boolean; + readonly error?: CodeRunError; +} + +interface SuccessfulScriptFileExecution extends ScriptFileExecutionStatus { + readonly success: true; + readonly error?: undefined; +} + +export interface FailedScriptFileExecution extends ScriptFileExecutionStatus { + readonly success: false; + readonly error: CodeRunError; } diff --git a/src/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.ts b/src/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.ts index f0867f67..882bbe90 100644 --- a/src/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.ts +++ b/src/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.ts @@ -5,7 +5,9 @@ import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment'; import { NodeElectronSystemOperations } from '@/infrastructure/CodeRunner/System/NodeElectronSystemOperations'; import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory'; -import { ScriptFileExecutor } from './ScriptFileExecutor'; +import { CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; +import { isString } from '@/TypeHelpers'; +import { FailedScriptFileExecution, ScriptFileExecutionOutcome, ScriptFileExecutor } from './ScriptFileExecutor'; export class VisibleTerminalScriptExecutor implements ScriptFileExecutor { constructor( @@ -14,38 +16,77 @@ export class VisibleTerminalScriptExecutor implements ScriptFileExecutor { private readonly environment: RuntimeEnvironment = CurrentEnvironment, ) { } - public async executeScriptFile(filePath: string): Promise { + public async executeScriptFile(filePath: string): Promise { const { os } = this.environment; if (os === undefined) { - throw new Error('Unknown operating system'); + return this.handleError('UnsupportedOperatingSystem', 'Operating system could not be identified from environment.'); } - await this.setFileExecutablePermissions(filePath); - await this.runFileWithRunner(filePath, os); + const filePermissionsResult = await this.setFileExecutablePermissions(filePath); + if (!filePermissionsResult.success) { + return filePermissionsResult; + } + const scriptExecutionResult = await this.runFileWithRunner(filePath, os); + if (!scriptExecutionResult.success) { + return scriptExecutionResult; + } + return { + success: true, + }; } - private async setFileExecutablePermissions(filePath: string): Promise { + private async setFileExecutablePermissions( + filePath: string, + ): Promise { /* This is required on macOS and Linux otherwise the terminal emulators will refuse to execute the script. It's not needed on Windows. */ - this.logger.info(`Setting execution permissions for file at ${filePath}`); - await this.system.fileSystem.setFilePermissions(filePath, '755'); - this.logger.info(`Execution permissions set successfully for ${filePath}`); + try { + this.logger.info(`Setting execution permissions for file at ${filePath}`); + await this.system.fileSystem.setFilePermissions(filePath, '755'); + this.logger.info(`Execution permissions set successfully for ${filePath}`); + return { success: true }; + } catch (error) { + return this.handleError('FileExecutionError', error); + } } - private async runFileWithRunner(filePath: string, os: OperatingSystem): Promise { + private async runFileWithRunner( + filePath: string, + os: OperatingSystem, + ): Promise { this.logger.info(`Executing script file: ${filePath} on ${OperatingSystem[os]}.`); const runner = TerminalRunners[os]; if (!runner) { - throw new Error(`Unsupported operating system: ${OperatingSystem[os]}`); + return this.handleError('UnsupportedOperatingSystem', `Unsupported operating system: ${OperatingSystem[os]}`); } const context: TerminalExecutionContext = { scriptFilePath: filePath, commandOps: this.system.command, logger: this.logger, }; - await runner(context); - this.logger.info('Command script file successfully.'); + try { + await runner(context); + this.logger.info('Command script file successfully.'); + return { success: true }; + } catch (error) { + return this.handleError('FileExecutionError', error); + } + } + + private handleError( + type: CodeRunErrorType, + error: Error | string, + ): FailedScriptFileExecution { + const errorMessage = 'Error during script file execution'; + this.logger.error([type, errorMessage, ...(error ? [error] : [])]); + return { + success: false, + error: { + type, + message: `${errorMessage}: ${isString(error) ? error : errorMessage}`, + }, + }; } } diff --git a/src/infrastructure/CodeRunner/ScriptFileCodeRunner.ts b/src/infrastructure/CodeRunner/ScriptFileCodeRunner.ts index 3b4ef39e..f330d29d 100644 --- a/src/infrastructure/CodeRunner/ScriptFileCodeRunner.ts +++ b/src/infrastructure/CodeRunner/ScriptFileCodeRunner.ts @@ -1,6 +1,8 @@ import { Logger } from '@/application/Common/Log/Logger'; -import { ScriptFileName } from '@/application/CodeRunner/ScriptFileName'; -import { CodeRunner } from '@/application/CodeRunner/CodeRunner'; +import { ScriptFilename } from '@/application/CodeRunner/ScriptFilename'; +import { + CodeRunError, CodeRunOutcome, CodeRunner, FailedCodeRun, +} from '@/application/CodeRunner/CodeRunner'; import { ElectronLogger } from '../Log/ElectronLogger'; import { ScriptFileExecutor } from './Execution/ScriptFileExecutor'; import { ScriptFileCreator } from './Creation/ScriptFileCreator'; @@ -18,18 +20,38 @@ export class ScriptFileCodeRunner implements CodeRunner { public async runCode( code: string, fileExtension: string, - ): Promise { + ): Promise { this.logger.info('Initiating script running process.'); - try { - const scriptFilePath = await this.scriptFileCreator.createScriptFile(code, { - scriptName: ScriptFileName, - scriptFileExtension: fileExtension, - }); - await this.scriptFileExecutor.executeScriptFile(scriptFilePath); - this.logger.info(`Successfully ran script at ${scriptFilePath}`); - } catch (error) { - this.logger.error(`Error running script: ${error.message}`, error); - throw error; + const { + success: isFileCreated, scriptFileAbsolutePath, error: fileCreationError, + } = await this.scriptFileCreator.createScriptFile(code, { + scriptName: ScriptFilename, + scriptFileExtension: fileExtension, + }); + if (!isFileCreated) { + return createFailure(fileCreationError); } + const { + success: isFileSuccessfullyExecuted, + error: fileExecutionError, + } = await this.scriptFileExecutor.executeScriptFile( + scriptFileAbsolutePath, + ); + if (!isFileSuccessfullyExecuted) { + return createFailure(fileExecutionError); + } + this.logger.info(`Successfully ran script at ${scriptFileAbsolutePath}`); + return { + success: true, + }; } } + +function createFailure( + error: CodeRunError, +): FailedCodeRun { + return { + success: false, + error, + }; +} diff --git a/src/infrastructure/Dialog/Browser/BrowserDialog.ts b/src/infrastructure/Dialog/Browser/BrowserDialog.ts index 10cb28d0..d08de04d 100644 --- a/src/infrastructure/Dialog/Browser/BrowserDialog.ts +++ b/src/infrastructure/Dialog/Browser/BrowserDialog.ts @@ -1,19 +1,30 @@ -import { Dialog, FileType } from '@/presentation/common/Dialog'; +import { Dialog, FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; import { FileSaverDialog } from './FileSaverDialog'; import { BrowserSaveFileDialog } from './BrowserSaveFileDialog'; export class BrowserDialog implements Dialog { - constructor(private readonly saveFileDialog: BrowserSaveFileDialog = new FileSaverDialog()) { + constructor( + private readonly window: WindowDialogAccessor = globalThis.window, + private readonly saveFileDialog: BrowserSaveFileDialog = new FileSaverDialog(), + ) { } + public showError(title: string, message: string): void { + this.window.alert(`❌ ${title}\n\n${message}`); + } + public saveFile( fileContents: string, - fileName: string, + defaultFilename: string, type: FileType, - ): Promise { + ): Promise { return Promise.resolve( - this.saveFileDialog.saveFile(fileContents, fileName, type), + this.saveFileDialog.saveFile(fileContents, defaultFilename, type), ); } } + +export interface WindowDialogAccessor { + readonly alert: typeof window.alert; +} diff --git a/src/infrastructure/Dialog/Browser/BrowserSaveFileDialog.ts b/src/infrastructure/Dialog/Browser/BrowserSaveFileDialog.ts index c64f2da2..c32d9dbb 100644 --- a/src/infrastructure/Dialog/Browser/BrowserSaveFileDialog.ts +++ b/src/infrastructure/Dialog/Browser/BrowserSaveFileDialog.ts @@ -1,9 +1,9 @@ -import { FileType } from '@/presentation/common/Dialog'; +import { FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; export interface BrowserSaveFileDialog { saveFile( fileContents: string, - fileName: string, + defaultFilename: string, fileType: FileType, - ): void; + ): SaveFileOutcome; } diff --git a/src/infrastructure/Dialog/Browser/FileSaverDialog.ts b/src/infrastructure/Dialog/Browser/FileSaverDialog.ts index 12639f04..6c4078dd 100644 --- a/src/infrastructure/Dialog/Browser/FileSaverDialog.ts +++ b/src/infrastructure/Dialog/Browser/FileSaverDialog.ts @@ -1,5 +1,5 @@ import fileSaver from 'file-saver'; -import { FileType } from '@/presentation/common/Dialog'; +import { FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; import { BrowserSaveFileDialog } from './BrowserSaveFileDialog'; export type SaveAsFunction = (data: Blob, filename?: string) => void; @@ -14,17 +14,20 @@ export class FileSaverDialog implements BrowserSaveFileDialog { public saveFile( fileContents: string, - fileName: string, + defaultFilename: string, fileType: FileType, - ): void { + ): SaveFileOutcome { const mimeType = MimeTypes[fileType]; - this.saveBlob(fileContents, mimeType, fileName); + this.saveBlob(fileContents, mimeType, defaultFilename); + return { + success: true, // Exceptions are handled internally + }; } - private saveBlob(file: BlobPart, mimeType: string, fileName: string): void { + private saveBlob(file: BlobPart, mimeType: string, defaultFilename: string): void { try { const blob = new Blob([file], { type: mimeType }); - this.fileSaverSaveAs(blob, fileName); + this.fileSaverSaveAs(blob, defaultFilename); } catch (e) { this.windowOpen(`data:${mimeType},${encodeURIComponent(file.toString())}`, '_blank', ''); } diff --git a/src/infrastructure/Dialog/Electron/ElectronDialog.ts b/src/infrastructure/Dialog/Electron/ElectronDialog.ts index cbcde8c3..c5922f8e 100644 --- a/src/infrastructure/Dialog/Electron/ElectronDialog.ts +++ b/src/infrastructure/Dialog/Electron/ElectronDialog.ts @@ -1,17 +1,29 @@ -import { Dialog, FileType } from '@/presentation/common/Dialog'; +import { dialog } from 'electron/main'; +import { Dialog, FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; import { NodeElectronSaveFileDialog } from './NodeElectronSaveFileDialog'; import { ElectronSaveFileDialog } from './ElectronSaveFileDialog'; export class ElectronDialog implements Dialog { constructor( - private readonly fileSaveDialog: ElectronSaveFileDialog = new NodeElectronSaveFileDialog(), + private readonly saveFileDialog: ElectronSaveFileDialog = new NodeElectronSaveFileDialog(), + private readonly electron: ElectronDialogAccessor = { + showErrorBox: dialog.showErrorBox.bind(dialog), + }, ) { } - public async saveFile( + public saveFile( fileContents: string, - fileName: string, + defaultFilename: string, type: FileType, - ): Promise { - await this.fileSaveDialog.saveFile(fileContents, fileName, type); + ): Promise { + return this.saveFileDialog.saveFile(fileContents, defaultFilename, type); + } + + public showError(title: string, message: string): void { + this.electron.showErrorBox(title, message); } } + +export interface ElectronDialogAccessor { + readonly showErrorBox: typeof dialog.showErrorBox; +} diff --git a/src/infrastructure/Dialog/Electron/ElectronSaveFileDialog.ts b/src/infrastructure/Dialog/Electron/ElectronSaveFileDialog.ts index 6e35fd52..905b84f1 100644 --- a/src/infrastructure/Dialog/Electron/ElectronSaveFileDialog.ts +++ b/src/infrastructure/Dialog/Electron/ElectronSaveFileDialog.ts @@ -1,9 +1,9 @@ -import { FileType } from '@/presentation/common/Dialog'; +import { FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; export interface ElectronSaveFileDialog { saveFile( fileContents: string, - fileName: string, + defaultFilename: string, type: FileType, - ): Promise; + ): Promise; } diff --git a/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts b/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts index 2753caac..fe4ca469 100644 --- a/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts +++ b/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts @@ -3,19 +3,11 @@ import { writeFile } from 'node:fs/promises'; import { app, dialog } from 'electron/main'; import { Logger } from '@/application/Common/Log/Logger'; import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; -import { FileType } from '@/presentation/common/Dialog'; +import { + FileType, SaveFileError, SaveFileErrorType, SaveFileOutcome, +} from '@/presentation/common/Dialog'; import { ElectronSaveFileDialog } from './ElectronSaveFileDialog'; -export interface ElectronFileDialogOperations { - getUserDownloadsPath(): string; - showSaveDialog(options: Electron.SaveDialogOptions): Promise; -} - -export interface NodeFileOperations { - readonly join: typeof join; - writeFile(file: string, data: string): Promise; -} - export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog { constructor( private readonly logger: Logger = ElectronLogger, @@ -31,44 +23,123 @@ export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog { public async saveFile( fileContents: string, - fileName: string, + defaultFilename: string, type: FileType, - ): Promise { - const userSelectedFilePath = await this.showSaveFileDialog(fileName, type); - if (!userSelectedFilePath) { - this.logger.info(`File save cancelled by user: ${fileName}`); - return; + ): Promise { + const { + success: isPathConstructed, + filePath: defaultFilePath, + error: pathConstructionError, + } = this.constructDefaultFilePath(defaultFilename); + if (!isPathConstructed) { + return { success: false, error: pathConstructionError }; } - await this.writeFile(userSelectedFilePath, fileContents); + const fileDialog = await this.showSaveFileDialog(defaultFilename, defaultFilePath, type); + if (!fileDialog.success) { + return { + success: false, + error: fileDialog.error, + }; + } + if (fileDialog.canceled) { + this.logger.info(`File save cancelled by user: ${defaultFilename}`); + return { + success: true, + }; + } + const result = await this.writeFile(fileDialog.filePath, fileContents); + return result; } - private async writeFile(filePath: string, fileContents: string): Promise { + private async writeFile( + filePath: string, + fileContents: string, + ): Promise { try { this.logger.info(`Saving file: ${filePath}`); await this.node.writeFile(filePath, fileContents); this.logger.info(`File saved: ${filePath}`); + return { + success: true, + }; } catch (error) { - this.logger.error(`Error saving file: ${error.message}`); + return { + success: false, + error: this.handleException(error, 'FileCreationError'), + }; } } - private async showSaveFileDialog(fileName: string, type: FileType): Promise { - const downloadsFolder = this.electron.getUserDownloadsPath(); - const defaultFilePath = this.node.join(downloadsFolder, fileName); - const dialogResult = await this.electron.showSaveDialog({ - title: fileName, - defaultPath: defaultFilePath, - filters: getDialogFileFilters(type), - properties: [ - 'createDirectory', // Enables directory creation on macOS. - 'showOverwriteConfirmation', // Shows overwrite confirmation on Linux. - ], - }); - if (dialogResult.canceled) { - return undefined; + private async showSaveFileDialog( + defaultFilename: string, + defaultFilePath: string, + type: FileType, + ): Promise { + try { + const dialogResult = await this.electron.showSaveDialog({ + title: defaultFilename, + defaultPath: defaultFilePath, + filters: getDialogFileFilters(type), + properties: [ + 'createDirectory', // Enables directory creation on macOS. + 'showOverwriteConfirmation', // Shows overwrite confirmation on Linux. + ], + }); + if (dialogResult.canceled) { + return { success: true, canceled: true }; + } + if (!dialogResult.filePath) { + return { + success: false, + error: { type: 'DialogDisplayError', message: 'Unexpected Error: File path is undefined after save dialog completion.' }, + }; + } + return { success: true, filePath: dialogResult.filePath }; + } catch (error) { + return { + success: false, + error: this.handleException(error, 'DialogDisplayError'), + }; } - return dialogResult.filePath; } + + private constructDefaultFilePath(defaultFilename: string): DefaultFilePathConstructionOutcome { + try { + const downloadsFolder = this.electron.getUserDownloadsPath(); + const defaultFilePath = this.node.join(downloadsFolder, defaultFilename); + return { + success: true, + filePath: defaultFilePath, + }; + } catch (err) { + return { + success: false, + error: this.handleException(err, 'DialogDisplayError'), + }; + } + } + + private handleException( + exception: Error, + errorType: SaveFileErrorType, + ): SaveFileError { + const errorMessage = 'Error during saving script file.'; + this.logger.error(errorType, errorMessage, exception); + return { + type: errorType, + message: `${errorMessage}: ${exception.message}`, + }; + } +} + +export interface ElectronFileDialogOperations { + getUserDownloadsPath(): string; + showSaveDialog(options: Electron.SaveDialogOptions): Promise; +} + +export interface NodeFileOperations { + readonly join: typeof join; + writeFile(file: string, data: string): Promise; } function getDialogFileFilters(fileType: FileType): Electron.FileFilter[] { @@ -96,3 +167,12 @@ const FileTypeSpecificFilters: Record = { }, ], }; + +type SaveDialogOutcome = + | { readonly success: true; readonly filePath: string; readonly canceled?: false } + | { readonly success: true; readonly canceled: true } + | { readonly success: false; readonly error: SaveFileError; readonly canceled?: false }; + +type DefaultFilePathConstructionOutcome = + | { readonly success: true; readonly filePath: string; readonly error?: undefined; } + | { readonly success: false; readonly filePath?: undefined; readonly error: SaveFileError; }; diff --git a/src/infrastructure/Dialog/LoggingDialogDecorator.ts b/src/infrastructure/Dialog/LoggingDialogDecorator.ts new file mode 100644 index 00000000..580be3ca --- /dev/null +++ b/src/infrastructure/Dialog/LoggingDialogDecorator.ts @@ -0,0 +1,36 @@ +import { Logger } from '@/application/Common/Log/Logger'; +import { Dialog, FileType } from '@/presentation/common/Dialog'; + +export function decorateWithLogging( + dialog: Dialog, + logger: Logger, +): Dialog { + return new LoggingDialogDecorator(dialog, logger); +} + +class LoggingDialogDecorator implements Dialog { + constructor( + private readonly dialog: Dialog, + private readonly logger: Logger, + ) { } + + public async saveFile( + fileContents: string, + defaultFilename: string, + fileType: FileType, + ) { + this.logger.info(`Opening save file dialog with default filename: ${defaultFilename}.`); + const dialogResult = await this.dialog.saveFile(fileContents, defaultFilename, fileType); + if (dialogResult.success) { + this.logger.info('File saving process completed successfully.'); + } else { + this.logger.error('Error encountered while saving the file.', dialogResult.error); + } + return dialogResult; + } + + public showError(title: string, message: string) { + this.logger.error(`Showing error dialog: ${title} - ${message}`); + this.dialog.showError(title, message); + } +} diff --git a/src/presentation/common/Dialog.ts b/src/presentation/common/Dialog.ts index adb39c02..21e4c0b5 100644 --- a/src/presentation/common/Dialog.ts +++ b/src/presentation/common/Dialog.ts @@ -1,8 +1,35 @@ export interface Dialog { - saveFile(fileContents: string, fileName: string, type: FileType): Promise; + showError(title: string, message: string): void; + saveFile(fileContents: string, defaultFilename: string, type: FileType): Promise; } export enum FileType { BatchFile, ShellScript, } + +export type SaveFileOutcome = SuccessfulSaveFile | FailedSaveFile; + +interface SaveFileStatus { + readonly success: boolean; + readonly error?: SaveFileError; +} + +interface SuccessfulSaveFile extends SaveFileStatus { + readonly success: true; + readonly error?: SaveFileError; +} + +interface FailedSaveFile extends SaveFileStatus { + readonly success: false; + readonly error: SaveFileError; +} + +export interface SaveFileError { + readonly type: SaveFileErrorType; + readonly message: string; +} + +export type SaveFileErrorType = + | 'FileCreationError' + | 'DialogDisplayError'; diff --git a/src/presentation/components/Code/CodeButtons/CodeRunButton.vue b/src/presentation/components/Code/CodeButtons/CodeRunButton.vue index 65c89594..90540f90 100644 --- a/src/presentation/components/Code/CodeButtons/CodeRunButton.vue +++ b/src/presentation/components/Code/CodeButtons/CodeRunButton.vue @@ -3,7 +3,7 @@ v-if="canRun" text="Run" icon-name="play" - @click="executeCode" + @click="runCode" /> @@ -11,6 +11,7 @@ import { defineComponent, computed } from 'vue'; import { injectKey } from '@/presentation/injectionSymbols'; import { OperatingSystem } from '@/domain/OperatingSystem'; +import { Dialog } from '@/presentation/common/Dialog'; import IconButton from './IconButton.vue'; export default defineComponent({ @@ -21,6 +22,7 @@ export default defineComponent({ const { currentState, currentContext } = injectKey((keys) => keys.useCollectionState); const { os, isRunningAsDesktopApplication } = injectKey((keys) => keys.useRuntimeEnvironment); const { codeRunner } = injectKey((keys) => keys.useCodeRunner); + const { dialog } = injectKey((keys) => keys.useDialog); const canRun = computed(() => getCanRunState( currentState.value.os, @@ -28,17 +30,20 @@ export default defineComponent({ os, )); - async function executeCode() { + async function runCode() { if (!codeRunner) { throw new Error('missing code runner'); } - await codeRunner.runCode( + const { success, error } = await codeRunner.runCode( currentContext.state.code.current, currentContext.state.collection.scripting.fileExtension, ); + if (!success) { + showScriptRunError(dialog, `${error.type}: ${error.message}`); + } } return { canRun, - executeCode, + runCode, }; }, }); @@ -51,4 +56,24 @@ function getCanRunState( const isRunningOnSelectedOs = selectedOs === hostOs; return isRunningAsDesktopApplication && isRunningOnSelectedOs; } + +function showScriptRunError(dialog: Dialog, technicalDetails: string) { + dialog.showError( + 'Error Running Script', + [ + 'We encountered an issue while running the script.', + 'This could be due to a variety of factors such as system permissions, resource constraints, or security software interventions.', + '\n', + 'Here are some steps you can take:', + '- Confirm that you have the necessary permissions to execute scripts on your system.', + '- Check if there is sufficient disk space and system resources available.', + '- Antivirus or security software can sometimes mistakenly block script execution. If you suspect this, verify your security settings, or temporarily disable the security software to see if that resolves the issue.', + '- If possible, try running a different script to determine if the issue is specific to a particular script.', + '- Should the problem persist, reach out to the community for further assistance.', + '\n', + 'For your reference, here are the technical details of the error:', + technicalDetails, + ].join('\n'), + ); +} diff --git a/src/presentation/components/Code/CodeButtons/Save/CodeSaveButton.vue b/src/presentation/components/Code/CodeButtons/Save/CodeSaveButton.vue index 1d35451a..4850c37a 100644 --- a/src/presentation/components/Code/CodeButtons/Save/CodeSaveButton.vue +++ b/src/presentation/components/Code/CodeButtons/Save/CodeSaveButton.vue @@ -19,8 +19,8 @@ import { injectKey } from '@/presentation/injectionSymbols'; import ModalDialog from '@/presentation/components/Shared/Modal/ModalDialog.vue'; import { ScriptingLanguage } from '@/domain/ScriptingLanguage'; import { IScriptingDefinition } from '@/domain/IScriptingDefinition'; -import { ScriptFileName } from '@/application/CodeRunner/ScriptFileName'; -import { FileType } from '@/presentation/common/Dialog'; +import { ScriptFilename } from '@/application/CodeRunner/ScriptFilename'; +import { Dialog, FileType } from '@/presentation/common/Dialog'; import IconButton from '../IconButton.vue'; import InstructionList from './Instructions/InstructionList.vue'; import { IInstructionListData } from './Instructions/InstructionListData'; @@ -38,25 +38,28 @@ export default defineComponent({ const { dialog } = injectKey((keys) => keys.useDialog); const areInstructionsVisible = ref(false); - const fileName = computed(() => buildFileName(currentState.value.collection.scripting)); + const filename = computed(() => buildFilename(currentState.value.collection.scripting)); const instructions = computed(() => getInstructions( currentState.value.collection.os, - fileName.value, + filename.value, )); async function saveCode() { - await dialog.saveFile( + const { success, error } = await dialog.saveFile( currentState.value.code.current, - fileName.value, + filename.value, getType(currentState.value.collection.scripting.language), ); + if (!success) { + showScriptSaveError(dialog, `${error.type}: ${error.message}`); + return; + } areInstructionsVisible.value = true; } return { isRunningAsDesktopApplication, instructions, - fileName, areInstructionsVisible, saveCode, }; @@ -74,10 +77,30 @@ function getType(language: ScriptingLanguage) { } } -function buildFileName(scripting: IScriptingDefinition) { +function buildFilename(scripting: IScriptingDefinition) { if (scripting.fileExtension) { - return `${ScriptFileName}.${scripting.fileExtension}`; + return `${ScriptFilename}.${scripting.fileExtension}`; } - return ScriptFileName; + return ScriptFilename; +} + +function showScriptSaveError(dialog: Dialog, technicalDetails: string) { + dialog.showError( + 'Error Saving Script', + [ + 'An error occurred while saving the script.', + 'This issue may arise from insufficient permissions, limited disk space, or interference from security software.', + '\n', + 'To address this:', + '- Verify your permissions for the selected save directory.', + '- Check available disk space.', + '- Review your antivirus or security settings; adding an exclusion for privacy.sexy might be necessary.', + '- Try saving the script to a different location or modifying your selection.', + '- If the problem persists, reach out to the community for further assistance.', + '\n', + 'Technical Details:', + technicalDetails, + ].join('\n'), + ); } diff --git a/src/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.ts b/src/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.ts index 6c219b6c..0282a08d 100644 --- a/src/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.ts +++ b/src/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.ts @@ -1,8 +1,25 @@ import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment'; import { Dialog } from '@/presentation/common/Dialog'; import { BrowserDialog } from '@/infrastructure/Dialog/Browser/BrowserDialog'; +import { decorateWithLogging } from '@/infrastructure/Dialog/LoggingDialogDecorator'; +import { ClientLoggerFactory } from '../Log/ClientLoggerFactory'; -export function determineDialogBasedOnEnvironment( +export function createEnvironmentSpecificLoggedDialog( + environment: RuntimeEnvironment, + dialogLoggingDecorator: DialogLoggingDecorator = ClientLoggingDecorator, + windowInjectedDialogFactory: WindowDialogCreationFunction = () => globalThis.window.dialog, + browserDialogFactory: BrowserDialogCreationFunction = () => new BrowserDialog(), +): Dialog { + const dialog = determineDialogBasedOnEnvironment( + environment, + windowInjectedDialogFactory, + browserDialogFactory, + ); + const loggingDialog = dialogLoggingDecorator(dialog); + return loggingDialog; +} + +function determineDialogBasedOnEnvironment( environment: RuntimeEnvironment, windowInjectedDialogFactory: WindowDialogCreationFunction = () => globalThis.window.dialog, browserDialogFactory: BrowserDialogCreationFunction = () => new BrowserDialog(), @@ -10,16 +27,23 @@ export function determineDialogBasedOnEnvironment( if (!environment.isRunningAsDesktopApplication) { return browserDialogFactory(); } - const dialog = windowInjectedDialogFactory(); - if (!dialog) { + const windowDialog = windowInjectedDialogFactory(); + if (!windowDialog) { throw new Error([ - 'The Dialog API could not be retrieved from the window object.', + 'Failed to retrieve Dialog API from window object in desktop environment.', 'This may indicate that the Dialog API is either not implemented or not correctly exposed in the current desktop environment.', ].join('\n')); } - return dialog; + return windowDialog; } export type WindowDialogCreationFunction = () => Dialog | undefined; export type BrowserDialogCreationFunction = () => Dialog; + +export type DialogLoggingDecorator = (dialog: Dialog) => Dialog; + +const ClientLoggingDecorator: DialogLoggingDecorator = (dialog) => decorateWithLogging( + dialog, + ClientLoggerFactory.Current.logger, +); diff --git a/src/presentation/components/Shared/Hooks/Dialog/UseDialog.ts b/src/presentation/components/Shared/Hooks/Dialog/UseDialog.ts index 60bcdfa5..6c088510 100644 --- a/src/presentation/components/Shared/Hooks/Dialog/UseDialog.ts +++ b/src/presentation/components/Shared/Hooks/Dialog/UseDialog.ts @@ -1,9 +1,9 @@ import { Dialog } from '@/presentation/common/Dialog'; import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory'; -import { determineDialogBasedOnEnvironment } from './ClientDialogFactory'; +import { createEnvironmentSpecificLoggedDialog } from './ClientDialogFactory'; export function useDialog( - factory: DialogFactory = () => determineDialogBasedOnEnvironment(CurrentEnvironment), + factory: DialogFactory = () => createEnvironmentSpecificLoggedDialog(CurrentEnvironment), ) { const dialog = factory(); return { diff --git a/src/presentation/electron/shared/IpcBridging/IpcChannelDefinitions.ts b/src/presentation/electron/shared/IpcBridging/IpcChannelDefinitions.ts index be5cd7c3..5e11a5a2 100644 --- a/src/presentation/electron/shared/IpcBridging/IpcChannelDefinitions.ts +++ b/src/presentation/electron/shared/IpcBridging/IpcChannelDefinitions.ts @@ -5,7 +5,7 @@ import { IpcChannel } from './IpcChannel'; export const IpcChannelDefinitions = { CodeRunner: defineElectronIpcChannel('code-run', ['runCode']), - Dialog: defineElectronIpcChannel('dialogs', ['saveFile']), + Dialog: defineElectronIpcChannel('dialogs', ['showError', 'saveFile']), } as const; export type ChannelDefinitionKey = keyof typeof IpcChannelDefinitions; diff --git a/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts b/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts index 12d5329a..e3776a13 100644 --- a/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts +++ b/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts @@ -6,10 +6,10 @@ import { describe, it } from 'vitest'; import { ScriptDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider'; import { ScriptFileCreationOrchestrator } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator'; import { ScriptFileCodeRunner } from '@/infrastructure/CodeRunner/ScriptFileCodeRunner'; -import { expectDoesNotThrowAsync } from '@tests/shared/Assertions/ExpectThrowsAsync'; import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory'; import { OperatingSystem } from '@/domain/OperatingSystem'; import { LinuxTerminalEmulator } from '@/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor'; +import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage'; describe('ScriptFileCodeRunner', () => { it('executes simple script correctly', async ({ skip }) => { @@ -23,9 +23,12 @@ describe('ScriptFileCodeRunner', () => { const codeRunner = createCodeRunner(temporaryDirectoryProvider); const args = getPlatformSpecificArguments(currentOperatingSystem); // act - const act = () => codeRunner.runCode(...args); + const { success, error } = await codeRunner.runCode(...args); // assert - await expectDoesNotThrowAsync(act); + expect(success).to.equal(true, formatAssertionMessage([ + 'Failed to successfully execute the script.', + 'Details:', JSON.stringify(error), + ])); }); }); @@ -88,7 +91,10 @@ function createTemporaryDirectoryProvider(): ScriptDirectoryProvider { provideScriptDirectory: async () => { const temporaryDirectoryPathPrefix = join(tmpdir(), 'privacy-sexy-tests-'); const temporaryDirectoryFullPath = await mkdtemp(temporaryDirectoryPathPrefix); - return temporaryDirectoryFullPath; + return { + success: true, + directoryAbsolutePath: temporaryDirectoryFullPath, + }; }, }; } diff --git a/tests/shared/Assertions/ExpectTrue.ts b/tests/shared/Assertions/ExpectTrue.ts new file mode 100644 index 00000000..bdc2e0df --- /dev/null +++ b/tests/shared/Assertions/ExpectTrue.ts @@ -0,0 +1,18 @@ +/** + * Asserts that the provided boolean value is true. + * + * Useful when TypeScript's control flow analysis does not recognize standard + * assertions, ensuring `value` is treated as true in subsequent code. This helps + * prevent type errors and improves code safety and clarity. An optional custom + * error message can be provided for more detailed assertion failures. + */ +export function expectTrue(value: boolean, errorMessage?: string): asserts value is true { + if (value !== true) { + throw new Error([ + `Assertion failed: Expected true, received ${value.toString()}.`, + 'Assertion failed: expected value is not true.', + ...(typeof value !== 'boolean' ? [`Received type: ${typeof value}`] : []), + ...(errorMessage ? [errorMessage] : []), + ].join('\n')); + } +} diff --git a/tests/unit/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts b/tests/unit/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts index 69b338f5..ff5d9655 100644 --- a/tests/unit/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts +++ b/tests/unit/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts @@ -8,12 +8,13 @@ import { OperatingSystemOpsStub } from '@tests/unit/shared/Stubs/OperatingSystem import { SystemOperationsStub } from '@tests/unit/shared/Stubs/SystemOperationsStub'; import { FileSystemOpsStub } from '@tests/unit/shared/Stubs/FileSystemOpsStub'; import { expectExists } from '@tests/shared/Assertions/ExpectExists'; -import { expectThrowsAsync } from '@tests/shared/Assertions/ExpectThrowsAsync'; +import { CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; +import { expectTrue } from '@tests/shared/Assertions/ExpectTrue'; describe('PersistentDirectoryProvider', () => { describe('createDirectory', () => { - describe('path generation', () => { - it('uses user directory as base', async () => { + describe('path construction', () => { + it('bases path on user directory', async () => { // arrange const expectedBaseDirectory = 'base-directory'; const pathSegmentSeparator = '/STUB-SEGMENT-SEPARATOR/'; @@ -26,17 +27,18 @@ describe('PersistentDirectoryProvider', () => { .withLocation(locationOps)); // act - const actualDirectoryResult = await context.createDirectory(); + const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); // assert - const actualBaseDirectory = actualDirectoryResult.split(pathSegmentSeparator)[0]; + expectTrue(success); + const actualBaseDirectory = directoryAbsolutePath.split(pathSegmentSeparator)[0]; expect(actualBaseDirectory).to.equal(expectedBaseDirectory); const calls = locationOps.callHistory.filter((call) => call.methodName === 'combinePaths'); expect(calls.length).to.equal(1); const [combinedBaseDirectory] = calls[0].args; expect(combinedBaseDirectory).to.equal(expectedBaseDirectory); }); - it('appends execution subdirectory', async () => { + it('includes execution subdirectory in path', async () => { // arrange const expectedSubdirectory = ExecutionSubdirectory; const pathSegmentSeparator = '/STUB-SEGMENT-SEPARATOR/'; @@ -46,10 +48,11 @@ describe('PersistentDirectoryProvider', () => { .withLocation(locationOps)); // act - const actualDirectoryResult = await context.createDirectory(); + const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); // assert - const actualSubdirectory = actualDirectoryResult + expectTrue(success); + const actualSubdirectory = directoryAbsolutePath .split(pathSegmentSeparator) .pop(); expect(actualSubdirectory).to.equal(expectedSubdirectory); @@ -58,7 +61,7 @@ describe('PersistentDirectoryProvider', () => { const [,combinedSubdirectory] = calls[0].args; expect(combinedSubdirectory).to.equal(expectedSubdirectory); }); - it('correctly forms the full path', async () => { + it('forms full path correctly', async () => { // arrange const pathSegmentSeparator = '/'; const baseDirectory = 'base-directory'; @@ -71,14 +74,15 @@ describe('PersistentDirectoryProvider', () => { )); // act - const actualDirectory = await context.createDirectory(); + const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); // assert - expect(actualDirectory).to.equal(expectedDirectory); + expectTrue(success); + expect(directoryAbsolutePath).to.equal(expectedDirectory); }); }); describe('directory creation', () => { - it('creates directory recursively', async () => { + it('creates directory with recursion', async () => { // arrange const expectedIsRecursive = true; const filesystem = new FileSystemOpsStub(); @@ -86,48 +90,100 @@ describe('PersistentDirectoryProvider', () => { .withSystem(new SystemOperationsStub().withFileSystem(filesystem)); // act - const expectedDir = await context.createDirectory(); + const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); // assert + expectTrue(success); const calls = filesystem.callHistory.filter((call) => call.methodName === 'createDirectory'); expect(calls.length).to.equal(1); const [actualPath, actualIsRecursive] = calls[0].args; - expect(actualPath).to.equal(expectedDir); + expect(actualPath).to.equal(directoryAbsolutePath); expect(actualIsRecursive).to.equal(expectedIsRecursive); }); - it('logs error when creation fails', async () => { - // arrange - const logger = new LoggerStub(); - const filesystem = new FileSystemOpsStub(); - filesystem.createDirectory = () => { throw new Error(); }; - const context = new PersistentDirectoryProviderTestSetup() - .withLogger(logger) - .withSystem(new SystemOperationsStub().withFileSystem(filesystem)); + }); + describe('error handling', () => { + const testScenarios: ReadonlyArray<{ + readonly description: string; + readonly expectedErrorType: CodeRunErrorType; + readonly expectedErrorMessage: string; + buildFaultyContext( + setup: PersistentDirectoryProviderTestSetup, + errorMessage: string, + ): PersistentDirectoryProviderTestSetup; + }> = [ + { + description: 'path combination failure', + expectedErrorType: 'DirectoryCreationError', + expectedErrorMessage: 'Error when combining paths', + buildFaultyContext: (setup, errorMessage) => { + const locationStub = new LocationOpsStub(); + locationStub.combinePaths = () => { + throw new Error(errorMessage); + }; + return setup.withSystem(new SystemOperationsStub().withLocation(locationStub)); + }, + }, + { + description: 'user data retrieval failure', + expectedErrorType: 'DirectoryCreationError', + expectedErrorMessage: 'Error when locating user data directory', + buildFaultyContext: (setup, errorMessage) => { + const operatingSystemStub = new OperatingSystemOpsStub(); + operatingSystemStub.getUserDataDirectory = () => { + throw new Error(errorMessage); + }; + return setup.withSystem( + new SystemOperationsStub().withOperatingSystem(operatingSystemStub), + ); + }, + }, + { + description: 'directory creation failure', + expectedErrorType: 'DirectoryCreationError', + expectedErrorMessage: 'Error when creating directory', + buildFaultyContext: (setup, errorMessage) => { + const fileSystemStub = new FileSystemOpsStub(); + fileSystemStub.createDirectory = () => { + throw new Error(errorMessage); + }; + return setup.withSystem(new SystemOperationsStub().withFileSystem(fileSystemStub)); + }, + }, + ]; + testScenarios.forEach(({ + description, expectedErrorType, expectedErrorMessage, buildFaultyContext, + }) => { + it(`handles error - ${description}`, async () => { + // arrange + const context = buildFaultyContext( + new PersistentDirectoryProviderTestSetup(), + expectedErrorMessage, + ); - // act - try { - await context.createDirectory(); - } catch { - // swallow - } + // act + const { success, error } = await context.provideScriptDirectory(); - // assert - const errorCall = logger.callHistory.find((c) => c.methodName === 'error'); - expectExists(errorCall); - }); - it('throws error on creation failure', async () => { - // arrange - const expectedError = 'expected file system error'; - const filesystem = new FileSystemOpsStub(); - filesystem.createDirectory = () => { throw new Error(expectedError); }; - const context = new PersistentDirectoryProviderTestSetup() - .withSystem(new SystemOperationsStub().withFileSystem(filesystem)); + // assert + expect(success).to.equal(false); + expectExists(error); + expect(error.message).to.include(expectedErrorMessage); + expect(error.type).to.equal(expectedErrorType); + }); + it(`logs error: ${description}`, async () => { + // arrange + const loggerStub = new LoggerStub(); + const context = buildFaultyContext( + new PersistentDirectoryProviderTestSetup() + .withLogger(loggerStub), + expectedErrorMessage, + ); - // act - const act = () => context.createDirectory(); + // act + await context.provideScriptDirectory(); - // assert - await expectThrowsAsync(act, expectedError); + // assert + loggerStub.assertLogsContainMessagePart('error', expectedErrorMessage); + }); }); }); }); @@ -148,7 +204,7 @@ class PersistentDirectoryProviderTestSetup { return this; } - public createDirectory(): ReturnType { + public provideScriptDirectory(): ReturnType { const provider = new PersistentDirectoryProvider(this.system, this.logger); return provider.provideScriptDirectory(); } diff --git a/tests/unit/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.spec.ts b/tests/unit/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.spec.ts index 8ed362cc..ff2a3c21 100644 --- a/tests/unit/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.spec.ts +++ b/tests/unit/infrastructure/CodeRunner/Creation/Filename/TimestampedFilenameGenerator.spec.ts @@ -38,7 +38,7 @@ describe('TimestampedFilenameGenerator', () => { const filename = generateFilenamePartsForTesting({ date }); // assert expect(filename.timestamp).to.equal(expectedTimestamp, formatAssertionMessage[ - `Generated file name: ${filename.generatedFilename}` + `Generated filename: ${filename.generatedFilename}` ]); }); describe('extension', () => { @@ -49,7 +49,7 @@ describe('TimestampedFilenameGenerator', () => { const filename = generateFilenamePartsForTesting({ extension: expectedExtension }); // assert expect(filename.extension).to.equal(expectedExtension, formatAssertionMessage[ - `Generated file name: ${filename.generatedFilename}` + `Generated filename: ${filename.generatedFilename}` ]); }); describe('handles absent extension', () => { diff --git a/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts b/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts index fdf67a2e..e79c414e 100644 --- a/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts +++ b/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts @@ -11,19 +11,21 @@ import { FilenameGeneratorStub } from '@tests/unit/shared/Stubs/FilenameGenerato import { SystemOperationsStub } from '@tests/unit/shared/Stubs/SystemOperationsStub'; import { SystemOperations } from '@/infrastructure/CodeRunner/System/SystemOperations'; import { LocationOpsStub } from '@tests/unit/shared/Stubs/LocationOpsStub'; -import { ScriptFileNameParts } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator'; +import { ScriptFilenameParts } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator'; import { expectExists } from '@tests/shared/Assertions/ExpectExists'; +import { expectTrue } from '@tests/shared/Assertions/ExpectTrue'; +import { CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; describe('ScriptFileCreationOrchestrator', () => { describe('createScriptFile', () => { describe('path generation', () => { - it('generates correct directory path', async () => { + it('correctly generates directory path', async () => { // arrange const pathSegmentSeparator = '/PATH-SEGMENT-SEPARATOR/'; const expectedScriptDirectory = '/expected-script-directory'; const filesystem = new FileSystemOpsStub(); - const context = new ScriptFileCreationOrchestratorTestSetup() - .withSystemOperations(new SystemOperationsStub() + const context = new ScriptFileCreatorTestSetup() + .withSystem(new SystemOperationsStub() .withLocation( new LocationOpsStub().withDefaultSeparator(pathSegmentSeparator), ) @@ -33,45 +35,47 @@ describe('ScriptFileCreationOrchestrator', () => { ); // act - const actualFilePath = await context.createScriptFile(); + const { success, scriptFileAbsolutePath } = await context.createScriptFile(); // assert - const actualDirectory = actualFilePath + expectTrue(success); + const actualDirectory = scriptFileAbsolutePath .split(pathSegmentSeparator) .slice(0, -1) .join(pathSegmentSeparator); expect(actualDirectory).to.equal(expectedScriptDirectory, formatAssertionMessage([ - `Actual file path: ${actualFilePath}`, + `Actual file path: ${scriptFileAbsolutePath}`, ])); }); - it('generates correct file name', async () => { + it('correctly generates filename', async () => { // arrange const pathSegmentSeparator = '/PATH-SEGMENT-SEPARATOR/'; const filesystem = new FileSystemOpsStub(); const expectedFilename = 'expected-script-file-name'; - const context = new ScriptFileCreationOrchestratorTestSetup() + const context = new ScriptFileCreatorTestSetup() .withFilenameGenerator(new FilenameGeneratorStub().withFilename(expectedFilename)) - .withSystemOperations(new SystemOperationsStub() + .withSystem(new SystemOperationsStub() .withFileSystem(filesystem) .withLocation(new LocationOpsStub().withDefaultSeparator(pathSegmentSeparator))); // act - const actualFilePath = await context.createScriptFile(); + const { success, scriptFileAbsolutePath } = await context.createScriptFile(); // assert - const actualFileName = actualFilePath + expectTrue(success); + const actualFileName = scriptFileAbsolutePath .split(pathSegmentSeparator) .pop(); expect(actualFileName).to.equal(expectedFilename); }); - it('generates file name using specified parts', async () => { + it('uses specified parts to generate filename', async () => { // arrange - const expectedParts: ScriptFileNameParts = { + const expectedParts: ScriptFilenameParts = { scriptName: 'expected-script-name', scriptFileExtension: 'expected-script-file-extension', }; const filenameGeneratorStub = new FilenameGeneratorStub(); - const context = new ScriptFileCreationOrchestratorTestSetup() + const context = new ScriptFileCreatorTestSetup() .withFileNameParts(expectedParts) .withFilenameGenerator(filenameGeneratorStub); @@ -79,58 +83,60 @@ describe('ScriptFileCreationOrchestrator', () => { await context.createScriptFile(); // assert - const fileNameGenerationCalls = filenameGeneratorStub.callHistory.filter((c) => c.methodName === 'generateFilename'); - expect(fileNameGenerationCalls).to.have.lengthOf(1); - const callArguments = fileNameGenerationCalls[0].args; + const filenameGenerationCalls = filenameGeneratorStub.callHistory.filter((c) => c.methodName === 'generateFilename'); + expect(filenameGenerationCalls).to.have.lengthOf(1); + const callArguments = filenameGenerationCalls[0].args; const [scriptNameFileParts] = callArguments; expectExists(scriptNameFileParts, `Call arguments: ${JSON.stringify(callArguments)}`); expect(scriptNameFileParts).to.equal(expectedParts); }); - it('generates complete file path', async () => { + it('correctly generates complete file path', async () => { // arrange const expectedPath = 'expected-script-path'; - const fileName = 'file-name'; + const filename = 'filename'; const directoryPath = 'directory-path'; const filesystem = new FileSystemOpsStub(); - const context = new ScriptFileCreationOrchestratorTestSetup() - .withFilenameGenerator(new FilenameGeneratorStub().withFilename(fileName)) + const context = new ScriptFileCreatorTestSetup() + .withFilenameGenerator(new FilenameGeneratorStub().withFilename(filename)) .withDirectoryProvider(new ScriptDirectoryProviderStub().withDirectoryPath(directoryPath)) - .withSystemOperations(new SystemOperationsStub() + .withSystem(new SystemOperationsStub() .withFileSystem(filesystem) .withLocation( - new LocationOpsStub().withJoinResult(expectedPath, directoryPath, fileName), + new LocationOpsStub().withJoinResult(expectedPath, directoryPath, filename), )); // act - const actualFilePath = await context.createScriptFile(); + const { success, scriptFileAbsolutePath } = await context.createScriptFile(); // assert - expect(actualFilePath).to.equal(expectedPath); + expectTrue(success); + expect(scriptFileAbsolutePath).to.equal(expectedPath); }); }); describe('file writing', () => { - it('writes file to the generated path', async () => { + it('writes to generated file path', async () => { // arrange const filesystem = new FileSystemOpsStub(); - const context = new ScriptFileCreationOrchestratorTestSetup() - .withSystemOperations(new SystemOperationsStub() + const context = new ScriptFileCreatorTestSetup() + .withSystem(new SystemOperationsStub() .withFileSystem(filesystem)); // act - const expectedPath = await context.createScriptFile(); + const { success, scriptFileAbsolutePath } = await context.createScriptFile(); // assert + expectTrue(success); const calls = filesystem.callHistory.filter((call) => call.methodName === 'writeToFile'); expect(calls.length).to.equal(1); const [actualFilePath] = calls[0].args; - expect(actualFilePath).to.equal(expectedPath); + expect(actualFilePath).to.equal(scriptFileAbsolutePath); }); - it('writes provided script content to file', async () => { + it('writes script content to file', async () => { // arrange const expectedCode = 'expected-code'; const filesystem = new FileSystemOpsStub(); - const context = new ScriptFileCreationOrchestratorTestSetup() - .withSystemOperations(new SystemOperationsStub().withFileSystem(filesystem)) + const context = new ScriptFileCreatorTestSetup() + .withSystem(new SystemOperationsStub().withFileSystem(filesystem)) .withFileContents(expectedCode); // act @@ -143,10 +149,119 @@ describe('ScriptFileCreationOrchestrator', () => { expect(actualData).to.equal(expectedCode); }); }); + describe('error handling', () => { + const testScenarios: ReadonlyArray<{ + readonly description: string; + readonly expectedErrorType: CodeRunErrorType; + readonly expectedErrorMessage: string; + readonly expectLogs: boolean; + buildFaultyContext( + setup: ScriptFileCreatorTestSetup, + errorMessage: string, + errorType: CodeRunErrorType, + ): ScriptFileCreatorTestSetup; + }> = [ + { + description: 'path combination failure', + expectedErrorType: 'FilePathGenerationError', + expectedErrorMessage: 'Error when combining paths', + expectLogs: true, + buildFaultyContext: (setup, errorMessage) => { + const locationStub = new LocationOpsStub(); + locationStub.combinePaths = () => { + throw new Error(errorMessage); + }; + return setup.withSystem(new SystemOperationsStub().withLocation(locationStub)); + }, + }, + { + description: 'file writing failure', + expectedErrorType: 'FileWriteError', + expectedErrorMessage: 'Error when writing to file', + expectLogs: true, + buildFaultyContext: (setup, errorMessage) => { + const fileSystemStub = new FileSystemOpsStub(); + fileSystemStub.writeToFile = () => { + throw new Error(errorMessage); + }; + return setup.withSystem(new SystemOperationsStub().withFileSystem(fileSystemStub)); + }, + }, + { + description: 'filename generation failure', + expectedErrorType: 'FilePathGenerationError', + expectedErrorMessage: 'Error when writing to file', + expectLogs: true, + buildFaultyContext: (setup, errorMessage) => { + const filenameGenerator = new FilenameGeneratorStub(); + filenameGenerator.generateFilename = () => { + throw new Error(errorMessage); + }; + return setup.withFilenameGenerator(filenameGenerator); + }, + }, + { + description: 'script directory provision failure', + expectedErrorType: 'DirectoryCreationError', + expectedErrorMessage: 'Error when providing directory', + expectLogs: false, + buildFaultyContext: (setup, errorMessage, errorType) => { + const directoryProvider = new ScriptDirectoryProviderStub(); + directoryProvider.provideScriptDirectory = () => Promise.resolve({ + success: false, + error: { + message: errorMessage, + type: errorType, + }, + }); + return setup.withDirectoryProvider(directoryProvider); + }, + }, + ]; + testScenarios.forEach(({ + description, expectedErrorType, expectedErrorMessage, buildFaultyContext, expectLogs, + }) => { + it(`handles error - ${description}`, async () => { + // arrange + const context = buildFaultyContext( + new ScriptFileCreatorTestSetup(), + expectedErrorMessage, + expectedErrorType, + ); + + // act + const { success, error } = await context.createScriptFile(); + + // assert + expect(success).to.equal(false); + expectExists(error); + expect(error.message).to.include(expectedErrorMessage); + expect(error.type).to.equal(expectedErrorType); + }); + if (expectLogs) { + it(`logs error: ${description}`, async () => { + // arrange + const loggerStub = new LoggerStub(); + const context = buildFaultyContext( + new ScriptFileCreatorTestSetup() + .withLogger(loggerStub), + expectedErrorMessage, + expectedErrorType, + ); + + // act + await context.createScriptFile(); + + // assert + loggerStub.assertLogsContainMessagePart('error', expectedErrorMessage); + }); + } + }); + }); }); }); -class ScriptFileCreationOrchestratorTestSetup { +class ScriptFileCreatorTestSetup { private system: SystemOperations = new SystemOperationsStub(); private filenameGenerator: FilenameGenerator = new FilenameGeneratorStub(); @@ -155,11 +270,11 @@ class ScriptFileCreationOrchestratorTestSetup { private logger: Logger = new LoggerStub(); - private fileContents = `[${ScriptFileCreationOrchestratorTestSetup.name}] script file contents`; + private fileContents = `[${ScriptFileCreatorTestSetup.name}] script file contents`; - private fileNameParts: ScriptFileNameParts = { - scriptName: `[${ScriptFileCreationOrchestratorTestSetup.name}] script name`, - scriptFileExtension: `[${ScriptFileCreationOrchestratorTestSetup.name}] file extension`, + private filenameParts: ScriptFilenameParts = { + scriptName: `[${ScriptFileCreatorTestSetup.name}] script name`, + scriptFileExtension: `[${ScriptFileCreatorTestSetup.name}] file extension`, }; public withFileContents(fileContents: string): this { @@ -177,13 +292,18 @@ class ScriptFileCreationOrchestratorTestSetup { return this; } - public withSystemOperations(system: SystemOperations): this { + public withSystem(system: SystemOperations): this { this.system = system; return this; } - public withFileNameParts(fileNameParts: ScriptFileNameParts): this { - this.fileNameParts = fileNameParts; + public withFileNameParts(filenameParts: ScriptFilenameParts): this { + this.filenameParts = filenameParts; + return this; + } + + public withLogger(logger: Logger): this { + this.logger = logger; return this; } @@ -194,6 +314,6 @@ class ScriptFileCreationOrchestratorTestSetup { this.directoryProvider, this.logger, ); - return creator.createScriptFile(this.fileContents, this.fileNameParts); + return creator.createScriptFile(this.fileContents, this.filenameParts); } } diff --git a/tests/unit/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.spec.ts b/tests/unit/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.spec.ts index 6daec82b..6418b811 100644 --- a/tests/unit/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.spec.ts +++ b/tests/unit/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor.spec.ts @@ -1,5 +1,4 @@ import { describe, it, expect } from 'vitest'; -import { expectThrowsAsync } from '@tests/shared/Assertions/ExpectThrowsAsync'; import { OperatingSystem } from '@/domain/OperatingSystem'; import { AllSupportedOperatingSystems, SupportedOperatingSystem } from '@tests/shared/TestCases/SupportedOperatingSystems'; import { VisibleTerminalScriptExecutor } from '@/infrastructure/CodeRunner/Execution/VisibleTerminalScriptFileExecutor'; @@ -9,41 +8,12 @@ import { SystemOperationsStub } from '@tests/unit/shared/Stubs/SystemOperationsS import { CommandOpsStub } from '@tests/unit/shared/Stubs/CommandOpsStub'; import { SystemOperations } from '@/infrastructure/CodeRunner/System/SystemOperations'; import { FileSystemOpsStub } from '@tests/unit/shared/Stubs/FileSystemOpsStub'; +import { CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; +import { Logger } from '@/application/Common/Log/Logger'; +import { expectExists } from '@tests/shared/Assertions/ExpectExists'; describe('VisibleTerminalScriptFileExecutor', () => { describe('executeScriptFile', () => { - describe('throws error for invalid operating systems', () => { - const testScenarios: ReadonlyArray<{ - readonly description: string; - readonly invalidOs?: OperatingSystem; - readonly expectedError: string; - }> = [ - (() => { - const unsupportedOs = OperatingSystem.Android; - return { - description: 'unsupported OS', - invalidOs: unsupportedOs, - expectedError: `Unsupported operating system: ${OperatingSystem[unsupportedOs]}`, - }; - })(), - { - description: 'undefined OS', - invalidOs: undefined, - expectedError: 'Unknown operating system', - }, - ]; - testScenarios.forEach(({ description, invalidOs, expectedError }) => { - it(description, async () => { - // arrange - const context = new ScriptFileTestSetup() - .withOs(invalidOs); - // act - const act = async () => { await context.executeScriptFile(); }; - // assert - await expectThrowsAsync(act, expectedError); - }); - }); - }); describe('command execution', () => { // arrange const testScenarios: Record { testScenarios[operatingSystem].forEach(( { description, filePath, expectedCommand }, ) => { - it(description, async () => { + it(`executes command - ${description}`, async () => { // arrange const command = new CommandOpsStub(); - const context = new ScriptFileTestSetup() + const context = new ScriptFileExecutorTestSetup() .withOs(operatingSystem) .withFilePath(filePath) .withSystemOperations(new SystemOperationsStub().withCommand(command)); @@ -124,7 +94,7 @@ describe('VisibleTerminalScriptFileExecutor', () => { isExecutedAfterPermissions = isPermissionsSet; return Promise.resolve(); }; - const context = new ScriptFileTestSetup() + const context = new ScriptFileExecutorTestSetup() .withSystemOperations(new SystemOperationsStub() .withFileSystem(fileSystemMock) .withCommand(commandMock)); @@ -139,7 +109,7 @@ describe('VisibleTerminalScriptFileExecutor', () => { // arrange const expectedMode = '755'; const fileSystem = new FileSystemOpsStub(); - const context = new ScriptFileTestSetup() + const context = new ScriptFileExecutorTestSetup() .withSystemOperations(new SystemOperationsStub().withFileSystem(fileSystem)); // act @@ -151,11 +121,11 @@ describe('VisibleTerminalScriptFileExecutor', () => { const [, actualMode] = calls[0].args; expect(actualMode).to.equal(expectedMode); }); - it('sets permissions on the correct file', async () => { + it('sets permissions for correct file', async () => { // arrange const expectedFilePath = 'expected-file-path'; const fileSystem = new FileSystemOpsStub(); - const context = new ScriptFileTestSetup() + const context = new ScriptFileExecutorTestSetup() .withFilePath(expectedFilePath) .withSystemOperations(new SystemOperationsStub().withFileSystem(fileSystem)); @@ -169,16 +139,121 @@ describe('VisibleTerminalScriptFileExecutor', () => { expect(actualFilePath).to.equal(expectedFilePath); }); }); + it('indicates success on successful execution', async () => { + // arrange + const expectedSuccessResult = true; + const context = new ScriptFileExecutorTestSetup(); + + // act + const { success: actualSuccessValue } = await context.executeScriptFile(); + + // assert + expect(actualSuccessValue).to.equal(expectedSuccessResult); + }); + describe('error handling', () => { + const testScenarios: ReadonlyArray<{ + readonly description: string; + readonly expectedErrorType: CodeRunErrorType; + readonly expectedErrorMessage: string; + buildFaultyContext( + setup: ScriptFileExecutorTestSetup, + errorMessage: string, + ): ScriptFileExecutorTestSetup; + }> = [ + { + description: 'unindentified os', + expectedErrorType: 'UnsupportedOperatingSystem', + expectedErrorMessage: 'Operating system could not be identified from environment', + buildFaultyContext: (setup) => { + return setup.withOs(undefined); + }, + }, + { + description: 'unsupported OS', + expectedErrorType: 'UnsupportedOperatingSystem', + expectedErrorMessage: `Unsupported operating system: ${OperatingSystem[OperatingSystem.Android]}`, + buildFaultyContext: (setup) => { + return setup.withOs(OperatingSystem.Android); + }, + }, + { + description: 'file permissions failure', + expectedErrorType: 'FileExecutionError', + expectedErrorMessage: 'Error when setting file permissions', + buildFaultyContext: (setup, errorMessage) => { + const fileSystem = new FileSystemOpsStub(); + fileSystem.setFilePermissions = () => Promise.reject(errorMessage); + return setup.withSystemOperations( + new SystemOperationsStub().withFileSystem(fileSystem), + ); + }, + }, + { + description: 'command failure', + expectedErrorType: 'FileExecutionError', + expectedErrorMessage: 'Error when setting file permissions', + buildFaultyContext: (setup, errorMessage) => { + const command = new CommandOpsStub(); + command.exec = () => Promise.reject(errorMessage); + return setup.withSystemOperations( + new SystemOperationsStub().withCommand(command), + ); + }, + }, + ]; + testScenarios.forEach(({ + description, expectedErrorType, expectedErrorMessage, buildFaultyContext, + }) => { + it(`handles error - ${description}`, async () => { + // arrange + const context = buildFaultyContext( + new ScriptFileExecutorTestSetup(), + expectedErrorMessage, + ); + + // act + const { success, error } = await context.executeScriptFile(); + + // assert + expect(success).to.equal(false); + expectExists(error); + expect(error.message).to.include(expectedErrorMessage); + expect(error.type).to.equal(expectedErrorType); + }); + it(`logs error - ${description}`, async () => { + // arrange + const loggerStub = new LoggerStub(); + const context = buildFaultyContext( + new ScriptFileExecutorTestSetup() + .withLogger(loggerStub), + expectedErrorMessage, + ); + + // act + await context.executeScriptFile(); + + // assert + loggerStub.assertLogsContainMessagePart('error', expectedErrorMessage); + }); + }); + }); }); }); -class ScriptFileTestSetup { +class ScriptFileExecutorTestSetup { private os?: OperatingSystem = OperatingSystem.Windows; - private filePath = `[${ScriptFileTestSetup.name}] file path`; + private filePath = `[${ScriptFileExecutorTestSetup.name}] file path`; private system: SystemOperations = new SystemOperationsStub(); + private logger: Logger = new LoggerStub(); + + public withLogger(logger: Logger): this { + this.logger = logger; + return this; + } + public withOs(os: OperatingSystem | undefined): this { this.os = os; return this; @@ -194,10 +269,9 @@ class ScriptFileTestSetup { return this; } - public executeScriptFile(): Promise { + public executeScriptFile() { const environment = new RuntimeEnvironmentStub().withOs(this.os); - const logger = new LoggerStub(); - const executor = new VisibleTerminalScriptExecutor(this.system, logger, environment); + const executor = new VisibleTerminalScriptExecutor(this.system, this.logger, environment); return executor.executeScriptFile(this.filePath); } } diff --git a/tests/unit/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts b/tests/unit/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts index c230fba5..81e45236 100644 --- a/tests/unit/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts +++ b/tests/unit/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts @@ -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 { + 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 { diff --git a/tests/unit/infrastructure/Dialog/Browser/BrowserDialog.spec.ts b/tests/unit/infrastructure/Dialog/Browser/BrowserDialog.spec.ts index 74e5aae5..eb9f1fa6 100644 --- a/tests/unit/infrastructure/Dialog/Browser/BrowserDialog.spec.ts +++ b/tests/unit/infrastructure/Dialog/Browser/BrowserDialog.spec.ts @@ -1,31 +1,108 @@ import { describe, it, expect } from 'vitest'; -import { FileType } from '@/presentation/common/Dialog'; -import { BrowserDialog } from '@/infrastructure/Dialog/Browser/BrowserDialog'; +import { FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; +import { BrowserDialog, WindowDialogAccessor } from '@/infrastructure/Dialog/Browser/BrowserDialog'; import { BrowserSaveFileDialog } from '@/infrastructure/Dialog/Browser/BrowserSaveFileDialog'; +import { expectExists } from '@tests/shared/Assertions/ExpectExists'; describe('BrowserDialog', () => { describe('saveFile', () => { - it('passes correct arguments', () => { + it('forwards arguments', async () => { // arrange - const expectedFileContents = 'test content'; - const expectedFileName = 'test.sh'; - const expectedFileType = FileType.ShellScript; + const expectedSaveFileArgs = createTestSaveFileArguments(); let actualSaveFileArgs: Parameters | undefined; const fileSaverDialogSpy: BrowserSaveFileDialog = { saveFile: (...args) => { actualSaveFileArgs = args; + return { success: true }; }, }; - const browserDialog = new BrowserDialog(fileSaverDialogSpy); + const browserDialog = new BrowserDialogBuilder() + .withBrowserSaveFileDialog(fileSaverDialogSpy) + .build(); // act - browserDialog.saveFile(expectedFileContents, expectedFileName, expectedFileType); + await browserDialog.saveFile(...expectedSaveFileArgs); // assert - expect(actualSaveFileArgs) - .to - .deep - .equal([expectedFileContents, expectedFileName, expectedFileType]); + expect(actualSaveFileArgs).to.deep.equal(expectedSaveFileArgs); + }); + it('forwards outcome', async () => { + // arrange + const expectedResult: SaveFileOutcome = { + success: true, + }; + const fileSaverDialogMock: BrowserSaveFileDialog = { + saveFile: () => expectedResult, + }; + const browserDialog = new BrowserDialogBuilder() + .withBrowserSaveFileDialog(fileSaverDialogMock) + .build(); + + // act + const actualResult = await browserDialog.saveFile(...createTestSaveFileArguments()); + + // assert + expect(actualResult).to.equal(expectedResult); + }); + }); + describe('showError', () => { + it('alerts with formatted error message', () => { + // arrange + const errorTitle = 'Expected Error Title'; + const errorMessage = 'expected error message'; + const expectedMessage = `❌ ${errorTitle}\n\n${errorMessage}`; + let actualShowErrorArgs: Parameters | undefined; + const windowDialogAccessorSpy: WindowDialogAccessor = { + alert: (...args) => { + actualShowErrorArgs = args; + }, + }; + const browserDialog = new BrowserDialogBuilder() + .withWindowDialogAccessor(windowDialogAccessorSpy) + .build(); + + // act + browserDialog.showError(errorTitle, errorMessage); + + // assert + expectExists(actualShowErrorArgs); + const [actualMessage] = actualShowErrorArgs; + expect(actualMessage).to.equal(expectedMessage); }); }); }); + +function createTestSaveFileArguments(): Parameters { + return [ + 'test file content', + 'test filename', + FileType.ShellScript, + ]; +} + +class BrowserDialogBuilder { + private browserSaveFileDialog: BrowserSaveFileDialog = { + saveFile: () => ({ success: true }), + }; + + private windowDialogAccessor: WindowDialogAccessor = { + alert: () => { /* NOOP */ }, + }; + + public withBrowserSaveFileDialog(browserSaveFileDialog: BrowserSaveFileDialog): this { + this.browserSaveFileDialog = browserSaveFileDialog; + return this; + } + + public withWindowDialogAccessor(windowDialogAccessor: WindowDialogAccessor): this { + this.windowDialogAccessor = windowDialogAccessor; + return this; + } + + public build() { + return new BrowserDialog( + this.windowDialogAccessor, + this.browserSaveFileDialog, + ); + } +} diff --git a/tests/unit/infrastructure/Dialog/Browser/FileSaverDialog.spec.ts b/tests/unit/infrastructure/Dialog/Browser/FileSaverDialog.spec.ts index 2be9ae1a..6b8135af 100644 --- a/tests/unit/infrastructure/Dialog/Browser/FileSaverDialog.spec.ts +++ b/tests/unit/infrastructure/Dialog/Browser/FileSaverDialog.spec.ts @@ -88,7 +88,7 @@ class SaveFileTestSetup { private fileContents: string = `${SaveFileTestSetup.name} file contents`; - private fileName: string = `${SaveFileTestSetup.name} file name`; + private filename: string = `${SaveFileTestSetup.name} filename`; private fileType: FileType = FileType.BatchFile; @@ -119,7 +119,7 @@ class SaveFileTestSetup { ); return dialog.saveFile( this.fileContents, - this.fileName, + this.filename, this.fileType, ); } diff --git a/tests/unit/infrastructure/Dialog/Electron/ElectronDialog.spec.ts b/tests/unit/infrastructure/Dialog/Electron/ElectronDialog.spec.ts index 96a690fa..9ee9da28 100644 --- a/tests/unit/infrastructure/Dialog/Electron/ElectronDialog.spec.ts +++ b/tests/unit/infrastructure/Dialog/Electron/ElectronDialog.spec.ts @@ -1,32 +1,104 @@ import { describe, it, expect } from 'vitest'; -import { FileType } from '@/presentation/common/Dialog'; -import { BrowserDialog } from '@/infrastructure/Dialog/Browser/BrowserDialog'; +import { FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; import { ElectronSaveFileDialog } from '@/infrastructure/Dialog/Electron/ElectronSaveFileDialog'; +import { ElectronDialog, ElectronDialogAccessor } from '@/infrastructure/Dialog/Electron/ElectronDialog'; -describe('BrowserDialog', () => { +describe('ElectronDialog', () => { describe('saveFile', () => { - it('passes correct arguments', async () => { + it('forwards arguments', async () => { // arrange - const expectedFileContents = 'test content'; - const expectedFileName = 'test.sh'; - const expectedFileType = FileType.ShellScript; + const expectedSaveFileArgs = createTestSaveFileArguments(); let actualSaveFileArgs: Parameters | undefined; const fileSaverDialogSpy: ElectronSaveFileDialog = { saveFile: (...args) => { actualSaveFileArgs = args; - return Promise.resolve(); + return Promise.resolve({ + success: true, + }); }, }; - const browserDialog = new BrowserDialog(fileSaverDialogSpy); + const electronDialog = new ElectronDialogBuilder() + .withSaveFileDialog(fileSaverDialogSpy) + .build(); // act - await browserDialog.saveFile(expectedFileContents, expectedFileName, expectedFileType); + await electronDialog.saveFile(...expectedSaveFileArgs); // assert - expect(actualSaveFileArgs) - .to - .deep - .equal([expectedFileContents, expectedFileName, expectedFileType]); + expect(actualSaveFileArgs).to.deep.equal(expectedSaveFileArgs); + }); + it('forwards outcome', async () => { + // arrange + const expectedResult: SaveFileOutcome = { + success: true, + }; + const fileSaverDialogMock: ElectronSaveFileDialog = { + saveFile: () => Promise.resolve(expectedResult), + }; + const browserDialog = new ElectronDialogBuilder() + .withSaveFileDialog(fileSaverDialogMock) + .build(); + + // act + const actualResult = await browserDialog.saveFile(...createTestSaveFileArguments()); + + // assert + expect(actualResult).to.equal(expectedResult); + }); + }); + describe('showError', () => { + it('forwards arguments', () => { + // arrange + const expectedShowErrorArguments: Parameters = [ + 'test title', 'test message', + ]; + let actualShowErrorArgs: Parameters | undefined; + const electronDialogAccessorSpy: ElectronDialogAccessor = { + showErrorBox: (...args) => { + actualShowErrorArgs = args; + }, + }; + const electronDialog = new ElectronDialogBuilder() + .withElectron(electronDialogAccessorSpy) + .build(); + + // act + electronDialog.showError(...expectedShowErrorArguments); + + // assert + expect(actualShowErrorArgs).to.deep.equal(expectedShowErrorArguments); }); }); }); + +function createTestSaveFileArguments(): Parameters { + return [ + 'test file content', + 'test filename', + FileType.ShellScript, + ]; +} + +class ElectronDialogBuilder { + private electron: ElectronDialogAccessor = { + showErrorBox: () => {}, + }; + + private saveFileDialog: ElectronSaveFileDialog = { + saveFile: () => Promise.resolve({ success: true }), + }; + + public withElectron(electron: ElectronDialogAccessor): this { + this.electron = electron; + return this; + } + + public withSaveFileDialog(saveFileDialog: ElectronSaveFileDialog): this { + this.saveFileDialog = saveFileDialog; + return this; + } + + public build() { + return new ElectronDialog(this.saveFileDialog, this.electron); + } +} diff --git a/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts b/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts index 2619729c..01d843a8 100644 --- a/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts +++ b/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { FileType } from '@/presentation/common/Dialog'; +import { FileType, SaveFileErrorType } from '@/presentation/common/Dialog'; import { ElectronFileDialogOperations, NodeElectronSaveFileDialog, NodeFileOperations } from '@/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog'; import { Logger } from '@/application/Common/Log/Logger'; import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; @@ -8,14 +8,14 @@ import { ElectronFileDialogOperationsStub } from './ElectronFileDialogOperations import { NodeFileOperationsStub } from './NodeFileOperationsStub'; describe('NodeElectronSaveFileDialog', () => { - describe('shows dialog with correct options', () => { + describe('dialog options', () => { it('correct title', async () => { // arrange const expectedFileName = 'expected-file-name'; const electronMock = new ElectronFileDialogOperationsStub(); const context = new SaveFileDialogTestSetup() .withElectron(electronMock) - .withFileName(expectedFileName); + .withDefaultFilename(expectedFileName); // act await context.saveFile(); // assert @@ -52,7 +52,7 @@ describe('NodeElectronSaveFileDialog', () => { .withUserDownloadsPath(expectedParentDirectory); const context = new SaveFileDialogTestSetup() .withElectron(electronMock) - .withFileName(expectedFileName) + .withDefaultFilename(expectedFileName) .withNode(new NodeFileOperationsStub().withPathSegmentSeparator(pathSegmentSeparator)); // act await context.saveFile(); @@ -63,7 +63,7 @@ describe('NodeElectronSaveFileDialog', () => { electronMock, ); }); - describe('correct filters', () => { + describe('correct file type filters', () => { const defaultFilter: Electron.FileFilter = { name: 'All Files', extensions: ['*'], @@ -109,92 +109,224 @@ describe('NodeElectronSaveFileDialog', () => { }); }); - describe('saves the file when the dialog is not canceled', () => { - it('writes to the selected file path', async () => { - // arrange - const expectedFilePath = 'expected-file-path'; - const isCancelled = false; - const electronMock = new ElectronFileDialogOperationsStub() - .withMimicUserCancel(isCancelled) - .withUserSelectedFilePath(expectedFilePath); - const nodeMock = new NodeFileOperationsStub(); - const context = new SaveFileDialogTestSetup() - .withElectron(electronMock) - .withNode(nodeMock); + describe('file saving process', () => { + describe('when dialog is confirmed', () => { + it('writes to the selected file path', async () => { + // arrange + const expectedFilePath = 'expected-file-path'; + const isCancelled = false; + const electronMock = new ElectronFileDialogOperationsStub() + .withMimicUserCancel(isCancelled) + .withUserSelectedFilePath(expectedFilePath); + const nodeMock = new NodeFileOperationsStub(); + const context = new SaveFileDialogTestSetup() + .withElectron(electronMock) + .withNode(nodeMock); - // act - await context.saveFile(); + // act + await context.saveFile(); - // assert - const saveFileCalls = nodeMock.callHistory.filter((c) => c.methodName === 'writeFile'); - expect(saveFileCalls).to.have.lengthOf(1); - const [actualFilePath] = saveFileCalls[0].args; - expect(actualFilePath).to.equal(expectedFilePath); + // assert + const saveFileCalls = nodeMock.callHistory.filter((c) => c.methodName === 'writeFile'); + expect(saveFileCalls).to.have.lengthOf(1); + const [actualFilePath] = saveFileCalls[0].args; + expect(actualFilePath).to.equal(expectedFilePath); + }); + it('writes the correct file contents', async () => { + // arrange + const expectedFileContents = 'expected-file-contents'; + const isCancelled = false; + const electronMock = new ElectronFileDialogOperationsStub() + .withMimicUserCancel(isCancelled); + const nodeMock = new NodeFileOperationsStub(); + const context = new SaveFileDialogTestSetup() + .withElectron(electronMock) + .withFileContents(expectedFileContents) + .withNode(nodeMock); + + // act + await context.saveFile(); + + // assert + const saveFileCalls = nodeMock.callHistory.filter((c) => c.methodName === 'writeFile'); + expect(saveFileCalls).to.have.lengthOf(1); + const [,actualFileContents] = saveFileCalls[0].args; + expect(actualFileContents).to.equal(expectedFileContents); + }); + it('returns success status', async () => { + // arrange + const expectedSuccessValue = true; + const context = new SaveFileDialogTestSetup(); + + // act + const { success } = await context.saveFile(); + + // assert + expect(success).to.equal(expectedSuccessValue); + }); }); + describe('when dialog is canceled', async () => { + it('does not save file', async () => { + // arrange + const isCancelled = true; + const electronMock = new ElectronFileDialogOperationsStub() + .withMimicUserCancel(isCancelled); + const nodeMock = new NodeFileOperationsStub(); + const context = new SaveFileDialogTestSetup() + .withElectron(electronMock) + .withNode(nodeMock); - it('writes the correct file contents', async () => { - // arrange - const expectedFileContents = 'expected-file-contents'; - const isCancelled = false; - const electronMock = new ElectronFileDialogOperationsStub() - .withMimicUserCancel(isCancelled); - const nodeMock = new NodeFileOperationsStub(); - const context = new SaveFileDialogTestSetup() - .withElectron(electronMock) - .withFileContents(expectedFileContents) - .withNode(nodeMock); + // act + await context.saveFile(); - // act - await context.saveFile(); + // assert + const saveFileCall = nodeMock.callHistory.find((c) => c.methodName === 'writeFile'); + expect(saveFileCall).to.equal(undefined); + }); + it('logs cancelation info', async () => { + // arrange + const expectedLogMessagePart = 'File save cancelled'; + const logger = new LoggerStub(); + const isCancelled = true; + const electronMock = new ElectronFileDialogOperationsStub() + .withMimicUserCancel(isCancelled); + const context = new SaveFileDialogTestSetup() + .withElectron(electronMock) + .withLogger(logger); - // assert - const saveFileCalls = nodeMock.callHistory.filter((c) => c.methodName === 'writeFile'); - expect(saveFileCalls).to.have.lengthOf(1); - const [,actualFileContents] = saveFileCalls[0].args; - expect(actualFileContents).to.equal(expectedFileContents); + // act + await context.saveFile(); + + // assert + logger.assertLogsContainMessagePart('info', expectedLogMessagePart); + }); + it('returns success', async () => { + // arrange + const expectedSuccessValue = true; + const isCancelled = true; + const electronMock = new ElectronFileDialogOperationsStub() + .withMimicUserCancel(isCancelled); + const context = new SaveFileDialogTestSetup() + .withElectron(electronMock); + + // act + const { success } = await context.saveFile(); + + // assert + expect(success).to.equal(expectedSuccessValue); + }); }); }); - it('does not save file when dialog is canceled', async () => { - // arrange - const isCancelled = true; - const electronMock = new ElectronFileDialogOperationsStub() - .withMimicUserCancel(isCancelled); - const nodeMock = new NodeFileOperationsStub(); - const context = new SaveFileDialogTestSetup() - .withElectron(electronMock) - .withNode(nodeMock); + describe('error handling', () => { + const testScenarios: ReadonlyArray<{ + readonly description: string; + readonly expectedErrorType: SaveFileErrorType; + readonly expectedErrorMessage: string; + buildFaultyContext( + setup: SaveFileDialogTestSetup, + errorMessage: string, + ): SaveFileDialogTestSetup; + }> = [ + { + description: 'file writing failure', + expectedErrorType: 'FileCreationError', + expectedErrorMessage: 'Error when writing file', + buildFaultyContext: (setup, errorMessage) => { + const electronMock = new ElectronFileDialogOperationsStub().withMimicUserCancel(false); + const nodeMock = new NodeFileOperationsStub(); + nodeMock.writeFile = () => Promise.reject(new Error(errorMessage)); + return setup + .withElectron(electronMock) + .withNode(nodeMock); + }, + }, + { + description: 'user path retrieval failure', + expectedErrorType: 'DialogDisplayError', + expectedErrorMessage: 'Error when retrieving user path', + buildFaultyContext: (setup, errorMessage) => { + const electronMock = new ElectronFileDialogOperationsStub().withMimicUserCancel(false); + electronMock.getUserDownloadsPath = () => { + throw new Error(errorMessage); + }; + return setup + .withElectron(electronMock); + }, + }, + { + description: 'path combination failure', + expectedErrorType: 'DialogDisplayError', + expectedErrorMessage: 'Error when combining paths', + buildFaultyContext: (setup, errorMessage) => { + const nodeMock = new NodeFileOperationsStub(); + nodeMock.join = () => { + throw new Error(errorMessage); + }; + return setup + .withNode(nodeMock); + }, + }, + { + description: 'dialog display failure', + expectedErrorType: 'DialogDisplayError', + expectedErrorMessage: 'Error when showing save dialog', + buildFaultyContext: (setup, errorMessage) => { + const electronMock = new ElectronFileDialogOperationsStub().withMimicUserCancel(false); + electronMock.showSaveDialog = () => Promise.reject(new Error(errorMessage)); + return setup + .withElectron(electronMock); + }, + }, + { + description: 'unexpected dialog return value failure', + expectedErrorType: 'DialogDisplayError', + expectedErrorMessage: 'Unexpected Error: File path is undefined after save dialog completion.', + buildFaultyContext: (setup) => { + const electronMock = new ElectronFileDialogOperationsStub().withMimicUserCancel(false); + electronMock.showSaveDialog = () => Promise.resolve({ + canceled: false, + filePath: undefined, + }); + return setup + .withElectron(electronMock); + }, + }, + ]; + testScenarios.forEach(({ + description, expectedErrorType, expectedErrorMessage, buildFaultyContext, + }) => { + it(`handles error - ${description}`, async () => { + // arrange + const context = buildFaultyContext( + new SaveFileDialogTestSetup(), + expectedErrorMessage, + ); - // act - await context.saveFile(); + // act + const { success, error } = await context.saveFile(); - // assert - const saveFileCall = nodeMock.callHistory.find((c) => c.methodName === 'writeFile'); - expect(saveFileCall).to.equal(undefined); - }); + // assert + expect(success).to.equal(false); + expectExists(error); + expect(error.message).to.include(expectedErrorMessage); + expect(error.type).to.equal(expectedErrorType); + }); + it(`logs error: ${description}`, async () => { + // arrange + const loggerStub = new LoggerStub(); + const context = buildFaultyContext( + new SaveFileDialogTestSetup() + .withLogger(loggerStub), + expectedErrorMessage, + ); - describe('logging', () => { - it('logs an error if writing the file fails', async () => { - // arrange - const expectedErrorMessage = 'Injected write error'; - const electronMock = new ElectronFileDialogOperationsStub().withMimicUserCancel(false); - const nodeMock = new NodeFileOperationsStub(); - nodeMock.writeFile = () => Promise.reject(new Error(expectedErrorMessage)); - const loggerStub = new LoggerStub(); - const context = new SaveFileDialogTestSetup() - .withElectron(electronMock) - .withNode(nodeMock) - .withLogger(loggerStub); + // act + await context.saveFile(); - // act - await context.saveFile(); - - // assert - const errorCalls = loggerStub.callHistory.filter((c) => c.methodName === 'error'); - expect(errorCalls.length).to.equal(1); - const errorCall = errorCalls[0]; - const [errorMessage] = errorCall.args; - expect(errorMessage).to.include(expectedErrorMessage); + // assert + loggerStub.assertLogsContainMessagePart('error', expectedErrorMessage); + }); }); }); }); @@ -202,7 +334,7 @@ describe('NodeElectronSaveFileDialog', () => { class SaveFileDialogTestSetup { private fileContents = `${SaveFileDialogTestSetup.name} file contents`; - private fileName = `${SaveFileDialogTestSetup.name} file name`; + private filename = `${SaveFileDialogTestSetup.name} filename`; private fileType = FileType.BatchFile; @@ -227,8 +359,8 @@ class SaveFileDialogTestSetup { return this; } - public withFileName(fileName: string): this { - this.fileName = fileName; + public withDefaultFilename(defaultFilename: string): this { + this.filename = defaultFilename; return this; } @@ -246,7 +378,7 @@ class SaveFileDialogTestSetup { const dialog = new NodeElectronSaveFileDialog(this.logger, this.electron, this.node); return dialog.saveFile( this.fileContents, - this.fileName, + this.filename, this.fileType, ); } diff --git a/tests/unit/infrastructure/Dialog/LoggingDialogDecorator.spec.ts b/tests/unit/infrastructure/Dialog/LoggingDialogDecorator.spec.ts new file mode 100644 index 00000000..206c48a8 --- /dev/null +++ b/tests/unit/infrastructure/Dialog/LoggingDialogDecorator.spec.ts @@ -0,0 +1,163 @@ +import { describe, it, expect } from 'vitest'; +import { Logger } from '@/application/Common/Log/Logger'; +import { decorateWithLogging } from '@/infrastructure/Dialog/LoggingDialogDecorator'; +import { Dialog, FileType, SaveFileOutcome } from '@/presentation/common/Dialog'; +import { expectExists } from '@tests/shared/Assertions/ExpectExists'; +import { DialogStub } from '@tests/unit/shared/Stubs/DialogStub'; +import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; + +describe('LoggingDialogDecorator', () => { + describe('decorateWithLogging', () => { + describe('saveFile', () => { + it('delegates call to dialog', async () => { + // arrange + const expectedArguments = createTestSaveFileArguments(); + const dialog = new DialogStub(); + const context = new LoggingDialogDecoratorTestSetup() + .withDialog(dialog); + + // act + const decorator = context.decorateWithLogging(); + await decorator.saveFile(...expectedArguments); + + // assert + expect(dialog.callHistory).to.have.lengthOf(1); + const call = dialog.callHistory.find((c) => c.methodName === 'saveFile'); + expectExists(call); + const actualArguments = call.args; + expect(expectedArguments).to.deep.equal(actualArguments); + }); + it('returns dialog\'s response', async () => { + // arrange + const expectedResult: SaveFileOutcome = { success: true }; + const dialog = new DialogStub(); + dialog.saveFile = () => Promise.resolve(expectedResult); + const context = new LoggingDialogDecoratorTestSetup() + .withDialog(dialog); + + // act + const decorator = context.decorateWithLogging(); + const actualResult = await decorator.saveFile(...createTestSaveFileArguments()); + + // assert + expect(expectedResult).to.equal(actualResult); + }); + it('logs information on invocation', async () => { + // arrange + const expectedLogMessagePart = 'Opening save file dialog'; + const loggerStub = new LoggerStub(); + const context = new LoggingDialogDecoratorTestSetup() + .withLogger(loggerStub); + + // act + const decorator = context.decorateWithLogging(); + await decorator.saveFile(...createTestSaveFileArguments()); + + // assert + loggerStub.assertLogsContainMessagePart('info', expectedLogMessagePart); + }); + it('logs information on success', async () => { + // arrange + const expectedLogMessagePart = 'completed successfully'; + const loggerStub = new LoggerStub(); + const context = new LoggingDialogDecoratorTestSetup() + .withLogger(loggerStub); + + // act + const decorator = context.decorateWithLogging(); + await decorator.saveFile(...createTestSaveFileArguments()); + + // assert + loggerStub.assertLogsContainMessagePart('info', expectedLogMessagePart); + }); + it('logs error on save failure', async () => { + // arrange + const expectedLogMessagePart = 'Error encountered'; + const loggerStub = new LoggerStub(); + const dialog = new DialogStub(); + dialog.saveFile = () => Promise.resolve({ success: false, error: { message: 'error', type: 'DialogDisplayError' } }); + const context = new LoggingDialogDecoratorTestSetup() + .withLogger(loggerStub); + + // act + const decorator = context.decorateWithLogging(); + await decorator.saveFile(...createTestSaveFileArguments()); + + // assert + loggerStub.assertLogsContainMessagePart('error', expectedLogMessagePart); + }); + }); + describe('showError', () => { + it('delegates call to the dialog', () => { + // arrange + const expectedArguments = createTestShowErrorArguments(); + const dialog = new DialogStub(); + const context = new LoggingDialogDecoratorTestSetup() + .withDialog(dialog); + + // act + const decorator = context.decorateWithLogging(); + decorator.showError(...expectedArguments); + + // assert + expect(dialog.callHistory).to.have.lengthOf(1); + const call = dialog.callHistory.find((c) => c.methodName === 'showError'); + expectExists(call); + const actualArguments = call.args; + expect(expectedArguments).to.deep.equal(actualArguments); + }); + it('logs error message', () => { + // arrange + const expectedLogMessagePart = 'Showing error dialog'; + const loggerStub = new LoggerStub(); + const context = new LoggingDialogDecoratorTestSetup() + .withLogger(loggerStub); + + // act + const decorator = context.decorateWithLogging(); + decorator.showError(...createTestShowErrorArguments()); + + // assert + loggerStub.assertLogsContainMessagePart('error', expectedLogMessagePart); + }); + }); + }); +}); + +class LoggingDialogDecoratorTestSetup { + private dialog: Dialog = new DialogStub(); + + private logger: Logger = new LoggerStub(); + + public withDialog(dialog: Dialog): this { + this.dialog = dialog; + return this; + } + + public withLogger(logger: Logger): this { + this.logger = logger; + return this; + } + + public decorateWithLogging() { + return decorateWithLogging( + this.dialog, + this.logger, + ); + } +} + +function createTestSaveFileArguments(): Parameters { + return [ + 'test-file-contents', + 'test-default-filename', + FileType.BatchFile, + ]; +} + +function createTestShowErrorArguments(): Parameters { + return [ + 'test-error-title', + 'test-error-message', + ]; +} diff --git a/tests/unit/presentation/bootstrapping/Modules/AppInitializationLogger.spec.ts b/tests/unit/presentation/bootstrapping/Modules/AppInitializationLogger.spec.ts index f72fdf00..0e31c345 100644 --- a/tests/unit/presentation/bootstrapping/Modules/AppInitializationLogger.spec.ts +++ b/tests/unit/presentation/bootstrapping/Modules/AppInitializationLogger.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it } from 'vitest'; import { AppInitializationLogger } from '@/presentation/bootstrapping/Modules/AppInitializationLogger'; import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; @@ -11,8 +11,6 @@ describe('AppInitializationLogger', () => { // act await sut.bootstrap(); // assert - expect(loggerStub.callHistory).to.have.lengthOf(1); - expect(loggerStub.callHistory[0].args).to.have.lengthOf(1); - expect(loggerStub.callHistory[0].args[0]).to.include(marker); + loggerStub.assertLogsContainMessagePart('info', marker); }); }); diff --git a/tests/unit/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.spec.ts b/tests/unit/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.spec.ts index 84cc60c3..f8261353 100644 --- a/tests/unit/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.spec.ts +++ b/tests/unit/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.spec.ts @@ -1,54 +1,94 @@ import { describe, it, expect } from 'vitest'; -import { determineDialogBasedOnEnvironment, WindowDialogCreationFunction, BrowserDialogCreationFunction } from '@/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory'; +import { + createEnvironmentSpecificLoggedDialog, WindowDialogCreationFunction, + BrowserDialogCreationFunction, DialogLoggingDecorator, +} from '@/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory'; import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment'; import { RuntimeEnvironmentStub } from '@tests/unit/shared/Stubs/RuntimeEnvironmentStub'; import { DialogStub } from '@tests/unit/shared/Stubs/DialogStub'; import { collectExceptionMessage } from '@tests/unit/shared/ExceptionCollector'; +import { Dialog } from '@/presentation/common/Dialog'; describe('ClientDialogFactory', () => { - describe('determineDialogBasedOnEnvironment', () => { - describe('non-desktop environment', () => { - it('returns browser dialog', () => { - // arrange - const expectedDialog = new DialogStub(); - const context = new DialogCreationTestSetup() - .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(false)) - .withBrowserDialogFactory(() => expectedDialog); + describe('createEnvironmentSpecificLoggedDialog', () => { + describe('dialog selection based on environment', () => { + describe('when in non-desktop environment', () => { + it('provides a browser dialog', () => { + // arrange + const expectedDialog = new DialogStub(); + const context = new DialogCreationTestSetup() + .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(false)) + .withBrowserDialogFactory(() => expectedDialog); - // act - const actualDialog = context.createDialogForTest(); + // act + const actualDialog = context.createDialogForTest(); - // assert - expect(expectedDialog).to.equal(actualDialog); + // assert + expect(expectedDialog).to.equal(actualDialog); + }); + }); + describe('when in desktop environment', () => { + it('provides a window-injected dialog', () => { + // arrange + const expectedDialog = new DialogStub(); + const context = new DialogCreationTestSetup() + .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(true)) + .withWindowInjectedDialogFactory(() => expectedDialog); + + // act + const actualDialog = context.createDialogForTest(); + + // assert + expect(expectedDialog).to.equal(actualDialog); + }); + it('throws error if window-injected dialog is not available', () => { + // arrange + const expectedError = 'Failed to retrieve Dialog API from window object in desktop environment.'; + const context = new DialogCreationTestSetup() + .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(true)) + .withWindowInjectedDialogFactory(() => undefined); + + // act + const act = () => context.createDialogForTest(); + + // assert + const actualError = collectExceptionMessage(act); + expect(actualError).to.include(expectedError); + }); }); }); - describe('desktop environment', () => { - it('returns window-injected dialog', () => { + describe('dialog decoration with logging', () => { + it('returns a dialog decorated with logging', () => { // arrange - const expectedDialog = new DialogStub(); + const expectedLoggingDialogStub = new DialogStub(); + const decoratorStub: DialogLoggingDecorator = () => expectedLoggingDialogStub; const context = new DialogCreationTestSetup() - .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(true)) - .withWindowInjectedDialogFactory(() => expectedDialog); + .withDialogLoggingDecorator(decoratorStub); // act const actualDialog = context.createDialogForTest(); // assert - expect(expectedDialog).to.equal(actualDialog); + expect(expectedLoggingDialogStub).to.equal(actualDialog); }); - it('throws error when window-injected dialog is unavailable', () => { + it('applies logging decorator to the provided dialog', () => { // arrange - const expectedError = 'The Dialog API could not be retrieved from the window object.'; + const expectedDialog = new DialogStub(); + let actualDecoratedDialog: Dialog | undefined; + const decoratorStub: DialogLoggingDecorator = (dialog) => { + actualDecoratedDialog = dialog; + return new DialogStub(); + }; const context = new DialogCreationTestSetup() - .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(true)) - .withWindowInjectedDialogFactory(() => undefined); + .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(false)) + .withBrowserDialogFactory(() => expectedDialog) + .withDialogLoggingDecorator(decoratorStub); // act - const act = () => context.createDialogForTest(); + context.createDialogForTest(); // assert - const actualError = collectExceptionMessage(act); - expect(actualError).to.include(expectedError); + expect(expectedDialog).to.equal(actualDecoratedDialog); }); }); }); @@ -61,6 +101,8 @@ class DialogCreationTestSetup { private windowInjectedDialogFactory: WindowDialogCreationFunction = () => new DialogStub(); + private dialogLoggingDecorator: DialogLoggingDecorator = (dialog) => dialog; + public withEnvironment(environment: RuntimeEnvironment): this { this.environment = environment; return this; @@ -78,9 +120,17 @@ class DialogCreationTestSetup { return this; } + public withDialogLoggingDecorator( + dialogLoggingDecorator: DialogLoggingDecorator, + ): this { + this.dialogLoggingDecorator = dialogLoggingDecorator; + return this; + } + public createDialogForTest() { - return determineDialogBasedOnEnvironment( + return createEnvironmentSpecificLoggedDialog( this.environment, + this.dialogLoggingDecorator, this.windowInjectedDialogFactory, this.browserDialogFactory, ); diff --git a/tests/unit/shared/Stubs/CodeRunnerStub.ts b/tests/unit/shared/Stubs/CodeRunnerStub.ts index 7353f867..46767658 100644 --- a/tests/unit/shared/Stubs/CodeRunnerStub.ts +++ b/tests/unit/shared/Stubs/CodeRunnerStub.ts @@ -1,7 +1,9 @@ -import { CodeRunner } from '@/application/CodeRunner/CodeRunner'; +import { CodeRunOutcome, CodeRunner } from '@/application/CodeRunner/CodeRunner'; export class CodeRunnerStub implements CodeRunner { - public runCode(): Promise { - return Promise.resolve(); + public runCode(): Promise { + return Promise.resolve({ + success: true, + }); } } diff --git a/tests/unit/shared/Stubs/DialogStub.ts b/tests/unit/shared/Stubs/DialogStub.ts index 17585280..abd7444d 100644 --- a/tests/unit/shared/Stubs/DialogStub.ts +++ b/tests/unit/shared/Stubs/DialogStub.ts @@ -1,7 +1,23 @@ -import { Dialog } from '@/presentation/common/Dialog'; +import { Dialog, SaveFileOutcome } from '@/presentation/common/Dialog'; +import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; -export class DialogStub implements Dialog { - public saveFile(): Promise { - return Promise.resolve(); +export class DialogStub + extends StubWithObservableMethodCalls + implements Dialog { + public saveFile(...args: Parameters): Promise { + this.registerMethodCall({ + methodName: 'saveFile', + args: [...args], + }); + return Promise.resolve({ + success: true, + }); + } + + public showError(...args: Parameters): void { + this.registerMethodCall({ + methodName: 'showError', + args: [...args], + }); } } diff --git a/tests/unit/shared/Stubs/FilenameGeneratorStub.ts b/tests/unit/shared/Stubs/FilenameGeneratorStub.ts index d41b5e6a..f87fb650 100644 --- a/tests/unit/shared/Stubs/FilenameGeneratorStub.ts +++ b/tests/unit/shared/Stubs/FilenameGeneratorStub.ts @@ -1,5 +1,5 @@ import { FilenameGenerator } from '@/infrastructure/CodeRunner/Creation/Filename/FilenameGenerator'; -import { ScriptFileNameParts } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator'; +import { ScriptFilenameParts } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator'; import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; export class FilenameGeneratorStub @@ -7,7 +7,7 @@ export class FilenameGeneratorStub implements FilenameGenerator { private filename = `[${FilenameGeneratorStub.name}]file-name-stub`; - public generateFilename(scriptFileNameParts: ScriptFileNameParts): string { + public generateFilename(scriptFileNameParts: ScriptFilenameParts): string { this.registerMethodCall({ methodName: 'generateFilename', args: [scriptFileNameParts], diff --git a/tests/unit/shared/Stubs/LoggerStub.ts b/tests/unit/shared/Stubs/LoggerStub.ts index 0e9b752a..17a7b086 100644 --- a/tests/unit/shared/Stubs/LoggerStub.ts +++ b/tests/unit/shared/Stubs/LoggerStub.ts @@ -1,4 +1,6 @@ import { Logger } from '@/application/Common/Log/Logger'; +import { FunctionKeys, isString } from '@/TypeHelpers'; +import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage'; import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; export class LoggerStub extends StubWithObservableMethodCalls implements Logger { @@ -29,4 +31,27 @@ export class LoggerStub extends StubWithObservableMethodCalls implements args: params, }); } + + public assertLogsContainMessagePart( + methodName: FunctionKeys, + expectedLogMessagePart: string, + ) { + const loggedMessages = this.getLoggedMessages(methodName); + expect( + loggedMessages.some((m) => m.includes(expectedLogMessagePart)), + formatAssertionMessage([ + `Log function: ${methodName}`, + `Expected log message part: ${expectedLogMessagePart}`, + 'Actual log messages:', + loggedMessages.join('\n- '), + ]), + ); + } + + private getLoggedMessages(methodName: FunctionKeys): string[] { + const calls = this.callHistory.filter((m) => m.methodName === methodName); + const loggedItems = calls.flatMap((call) => call.args); + const stringLogs = loggedItems.filter((message): message is string => isString(message)); + return stringLogs; + } } diff --git a/tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts b/tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts index ad79a82c..4608969b 100644 --- a/tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts +++ b/tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts @@ -1,4 +1,4 @@ -import { ScriptDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider'; +import { ScriptDirectoryOutcome, ScriptDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider'; export class ScriptDirectoryProviderStub implements ScriptDirectoryProvider { private directoryPath = `[${ScriptDirectoryProviderStub.name}]scriptDirectory`; @@ -8,7 +8,10 @@ export class ScriptDirectoryProviderStub implements ScriptDirectoryProvider { return this; } - public provideScriptDirectory(): Promise { - return Promise.resolve(this.directoryPath); + public provideScriptDirectory(): Promise { + return Promise.resolve({ + success: true, + directoryAbsolutePath: this.directoryPath, + }); } } diff --git a/tests/unit/shared/Stubs/ScriptFileCreatorStub.ts b/tests/unit/shared/Stubs/ScriptFileCreatorStub.ts index d2593762..52931283 100644 --- a/tests/unit/shared/Stubs/ScriptFileCreatorStub.ts +++ b/tests/unit/shared/Stubs/ScriptFileCreatorStub.ts @@ -1,4 +1,4 @@ -import { ScriptFileCreator, ScriptFileNameParts } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator'; +import { ScriptFileCreationOutcome, ScriptFileCreator, ScriptFilenameParts } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator'; import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; export class ScriptFileCreatorStub @@ -13,12 +13,15 @@ export class ScriptFileCreatorStub public createScriptFile( contents: string, - scriptFileNameParts: ScriptFileNameParts, - ): Promise { + scriptFileNameParts: ScriptFilenameParts, + ): Promise { this.registerMethodCall({ methodName: 'createScriptFile', args: [contents, scriptFileNameParts], }); - return Promise.resolve(this.createdFilePath); + return Promise.resolve({ + success: true, + scriptFileAbsolutePath: this.createdFilePath, + }); } } diff --git a/tests/unit/shared/Stubs/ScriptFileExecutorStub.ts b/tests/unit/shared/Stubs/ScriptFileExecutorStub.ts index 99b20da9..23723304 100644 --- a/tests/unit/shared/Stubs/ScriptFileExecutorStub.ts +++ b/tests/unit/shared/Stubs/ScriptFileExecutorStub.ts @@ -1,14 +1,16 @@ -import { ScriptFileExecutor } from '@/infrastructure/CodeRunner/Execution/ScriptFileExecutor'; +import { ScriptFileExecutionOutcome, ScriptFileExecutor } from '@/infrastructure/CodeRunner/Execution/ScriptFileExecutor'; import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; export class ScriptFileExecutorStub extends StubWithObservableMethodCalls implements ScriptFileExecutor { - public executeScriptFile(filePath: string): Promise { + public executeScriptFile(filePath: string): Promise { this.registerMethodCall({ methodName: 'executeScriptFile', args: [filePath], }); - return Promise.resolve(); + return Promise.resolve({ + success: true, + }); } }