From 785549cbec71d2caaab4dc56f4301bd1bb643146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 13 Dec 2024 12:52:36 +0100 Subject: [PATCH] refactor(core): Add support for memoized evaluation on getters (no-changelog) (#12185) Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> --- packages/cli/src/services/frontend.service.ts | 19 +- packages/core/src/InstanceSettings.ts | 19 +- .../src/decorators/__tests__/memoized.test.ts | 153 ++++++++++++++++ packages/core/src/decorators/index.ts | 1 + packages/core/src/decorators/memoized.ts | 41 +++++ packages/core/src/index.ts | 1 + packages/core/test/InstanceSettings.test.ts | 168 ++++++++++++------ 7 files changed, 325 insertions(+), 77 deletions(-) create mode 100644 packages/core/src/decorators/__tests__/memoized.test.ts create mode 100644 packages/core/src/decorators/index.ts create mode 100644 packages/core/src/decorators/memoized.ts diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 0e1a64e0e7..1645e98304 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -5,7 +5,6 @@ import { mkdir } from 'fs/promises'; import uniq from 'lodash/uniq'; import { InstanceSettings } from 'n8n-core'; import type { ICredentialType, INodeTypeBaseDescription } from 'n8n-workflow'; -import fs from 'node:fs'; import path from 'path'; import { Container, Service } from 'typedi'; @@ -83,7 +82,7 @@ export class FrontendService { this.settings = { inE2ETests, - isDocker: this.isDocker(), + isDocker: this.instanceSettings.isDocker, databaseType: this.globalConfig.database.type, previewMode: process.env.N8N_PREVIEW_MODE === 'true', endpointForm: this.globalConfig.endpoints.form, @@ -392,20 +391,4 @@ export class FrontendService { } } } - - /** - * Whether this instance is running inside a Docker container. - * - * Based on: https://github.com/sindresorhus/is-docker - */ - private isDocker() { - try { - return ( - fs.existsSync('/.dockerenv') || - fs.readFileSync('/proc/self/cgroup', 'utf8').includes('docker') - ); - } catch { - return false; - } - } } diff --git a/packages/core/src/InstanceSettings.ts b/packages/core/src/InstanceSettings.ts index c51b0e94bb..f611e034b3 100644 --- a/packages/core/src/InstanceSettings.ts +++ b/packages/core/src/InstanceSettings.ts @@ -1,10 +1,11 @@ import { createHash, randomBytes } from 'crypto'; -import { chmodSync, existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'fs'; import { ApplicationError, jsonParse, ALPHABET, toResult } from 'n8n-workflow'; import { customAlphabet } from 'nanoid'; +import { chmodSync, existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'node:fs'; import path from 'path'; import { Service } from 'typedi'; +import { Memoized } from './decorators'; import { InstanceSettingsConfig } from './InstanceSettingsConfig'; const nanoid = customAlphabet(ALPHABET, 16); @@ -133,6 +134,22 @@ export class InstanceSettings { return this.settings.tunnelSubdomain; } + /** + * Whether this instance is running inside a Docker container. + * + * Based on: https://github.com/sindresorhus/is-docker + */ + @Memoized + get isDocker() { + try { + return ( + existsSync('/.dockerenv') || readFileSync('/proc/self/cgroup', 'utf8').includes('docker') + ); + } catch { + return false; + } + } + update(newSettings: WritableSettings) { this.save({ ...this.settings, ...newSettings }); } diff --git a/packages/core/src/decorators/__tests__/memoized.test.ts b/packages/core/src/decorators/__tests__/memoized.test.ts new file mode 100644 index 0000000000..f29ea4d469 --- /dev/null +++ b/packages/core/src/decorators/__tests__/memoized.test.ts @@ -0,0 +1,153 @@ +import { AssertionError, ok } from 'node:assert'; +import { setFlagsFromString } from 'node:v8'; +import { runInNewContext } from 'node:vm'; + +import { Memoized } from '../memoized'; + +describe('Memoized Decorator', () => { + class TestClass { + private computeCount = 0; + + constructor(private readonly value: number = 42) {} + + @Memoized + get expensiveComputation() { + this.computeCount++; + return this.value * 2; + } + + getComputeCount() { + return this.computeCount; + } + } + + it('should only compute the value once', () => { + const instance = new TestClass(); + + // First access should compute + expect(instance.expensiveComputation).toBe(84); + expect(instance.getComputeCount()).toBe(1); + + // Second access should use cached value + expect(instance.expensiveComputation).toBe(84); + expect(instance.getComputeCount()).toBe(1); + + // Third access should still use cached value + expect(instance.expensiveComputation).toBe(84); + expect(instance.getComputeCount()).toBe(1); + }); + + it('should cache values independently for different instances', () => { + const instance1 = new TestClass(10); + const instance2 = new TestClass(20); + + expect(instance1.expensiveComputation).toBe(20); + expect(instance2.expensiveComputation).toBe(40); + + expect(instance1.getComputeCount()).toBe(1); + expect(instance2.getComputeCount()).toBe(1); + }); + + it('should throw error when used on non-getter', () => { + expect(() => { + class InvalidClass { + // @ts-expect-error this code will fail at compile time and at runtime + @Memoized + public normalProperty = 42; + } + new InvalidClass(); + }).toThrow(AssertionError); + }); + + it('should make cached value non-enumerable', () => { + const instance = new TestClass(); + instance.expensiveComputation; // Access to trigger caching + + const propertyNames = Object.keys(instance); + expect(propertyNames).not.toContain('expensiveComputation'); + }); + + it('should not allow reconfiguring the cached value', () => { + const instance = new TestClass(); + instance.expensiveComputation; // Access to trigger caching + + expect(() => { + Object.defineProperty(instance, 'expensiveComputation', { + value: 999, + configurable: true, + }); + }).toThrow(); + }); + + it('should work when child class references memoized getter in parent class', () => { + class ParentClass { + protected computeCount = 0; + + @Memoized + get parentValue() { + this.computeCount++; + return 42; + } + + getComputeCount() { + return this.computeCount; + } + } + + class ChildClass extends ParentClass { + get childValue() { + return this.parentValue * 2; + } + } + + const child = new ChildClass(); + + expect(child.childValue).toBe(84); + expect(child.getComputeCount()).toBe(1); + + expect(child.childValue).toBe(84); + expect(child.getComputeCount()).toBe(1); + }); + + it('should have correct property descriptor after memoization', () => { + const instance = new TestClass(); + + // Before accessing (original getter descriptor) + const beforeDescriptor = Object.getOwnPropertyDescriptor( + TestClass.prototype, + 'expensiveComputation', + ); + expect(beforeDescriptor?.configurable).toBe(true); + expect(beforeDescriptor?.enumerable).toBe(false); + expect(typeof beforeDescriptor?.get).toBe('function'); + expect(beforeDescriptor?.set).toBeUndefined(); + + // After accessing (memoized value descriptor) + instance.expensiveComputation; // Trigger memoization + const afterDescriptor = Object.getOwnPropertyDescriptor(instance, 'expensiveComputation'); + expect(afterDescriptor?.configurable).toBe(false); + expect(afterDescriptor?.enumerable).toBe(false); + expect(afterDescriptor?.writable).toBe(false); + expect(afterDescriptor?.value).toBe(84); + expect(afterDescriptor?.get).toBeUndefined(); + }); + + it('should not prevent garbage collection of instances', async () => { + setFlagsFromString('--expose_gc'); + const gc = runInNewContext('gc') as unknown as () => void; + + let instance: TestClass | undefined = new TestClass(); + const weakRef = new WeakRef(instance); + instance.expensiveComputation; + + // Remove the strong reference + instance = undefined; + + // Wait for garbage collection, forcing it if needed + await new Promise((resolve) => setTimeout(resolve, 10)); + gc(); + + const ref = weakRef.deref(); + ok(!ref, 'GC did not collect the instance ref'); + }); +}); diff --git a/packages/core/src/decorators/index.ts b/packages/core/src/decorators/index.ts new file mode 100644 index 0000000000..2bf9174d7a --- /dev/null +++ b/packages/core/src/decorators/index.ts @@ -0,0 +1 @@ +export { Memoized } from './memoized'; diff --git a/packages/core/src/decorators/memoized.ts b/packages/core/src/decorators/memoized.ts new file mode 100644 index 0000000000..3ff5c56619 --- /dev/null +++ b/packages/core/src/decorators/memoized.ts @@ -0,0 +1,41 @@ +import assert from 'node:assert'; + +/** + * A decorator that implements memoization for class property getters. + * + * The decorated getter will only be executed once and its value cached for subsequent access + * + * @example + * class Example { + * @Memoized + * get computedValue() { + * // This will only run once and the result will be cached + * return heavyComputation(); + * } + * } + * + * @throws If decorator is used on something other than a getter + */ +export function Memoized( + target: object, + propertyKey: string | symbol, + descriptor?: TypedPropertyDescriptor, +): TypedPropertyDescriptor { + const originalGetter = descriptor?.get; + assert(originalGetter, '@Memoized can only be used on getters'); + + // Replace the original getter for the first call + descriptor.get = function (this: typeof target.constructor): T { + const value = originalGetter.call(this); + // Add a property on the class instance to stop reading from the getter on class prototype + Object.defineProperty(this, propertyKey, { + value, + configurable: false, + enumerable: false, + writable: false, + }); + return value; + }; + + return descriptor; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 13598d6d2b..f2f2149b60 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,5 +1,6 @@ import * as NodeExecuteFunctions from './NodeExecuteFunctions'; +export * from './decorators'; export * from './errors'; export * from './ActiveWorkflows'; export * from './BinaryData/BinaryData.service'; diff --git a/packages/core/test/InstanceSettings.test.ts b/packages/core/test/InstanceSettings.test.ts index 22f8fadff8..b6a9cf63c4 100644 --- a/packages/core/test/InstanceSettings.test.ts +++ b/packages/core/test/InstanceSettings.test.ts @@ -1,16 +1,18 @@ -import fs from 'fs'; +import { mock } from 'jest-mock-extended'; +jest.mock('node:fs', () => mock()); +import * as fs from 'node:fs'; -import { InstanceSettings } from '../src/InstanceSettings'; -import { InstanceSettingsConfig } from '../src/InstanceSettingsConfig'; +import { InstanceSettings } from '@/InstanceSettings'; +import { InstanceSettingsConfig } from '@/InstanceSettingsConfig'; describe('InstanceSettings', () => { - process.env.N8N_USER_FOLDER = '/test'; + const userFolder = '/test'; + process.env.N8N_USER_FOLDER = userFolder; + const settingsFile = `${userFolder}/.n8n/config`; - const existSpy = jest.spyOn(fs, 'existsSync'); - const statSpy = jest.spyOn(fs, 'statSync'); - const chmodSpy = jest.spyOn(fs, 'chmodSync'); + const mockFs = mock(fs); - const createSettingsInstance = (opts?: Partial) => + const createInstanceSettings = (opts?: Partial) => new InstanceSettings({ ...new InstanceSettingsConfig(), ...opts, @@ -18,18 +20,17 @@ describe('InstanceSettings', () => { beforeEach(() => { jest.resetAllMocks(); - statSpy.mockReturnValue({ mode: 0o600 } as fs.Stats); + mockFs.statSync.mockReturnValue({ mode: 0o600 } as fs.Stats); }); describe('If the settings file exists', () => { - const readSpy = jest.spyOn(fs, 'readFileSync'); beforeEach(() => { - existSpy.mockReturnValue(true); + mockFs.existsSync.mockReturnValue(true); }); it('should load settings from the file', () => { - readSpy.mockReturnValue(JSON.stringify({ encryptionKey: 'test_key' })); - const settings = createSettingsInstance(); + mockFs.readFileSync.mockReturnValue(JSON.stringify({ encryptionKey: 'test_key' })); + const settings = createInstanceSettings(); expect(settings.encryptionKey).toEqual('test_key'); expect(settings.instanceId).toEqual( '6ce26c63596f0cc4323563c529acfca0cccb0e57f6533d79a60a42c9ff862ae7', @@ -37,71 +38,69 @@ describe('InstanceSettings', () => { }); it('should throw error if settings file is not valid JSON', () => { - readSpy.mockReturnValue('{"encryptionKey":"test_key"'); - expect(() => createSettingsInstance()).toThrowError(); + mockFs.readFileSync.mockReturnValue('{"encryptionKey":"test_key"'); + expect(() => createInstanceSettings()).toThrowError(); }); it('should throw if the env and file keys do not match', () => { - readSpy.mockReturnValue(JSON.stringify({ encryptionKey: 'key_1' })); + mockFs.readFileSync.mockReturnValue(JSON.stringify({ encryptionKey: 'key_1' })); process.env.N8N_ENCRYPTION_KEY = 'key_2'; - expect(() => createSettingsInstance()).toThrowError(); + expect(() => createInstanceSettings()).toThrowError(); }); it('should check if the settings file has the correct permissions', () => { process.env.N8N_ENCRYPTION_KEY = 'test_key'; - readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); - statSpy.mockReturnValueOnce({ mode: 0o600 } as fs.Stats); - const settings = createSettingsInstance(); + mockFs.readFileSync.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); + mockFs.statSync.mockReturnValueOnce({ mode: 0o600 } as fs.Stats); + const settings = createInstanceSettings(); expect(settings.encryptionKey).toEqual('test_key'); expect(settings.instanceId).toEqual( '6ce26c63596f0cc4323563c529acfca0cccb0e57f6533d79a60a42c9ff862ae7', ); - expect(statSpy).toHaveBeenCalledWith('/test/.n8n/config'); + expect(mockFs.statSync).toHaveBeenCalledWith('/test/.n8n/config'); }); it('should check the permissions but not fix them if settings file has incorrect permissions by default', () => { - readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); - statSpy.mockReturnValueOnce({ mode: 0o644 } as fs.Stats); - createSettingsInstance(); - expect(statSpy).toHaveBeenCalledWith('/test/.n8n/config'); - expect(chmodSpy).not.toHaveBeenCalled(); + mockFs.readFileSync.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); + mockFs.statSync.mockReturnValueOnce({ mode: 0o644 } as fs.Stats); + createInstanceSettings(); + expect(mockFs.statSync).toHaveBeenCalledWith('/test/.n8n/config'); + expect(mockFs.chmodSync).not.toHaveBeenCalled(); }); it("should not check the permissions if 'N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS' is false", () => { process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'false'; - readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); - createSettingsInstance(); - expect(statSpy).not.toHaveBeenCalled(); - expect(chmodSpy).not.toHaveBeenCalled(); + mockFs.readFileSync.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); + createInstanceSettings(); + expect(mockFs.statSync).not.toHaveBeenCalled(); + expect(mockFs.chmodSync).not.toHaveBeenCalled(); }); it("should fix the permissions of the settings file if 'N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS' is true", () => { process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'true'; - readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); - statSpy.mockReturnValueOnce({ mode: 0o644 } as fs.Stats); - createSettingsInstance({ + mockFs.readFileSync.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' })); + mockFs.statSync.mockReturnValueOnce({ mode: 0o644 } as fs.Stats); + createInstanceSettings({ enforceSettingsFilePermissions: true, }); - expect(statSpy).toHaveBeenCalledWith('/test/.n8n/config'); - expect(chmodSpy).toHaveBeenCalledWith('/test/.n8n/config', 0o600); + expect(mockFs.statSync).toHaveBeenCalledWith('/test/.n8n/config'); + expect(mockFs.chmodSync).toHaveBeenCalledWith('/test/.n8n/config', 0o600); }); }); describe('If the settings file does not exist', () => { - const mkdirSpy = jest.spyOn(fs, 'mkdirSync'); - const writeFileSpy = jest.spyOn(fs, 'writeFileSync'); beforeEach(() => { - existSpy.mockReturnValue(false); - mkdirSpy.mockReturnValue(''); - writeFileSpy.mockReturnValue(); + mockFs.existsSync.mockReturnValue(false); + mockFs.mkdirSync.mockReturnValue(''); + mockFs.writeFileSync.mockReturnValue(); }); it('should create a new settings file without explicit permissions if N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS is not set', () => { process.env.N8N_ENCRYPTION_KEY = 'key_2'; - const settings = createSettingsInstance(); + const settings = createInstanceSettings(); expect(settings.encryptionKey).not.toEqual('test_key'); - expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); - expect(writeFileSpy).toHaveBeenCalledWith( + expect(mockFs.mkdirSync).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); + expect(mockFs.writeFileSync).toHaveBeenCalledWith( '/test/.n8n/config', expect.stringContaining('"encryptionKey":'), { @@ -114,10 +113,10 @@ describe('InstanceSettings', () => { it('should create a new settings file without explicit permissions if N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=false', () => { process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'false'; process.env.N8N_ENCRYPTION_KEY = 'key_2'; - const settings = createSettingsInstance(); + const settings = createInstanceSettings(); expect(settings.encryptionKey).not.toEqual('test_key'); - expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); - expect(writeFileSpy).toHaveBeenCalledWith( + expect(mockFs.mkdirSync).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); + expect(mockFs.writeFileSync).toHaveBeenCalledWith( '/test/.n8n/config', expect.stringContaining('"encryptionKey":'), { @@ -130,12 +129,12 @@ describe('InstanceSettings', () => { it('should create a new settings file with explicit permissions if N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=true', () => { process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'true'; process.env.N8N_ENCRYPTION_KEY = 'key_2'; - const settings = createSettingsInstance({ + const settings = createInstanceSettings({ enforceSettingsFilePermissions: true, }); expect(settings.encryptionKey).not.toEqual('test_key'); - expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); - expect(writeFileSpy).toHaveBeenCalledWith( + expect(mockFs.mkdirSync).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); + expect(mockFs.writeFileSync).toHaveBeenCalledWith( '/test/.n8n/config', expect.stringContaining('"encryptionKey":'), { @@ -147,14 +146,14 @@ describe('InstanceSettings', () => { it('should pick up the encryption key from env var N8N_ENCRYPTION_KEY', () => { process.env.N8N_ENCRYPTION_KEY = 'env_key'; - const settings = createSettingsInstance(); + const settings = createInstanceSettings(); expect(settings.encryptionKey).toEqual('env_key'); expect(settings.instanceId).toEqual( '2c70e12b7a0646f92279f427c7b38e7334d8e5389cff167a1dc30e73f826b683', ); expect(settings.encryptionKey).not.toEqual('test_key'); - expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); - expect(writeFileSpy).toHaveBeenCalledWith( + expect(mockFs.mkdirSync).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); + expect(mockFs.writeFileSync).toHaveBeenCalledWith( '/test/.n8n/config', expect.stringContaining('"encryptionKey":'), { @@ -167,10 +166,10 @@ describe('InstanceSettings', () => { it("should not set the permissions of the settings file if 'N8N_IGNORE_SETTINGS_FILE_PERMISSIONS' is true", () => { process.env.N8N_ENCRYPTION_KEY = 'key_2'; process.env.N8N_IGNORE_SETTINGS_FILE_PERMISSIONS = 'true'; - const settings = createSettingsInstance(); + const settings = createInstanceSettings(); expect(settings.encryptionKey).not.toEqual('test_key'); - expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); - expect(writeFileSpy).toHaveBeenCalledWith( + expect(mockFs.mkdirSync).toHaveBeenCalledWith('/test/.n8n', { recursive: true }); + expect(mockFs.writeFileSync).toHaveBeenCalledWith( '/test/.n8n/config', expect.stringContaining('"encryptionKey":'), { @@ -185,14 +184,67 @@ describe('InstanceSettings', () => { it('should generate a `hostId`', () => { const encryptionKey = 'test_key'; process.env.N8N_ENCRYPTION_KEY = encryptionKey; - jest.spyOn(fs, 'existsSync').mockReturnValueOnce(true); - jest.spyOn(fs, 'readFileSync').mockReturnValueOnce(JSON.stringify({ encryptionKey })); + mockFs.existsSync.mockReturnValueOnce(true); + mockFs.readFileSync.mockReturnValueOnce(JSON.stringify({ encryptionKey })); - const settings = createSettingsInstance(); + const settings = createInstanceSettings(); const [instanceType, nanoid] = settings.hostId.split('-'); expect(instanceType).toEqual('main'); expect(nanoid).toHaveLength(16); // e.g. sDX6ZPc0bozv66zM }); }); + + describe('isDocker', () => { + let settings: InstanceSettings; + + beforeEach(() => { + mockFs.existsSync.calledWith(settingsFile).mockReturnValue(true); + mockFs.readFileSync + .calledWith(settingsFile) + .mockReturnValue(JSON.stringify({ encryptionKey: 'test_key' })); + settings = new InstanceSettings(mock()); + }); + + it('should return true if /.dockerenv exists', () => { + mockFs.existsSync.calledWith('/.dockerenv').mockReturnValueOnce(true); + expect(settings.isDocker).toBe(true); + expect(mockFs.existsSync).toHaveBeenCalledWith('/.dockerenv'); + expect(mockFs.readFileSync).not.toHaveBeenCalledWith('/proc/self/cgroup', 'utf8'); + }); + + it('should return true if /proc/self/cgroup contains docker', () => { + mockFs.existsSync.calledWith('/.dockerenv').mockReturnValueOnce(false); + mockFs.readFileSync + .calledWith('/proc/self/cgroup', 'utf8') + .mockReturnValueOnce('docker cgroup'); + + expect(settings.isDocker).toBe(true); + expect(mockFs.existsSync).toHaveBeenCalledWith('/.dockerenv'); + expect(mockFs.readFileSync).toHaveBeenCalledWith('/proc/self/cgroup', 'utf8'); + }); + + it('should return false if no docker indicators are found', () => { + mockFs.existsSync.calledWith('/.dockerenv').mockReturnValueOnce(false); + mockFs.readFileSync.calledWith('/proc/self/cgroup', 'utf8').mockReturnValueOnce(''); + expect(settings.isDocker).toBe(false); + }); + + it('should return false if checking for docker throws an error', () => { + mockFs.existsSync.calledWith('/.dockerenv').mockImplementationOnce(() => { + throw new Error('Access denied'); + }); + expect(settings.isDocker).toBe(false); + }); + + it('should cache the result of isDocker check', () => { + mockFs.existsSync.calledWith('/.dockerenv').mockReturnValueOnce(true); + + expect(settings.isDocker).toBe(true); + + mockFs.existsSync.mockClear(); + expect(settings.isDocker).toBe(true); + expect(mockFs.existsSync).not.toHaveBeenCalled(); + }); + }); });