diff --git a/AGENTS.md b/AGENTS.md index a0d1fa98..e37281b8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,13 +26,14 @@ npm install # Install dependencies - takes ~55 seconds - `npm run build:win` - Builds and bundles the application for Windows systems ### Testing -- `npm test` - Run full test suite - takes ~13 seconds, runs 641 tests. NEVER CANCEL - Set timeout to 30+ minutes. +- `npm test` - Run full test suite - takes ~13 seconds, runs 656 tests. NEVER CANCEL - Set timeout to 30+ minutes. - `npm run test:e2e` - Run end-to-end tests to verify application startup - takes ~10-15 seconds - The project uses Jest with ECMAScript modules requiring the experimental VM modules flag for Node.js ### Running the Application - `npm run prestart && npm start` - Build and run the Electron application locally -- In CI/headless environments: `DISPLAY=:99 ./node_modules/.bin/electron . --no-sandbox` +- `npm start -- --settings-path /path/to/settings.json` - Run with a custom settings file location (useful for testing or multiple profiles) +- In CI/headless environments: `DISPLAY=:99 ./node_modules/.bin/electron . --no-sandbox` - The application requires X11 display and may need sandbox disabled in CI environments - **Global NPX usage**: `npx electronim` - Installs and runs the latest published version from npm registry @@ -85,6 +86,7 @@ The project includes E2E tests to verify the complete Electron application stack - Window verification uses DevTools output analysis to confirm successful rendering - Process termination uses SIGKILL due to tray icon preventing graceful SIGTERM shutdown - **Startup E2E Tests** (`src/__tests__/startup.test.e2e.js`) - Tests actual Electron application startup by spawning the full process +- **Settings Dialog E2E Tests** (`src/__tests__/settings-dialog.test.e2e.js`) - Tests that settings dialog appears when app starts with empty settings using `--settings-path` argument ## Technical Architecture @@ -100,6 +102,7 @@ The project includes E2E tests to verify the complete Electron application stack - `main/` - Electron main process logic - `tab-manager/` - Core tab and messaging functionality - `settings/` - Application settings and configuration UI + - `cli/` - Command-line argument parsing and validation - `about/` - About dialog and information - `chrome-tabs/` - Tab UI components based on Chrome tabs - `components/` - Reusable UI components (Material Design 3 style) @@ -139,6 +142,8 @@ The settings system uses Preact components with Material Design 3 styling: - Settings UI is at `src/settings/` - Browser tests cover URL validation and settings persistence - Settings include service tabs, spell check languages, and other configuration +- Custom settings path can be specified via `--settings-path` command-line argument for testing or multi-profile support +- The `setSettingsPath()` function in `src/settings/index.js` allows programmatic override of the default settings location ### Spell Check System - Supports 20+ languages using dictionary packages diff --git a/src/__tests__/settings.js b/src/__tests__/settings.js index acf22b30..90c16506 100644 --- a/src/__tests__/settings.js +++ b/src/__tests__/settings.js @@ -34,9 +34,9 @@ const testSettings = async () => { const os = require('node:os'); const fs = require('node:fs'); const settings = require('../settings'); - settings.paths.appDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'electronim-test-')); - settings.paths.settingsPath = path.join(settings.paths.appDir, 'settings.json'); - global.__testTempDirectories__.push(settings.paths.appDir); + const tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'electronim-test-')); + settings.setSettingsPath(path.join(tempDir, 'settings.json')); + global.__testTempDirectories__.push(tempDir); return settings; }; diff --git a/src/cli/__tests__/index.test.js b/src/cli/__tests__/index.test.js new file mode 100644 index 00000000..f0397313 --- /dev/null +++ b/src/cli/__tests__/index.test.js @@ -0,0 +1,142 @@ +/* + Copyright 2025 Marc Nuri San Felix + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +describe('CLI module test suite', () => { + let parseSettingsPath; + let consoleErrorSpy; + + beforeEach(() => { + jest.resetModules(); + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + ({parseSettingsPath} = require('../')); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + }); + + describe('parseSettingsPath', () => { + test('with no --settings-path flag, should return null', () => { + // Given + const args = ['--some-other-flag', 'value']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBeNull(); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + test('with valid --settings-path, should return the path', () => { + // Given + const args = ['--settings-path', '/path/to/settings.json']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBe('/path/to/settings.json'); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + test('with relative --settings-path, should return the path', () => { + // Given + const args = ['--settings-path', './settings.json']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBe('./settings.json'); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + test('with --settings-path and other flags, should return the path', () => { + // Given + const args = ['--no-sandbox', '--settings-path', '/custom/settings.json', '--disable-gpu']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBe('/custom/settings.json'); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + test('with --settings-path but no value, should return null and log error', () => { + // Given + const args = ['--settings-path']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBeNull(); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error: --settings-path requires a valid file path argument'); + }); + + test('with --settings-path followed by another flag, should return null and log error', () => { + // Given + const args = ['--settings-path', '--no-sandbox']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBeNull(); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error: --settings-path requires a valid file path argument'); + }); + + test('with --settings-path containing null byte, should return null and log error', () => { + // Given + const args = ['--settings-path', '/path/to/\0settings.json']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBeNull(); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error: --settings-path contains invalid characters'); + }); + + test('with empty string after --settings-path, should return null and log error', () => { + // Given + const args = ['--settings-path', '']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBeNull(); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error: --settings-path requires a valid file path argument'); + }); + + test('with --settings-path at end of args, should return null and log error', () => { + // Given + const args = ['--no-sandbox', '--settings-path']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBeNull(); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error: --settings-path requires a valid file path argument'); + }); + + test('with path containing spaces, should return the path', () => { + // Given + const args = ['--settings-path', '/path with spaces/settings.json']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBe('/path with spaces/settings.json'); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + test('with path containing special characters, should return the path', () => { + // Given + const args = ['--settings-path', '/path/with-special_chars.123/settings.json']; + // When + const result = parseSettingsPath(args); + // Then + expect(result).toBe('/path/with-special_chars.123/settings.json'); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/cli/index.js b/src/cli/index.js new file mode 100644 index 00000000..d8408b57 --- /dev/null +++ b/src/cli/index.js @@ -0,0 +1,47 @@ +/* + Copyright 2025 Marc Nuri San Felix + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +/** + * Parses command line arguments to extract the custom settings path. + * Validates that the value after --settings-path is a valid path and not another flag. + * + * @param {string[]} args - Command line arguments array + * @returns {string|null} The settings path if valid, null otherwise + */ +const parseSettingsPath = args => { + const settingsPathIndex = args.indexOf('--settings-path'); + if (settingsPathIndex === -1) { + return null; + } + + const settingsPathValue = args[settingsPathIndex + 1]; + + // Validate that there is a value and it's not another flag + if (!settingsPathValue || settingsPathValue.startsWith('--')) { + console.error('Error: --settings-path requires a valid file path argument'); + return null; + } + + // Basic validation: path should not contain null bytes (security) + if (settingsPathValue.includes('\0')) { + console.error('Error: --settings-path contains invalid characters'); + return null; + } + + return settingsPathValue; +}; + +module.exports = {parseSettingsPath}; diff --git a/src/index.js b/src/index.js index 91a3193d..3c9704a4 100644 --- a/src/index.js +++ b/src/index.js @@ -20,7 +20,16 @@ if (process.platform === 'linux') { app.commandLine.appendSwitch('gtk-version', '3'); } const {registerAppShortcuts} = require('./base-window'); +const {parseSettingsPath} = require('./cli'); const {init, quit} = require('./main'); +const {setSettingsPath} = require('./settings'); + +// Parse command line arguments for custom settings path +const args = process.argv.slice(process.defaultApp ? 2 : 1); +const customSettingsPath = parseSettingsPath(args); +if (customSettingsPath) { + setSettingsPath(customSettingsPath); +} app.name = 'ElectronIM'; diff --git a/src/settings/__tests__/index.test.js b/src/settings/__tests__/index.test.js index 433132c3..50bb0f52 100644 --- a/src/settings/__tests__/index.test.js +++ b/src/settings/__tests__/index.test.js @@ -356,4 +356,39 @@ describe('Settings module test suite', () => { const expectHomeDirectoryCreated = () => { expect(fs.mkdirSync).toHaveBeenCalledWith(path.join('$HOME', '.electronim'), {recursive: true}); }; + describe('setSettingsPath', () => { + test('should resolve relative paths to absolute', () => { + // Given + const relativePath = './my-settings.json'; + jest.spyOn(path, 'resolve').mockReturnValueOnce('/resolved/my-settings.json'); + // When + settings.setSettingsPath(relativePath); + // Then + expect(path.resolve).toHaveBeenCalledWith(relativePath); + }); + test('loadSettings should use custom path when set', () => { + // Given + const customPath = '/custom/settings.json'; + settings.setSettingsPath(customPath); + fs.existsSync.mockImplementationOnce(() => true); + fs.readFileSync.mockImplementationOnce(() => '{"tabs": [{"id": "custom"}], "theme": "dark"}'); + // When + const result = settings.loadSettings(); + // Then + expect(fs.mkdirSync).toHaveBeenCalledWith('/custom', {recursive: true}); + expect(fs.readFileSync).toHaveBeenCalledWith(customPath); + expect(result.tabs).toEqual([{id: 'custom'}]); + expect(result.theme).toBe('dark'); + }); + test('updateSettings should write to custom path when set', () => { + // Given + const customPath = '/custom/settings.json'; + settings.setSettingsPath(customPath); + fs.existsSync.mockImplementationOnce(() => false); + // When + settings.updateSettings({tabs: [{id: 'test'}]}); + // Then + expect(fs.writeFileSync).toHaveBeenCalledWith(customPath, expect.any(String)); + }); + }); }); diff --git a/src/settings/index.js b/src/settings/index.js index f06aa80c..2f6c0874 100644 --- a/src/settings/index.js +++ b/src/settings/index.js @@ -50,6 +50,19 @@ const paths = {}; paths.appDir = path.join(HOME_DIR, APP_DIR); paths.settingsPath = path.join(paths.appDir, SETTINGS_FILE); +/** + * Sets a custom settings file path. + * This allows users to specify a different location for their settings file, + * enabling multiple profiles or custom configurations. + * + * @param {string} customPath - The absolute or relative path to the settings file + */ +const setSettingsPath = customPath => { + const resolvedPath = path.resolve(customPath); + paths.settingsPath = resolvedPath; + paths.appDir = path.dirname(resolvedPath); +}; + /** * Wrapper function to retrieve the current system's platform. * @@ -185,6 +198,5 @@ const openSettingsDialog = mainWindow => () => { module.exports = { getPlatform, loadSettings, updateSettings, openSettingsDialog, exportSettings, importSettings, openElectronimFolder, - // Visible for testing - paths + setSettingsPath };