From d234b41853fc646a07051736a99d0096b75aa98e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 13:36:48 +0200 Subject: [PATCH 01/17] backend-app-api: refactor test to remove mock-fs Signed-off-by: Patrik Oldsberg --- yarn.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 8846ec0813..70690186aa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3603,7 +3603,6 @@ __metadata: chokidar: ^3.5.3 express: ^4.17.1 lodash: ^4.17.21 - mock-fs: ^5.2.0 wait-for-expect: ^3.0.2 winston: ^3.2.1 languageName: unknown From c45f587b1abf0d15f4eb21838c9870c41d7023bd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 14:07:52 +0200 Subject: [PATCH 02/17] backend-plugin-manager: refactor test to remove mock-fs Signed-off-by: Patrik Oldsberg --- packages/backend-plugin-manager/package.json | 1 - .../src/manager/plugin-manager.test.ts | 53 ++++++++++--------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/packages/backend-plugin-manager/package.json b/packages/backend-plugin-manager/package.json index 67697981a1..33675dda1f 100644 --- a/packages/backend-plugin-manager/package.json +++ b/packages/backend-plugin-manager/package.json @@ -55,7 +55,6 @@ "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@backstage/config-loader": "workspace:^", - "mock-fs": "^5.2.0", "wait-for-expect": "^3.0.2" }, "files": [ diff --git a/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts b/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts index 093776e5b0..f356163dea 100644 --- a/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts +++ b/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts @@ -20,10 +20,9 @@ import { coreServices, createServiceFactory, } from '@backstage/backend-plugin-api'; -import mockFs, { directory, symlink } from 'mock-fs'; import * as path from 'path'; import * as url from 'url'; - +import fs from 'fs'; import { BackendDynamicPlugin, BaseDynamicPlugin, @@ -43,11 +42,13 @@ import { ConfigSources } from '@backstage/config-loader'; import { Logs, MockedLogger, LogContent } from '../__testUtils__/testUtils'; import { PluginScanner } from '../scanner/plugin-scanner'; import { findPaths } from '@backstage/cli-common'; +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('backend-plugin-manager', () => { + const mockDir = createMockDirectory(); + describe('loadPlugins', () => { afterEach(() => { - mockFs.restore(); jest.resetModules(); }); @@ -56,7 +57,7 @@ describe('backend-plugin-manager', () => { packageManifest: ScannedPluginManifest; indexFile?: { retativePath: string[]; - content?: string; + content: string; }; expectedLogs?(location: URL): { errors?: LogContent[]; @@ -354,17 +355,13 @@ describe('backend-plugin-manager', () => { }, ])('$name', async (tc: TestCase): Promise => { const plugin: ScannedPluginPackage = { - location: url.pathToFileURL( - path.resolve(`/node_modules/jest-tests/${randomUUID()}`), - ), + location: url.pathToFileURL(mockDir.resolve(randomUUID())), manifest: tc.packageManifest, }; const mockedFiles = { [path.join(url.fileURLToPath(plugin.location), 'package.json')]: - mockFs.file({ - content: JSON.stringify(plugin), - }), + JSON.stringify(plugin), }; if (tc.indexFile) { mockedFiles[ @@ -372,11 +369,9 @@ describe('backend-plugin-manager', () => { url.fileURLToPath(plugin.location), ...tc.indexFile.retativePath, ) - ] = mockFs.file({ - content: tc.indexFile.content, - }); + ] = tc.indexFile.content; } - mockFs(mockedFiles); + mockDir.setContent(mockedFiles); const logger = new MockedLogger(); const pluginManager = new (PluginManager as any)(logger, [plugin], { @@ -440,8 +435,11 @@ describe('backend-plugin-manager', () => { }); describe('dynamicPluginsServiceFactory', () => { + const otherMockDir = createMockDirectory(); + afterEach(() => { - mockFs.restore(); + mockDir.clear(); + otherMockDir.clear(); jest.resetModules(); }); @@ -449,15 +447,20 @@ describe('backend-plugin-manager', () => { const logger = new MockedLogger(); const rootLogger = new MockedLogger(); - mockFs({ - [findPaths(__dirname).resolveTargetRoot('package.json')]: mockFs.load( + mockDir.setContent({ + 'package.json': fs.readFileSync( findPaths(__dirname).resolveTargetRoot('package.json'), ), - '/somewhere/dynamic-plugins-root/a-dynamic-plugin': symlink({ - path: '/somewhere-else/a-dynamic-plugin', - }), - '/somewhere-else/a-dynamic-plugin': directory({}), + 'dynamic-plugins-root': {}, }); + otherMockDir.setContent({ + 'a-dynamic-plugin': {}, + }); + + fs.symlinkSync( + otherMockDir.resolve('a-dynamic-plugin'), + mockDir.resolve('dynamic-plugins-root/a-dynamic-plugin'), + ); const fromConfigSpier = jest.spyOn(PluginManager, 'fromConfig'); const applyConfigSpier = jest @@ -468,7 +471,7 @@ describe('backend-plugin-manager', () => { .mockImplementation(async () => [ { location: url.pathToFileURL( - path.resolve('/somewhere/dynamic-plugins-root/a-dynamic-plugin'), + mockDir.resolve('dynamic-plugins-root/a-dynamic-plugin'), ), manifest: { name: 'test', @@ -533,11 +536,11 @@ describe('backend-plugin-manager', () => { expect(scanRootSpier).toHaveBeenCalled(); expect(mockedModuleLoader.bootstrap).toHaveBeenCalledWith( findPaths(__dirname).targetRoot, - [path.resolve('/somewhere-else/a-dynamic-plugin')], + [fs.realpathSync(otherMockDir.resolve('a-dynamic-plugin'))], ); expect(mockedModuleLoader.load).toHaveBeenCalledWith( - path.resolve( - '/somewhere/dynamic-plugins-root/a-dynamic-plugin/dist/index.cjs.js', + mockDir.resolve( + 'dynamic-plugins-root/a-dynamic-plugin/dist/index.cjs.js', ), ); }); From d45375bccf67b59b908d17d590e5e1b18673e1be Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 15:06:42 +0200 Subject: [PATCH 03/17] backend-plugin-manager: migrate plugin-scanner tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../src/scanner/plugin-scanner.test.ts | 493 ++++++++---------- 1 file changed, 204 insertions(+), 289 deletions(-) diff --git a/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts b/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts index 6a36a377f5..04ce69ec98 100644 --- a/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts +++ b/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts @@ -15,13 +15,16 @@ */ import { PluginScanner } from './plugin-scanner'; -import mockFs from 'mock-fs'; import { JsonObject } from '@backstage/types'; import { Logs, MockedLogger } from '../__testUtils__/testUtils'; import { ConfigReader } from '@backstage/config'; import path from 'path'; +import fs from 'fs'; import * as url from 'url'; import { ScannedPluginPackage } from './types'; +import { createMockDirectory } from '@backstage/backend-test-utils'; + +const mockDir = createMockDirectory(); describe('plugin-scanner', () => { const env = process.env; @@ -30,7 +33,7 @@ describe('plugin-scanner', () => { }); afterEach(() => { - mockFs.restore(); + mockDir.clear(); process.env = env; }); @@ -61,85 +64,77 @@ describe('plugin-scanner', () => { }, { name: 'valid config with relative root directory path', - backstageRoot: '/backstageRoot', + backstageRoot: mockDir.resolve('backstageRoot'), fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory(), - }, - }), + backstageRoot: { + 'dist-dynamic': {}, + }, }, config: { dynamicPlugins: { rootDirectory: 'dist-dynamic', }, }, - expectedRootDirectory: path.resolve('/backstageRoot/dist-dynamic'), + expectedRootDirectory: mockDir.resolve('backstageRoot/dist-dynamic'), }, { name: 'valid config with absolute root directory path inside the backstage root', - backstageRoot: '/backstageRoot', + backstageRoot: mockDir.resolve('backstageRoot'), fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory(), - }, - }), + backstageRoot: { + 'dist-dynamic': {}, + }, }, config: { dynamicPlugins: { - rootDirectory: '/backstageRoot/dist-dynamic', + rootDirectory: mockDir.resolve('backstageRoot/dist-dynamic'), }, }, - expectedRootDirectory: path.resolve('/backstageRoot/dist-dynamic'), + expectedRootDirectory: mockDir.resolve('backstageRoot/dist-dynamic'), }, { name: 'valid config with absolute root directory path outside the backstage root', - backstageRoot: '/backstageRoot', + backstageRoot: mockDir.resolve('backstageRoot'), fileSystem: { - '/somewhere': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory(), - }, - }), + somewhere: { + 'dist-dynamic': {}, + }, }, config: { dynamicPlugins: { - rootDirectory: '/somewhere/dist-dynamic', + rootDirectory: mockDir.resolve('somewhere/dist-dynamic'), }, }, - expectedError: `Dynamic plugins under '${path.resolve( - '/somewhere/dist-dynamic', - )}' cannot access backstage modules in '${path.resolve( - '/backstageRoot/node_modules', + expectedError: `Dynamic plugins under '${mockDir.resolve( + 'somewhere/dist-dynamic', + )}' cannot access backstage modules in '${mockDir.resolve( + 'backstageRoot/node_modules', )}'. -Please add '${path.resolve( - '/backstageRoot/node_modules', +Please add '${mockDir.resolve( + 'backstageRoot/node_modules', )}' to the 'NODE_PATH' when running the backstage backend.`, }, { name: 'valid config with absolute root directory path outside the backstage root but with backstage root included in NODE_PATH', - backstageRoot: '/backstageRoot', + backstageRoot: mockDir.resolve('backstageRoot'), fileSystem: { - '/somewhere': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory(), - }, - }), + somewhere: { + 'dist-dynamic': {}, + }, }, config: { dynamicPlugins: { - rootDirectory: '/somewhere/dist-dynamic', + rootDirectory: mockDir.resolve('somewhere/dist-dynamic'), }, }, environment: { - NODE_PATH: `${path.resolve('/somewhere-else')}${ + NODE_PATH: `${mockDir.resolve('somewhere-else')}${ path.delimiter - }${path.resolve('/backstageRoot', 'node_modules')}${ + }${mockDir.resolve('backstageRoot', 'node_modules')}${ path.delimiter - }${path.resolve('anywhere-else')}`, + }${mockDir.resolve('anywhere-else')}`, }, - expectedRootDirectory: path.resolve('/somewhere/dist-dynamic'), + expectedRootDirectory: mockDir.resolve('somewhere/dist-dynamic'), }, { name: 'invalid config: dynamicPlugins not an object', @@ -186,13 +181,11 @@ Please add '${path.resolve( }, { name: 'valid config pointing to a file instead of a directory', - backstageRoot: '/backstageRoot', + backstageRoot: mockDir.resolve('backstageRoot'), fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.file(), - }, - }), + backstageRoot: { + 'dist-dynamic': '', + }, }, config: { dynamicPlugins: { @@ -218,7 +211,7 @@ Please add '${path.resolve( ); } if (tc.fileSystem) { - mockFs(tc.fileSystem); + mockDir.setContent(tc.fileSystem); } if (tc.expectedError) { /* eslint-disable-next-line jest/no-conditional-expect */ @@ -241,6 +234,7 @@ Please add '${path.resolve( name: string; preferAlpha?: boolean; fileSystem?: any; + symlinks?: { source: string; target: string }[]; expectedLogs?: Logs; expectedPluginPackages?: ScannedPluginPackage[]; expectedError?: string; @@ -261,31 +255,23 @@ Please add '${path.resolve( { name: 'manifest found in directory', fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: 'dist/index.cjs.js', - backstage: { role: 'backend-plugin' }, - }), - }), - }, - }), - }, - }), + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: 'dist/index.cjs.js', + backstage: { role: 'backend-plugin' }, + }), + }, }, - }), + }, }, expectedPluginPackages: [ { location: url.pathToFileURL( - path.resolve('/backstageRoot/dist-dynamic/test-backend-plugin'), + mockDir.resolve('backstageRoot/dist-dynamic/test-backend-plugin'), ), manifest: { name: 'test-backend-plugin-dynamic', @@ -299,38 +285,34 @@ Please add '${path.resolve( { name: 'backend plugin found in symlink', fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.symlink({ - path: '/somewhere-else/test-backend-plugin-target', - }), - }, + backstageRoot: { + 'dist-dynamic': {}, + }, + 'somewhere-else': { + 'test-backend-plugin-target': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: 'dist/index.cjs.js', + backstage: { role: 'backend-plugin' }, }), }, - }), - '/somewhere-else': mockFs.directory({ - items: { - 'test-backend-plugin-target': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: 'dist/index.cjs.js', - backstage: { role: 'backend-plugin' }, - }), - }), - }, - }), - }, - }), + }, }, + symlinks: [ + { + source: mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin', + ), + target: mockDir.resolve( + 'somewhere-else/test-backend-plugin-target', + ), + }, + ], expectedPluginPackages: [ { location: url.pathToFileURL( - path.resolve('/backstageRoot/dist-dynamic/test-backend-plugin'), + mockDir.resolve('backstageRoot/dist-dynamic/test-backend-plugin'), ), manifest: { name: 'test-backend-plugin-dynamic', @@ -344,22 +326,18 @@ Please add '${path.resolve( { name: 'ignored folder child: not a directory', fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.file({}), - }, - }), + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': '', }, - }), + }, }, expectedPluginPackages: [], expectedLogs: { infos: [ { - message: `skipping '${path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin', + message: `skipping '${mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin', )}' since it is not a directory`, }, ], @@ -368,29 +346,29 @@ Please add '${path.resolve( { name: 'ignored folder child symlink: target is not a directory', fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.symlink({ - path: '/somewhere-else/test-backend-plugin-target', - }), - }, - }), - }, - }), - '/somewhere-else': mockFs.directory({ - items: { - 'test-backend-plugin-target': mockFs.file({}), - }, - }), + backstageRoot: { + 'dist-dynamic': {}, + }, + 'somewhere-else': { + 'test-backend-plugin-target': '', + }, }, + symlinks: [ + { + source: mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin', + ), + target: mockDir.resolve( + 'somewhere-else/test-backend-plugin-target', + ), + }, + ], expectedPluginPackages: [], expectedLogs: { infos: [ { - message: `skipping '${path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin', + message: `skipping '${mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin', )}' since it is not a directory`, }, ], @@ -400,42 +378,30 @@ Please add '${path.resolve( name: 'alpha manifest available but not preferred', preferAlpha: false, fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: 'dist/index.cjs.js', - backstage: { role: 'backend-plugin' }, - }), - }), - alpha: mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: '../dist/alpha.cjs.js', - }), - }), - }, - }), - }, + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: 'dist/index.cjs.js', + backstage: { role: 'backend-plugin' }, + }), + alpha: { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: '../dist/alpha.cjs.js', }), }, - }), + }, }, - }), + }, }, expectedPluginPackages: [ { location: url.pathToFileURL( - path.resolve('/backstageRoot/dist-dynamic/test-backend-plugin'), + mockDir.resolve('backstageRoot/dist-dynamic/test-backend-plugin'), ), manifest: { name: 'test-backend-plugin-dynamic', @@ -450,43 +416,31 @@ Please add '${path.resolve( name: 'alpha manifest preferred and found in directory', preferAlpha: true, fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: 'dist/index.cjs.js', - backstage: { role: 'backend-plugin' }, - }), - }), - alpha: mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: '../dist/alpha.cjs.js', - }), - }), - }, - }), - }, + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: 'dist/index.cjs.js', + backstage: { role: 'backend-plugin' }, + }), + alpha: { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: '../dist/alpha.cjs.js', }), }, - }), + }, }, - }), + }, }, expectedPluginPackages: [ { location: url.pathToFileURL( - path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin/alpha', + mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin/alpha', ), ), manifest: { @@ -502,32 +456,24 @@ Please add '${path.resolve( name: 'alpha manifest preferred but skipped because not a directory', preferAlpha: true, fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: 'dist/index.cjs.js', - backstage: { role: 'backend-plugin' }, - }), - }), - alpha: mockFs.file({}), - }, - }), - }, - }), + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: 'dist/index.cjs.js', + backstage: { role: 'backend-plugin' }, + }), + alpha: '', + }, }, - }), + }, }, expectedPluginPackages: [ { location: url.pathToFileURL( - path.resolve('/backstageRoot/dist-dynamic/test-backend-plugin'), + mockDir.resolve('backstageRoot/dist-dynamic/test-backend-plugin'), ), manifest: { name: 'test-backend-plugin-dynamic', @@ -540,8 +486,8 @@ Please add '${path.resolve( expectedLogs: { warns: [ { - message: `skipping '${path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin/alpha', + message: `skipping '${mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin/alpha', )}' since it is not a directory`, }, ], @@ -551,40 +497,28 @@ Please add '${path.resolve( name: 'invalid alpha package.json', preferAlpha: true, fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: 'dist/index.cjs.js', - backstage: { role: 'backend-plugin' }, - }), - }), - alpha: mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: "invalid json content, 1, '", - }), - }, - }), - }, - }), + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: 'dist/index.cjs.js', + backstage: { role: 'backend-plugin' }, + }), + alpha: { + 'package.json': "invalid json content, 1, '", }, - }), + }, }, - }), + }, }, expectedPluginPackages: [], expectedLogs: { errors: [ { - message: `failed to load dynamic plugin manifest from '${path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin/alpha', + message: `failed to load dynamic plugin manifest from '${mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin/alpha', )}'`, meta: { name: 'SyntaxError', @@ -597,28 +531,20 @@ Please add '${path.resolve( { name: 'invalid package.json', fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: "invalid json content, 1, '", - }), - }, - }), - }, - }), + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': "invalid json content, 1, '", + }, }, - }), + }, }, expectedPluginPackages: [], expectedLogs: { errors: [ { - message: `failed to load dynamic plugin manifest from '${path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin', + message: `failed to load dynamic plugin manifest from '${mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin', )}'`, meta: { name: 'SyntaxError', @@ -631,32 +557,24 @@ Please add '${path.resolve( { name: 'missing backstage role in package.json', fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - main: 'dist/index.cjs.js', - }), - }), - }, - }), - }, - }), + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: 'dist/index.cjs.js', + }), + }, }, - }), + }, }, expectedPluginPackages: [], expectedLogs: { errors: [ { - message: `failed to load dynamic plugin manifest from '${path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin', + message: `failed to load dynamic plugin manifest from '${mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin', )}'`, meta: { name: 'Error', @@ -669,32 +587,24 @@ Please add '${path.resolve( { name: 'missing main field in package.json', fileSystem: { - '/backstageRoot': mockFs.directory({ - items: { - 'dist-dynamic': mockFs.directory({ - items: { - 'test-backend-plugin': mockFs.directory({ - items: { - 'package.json': mockFs.file({ - content: JSON.stringify({ - name: 'test-backend-plugin-dynamic', - version: '0.0.0', - backstage: { role: 'backend-plugin' }, - }), - }), - }, - }), - }, - }), + backstageRoot: { + 'dist-dynamic': { + 'test-backend-plugin': { + 'package.json': JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + backstage: { role: 'backend-plugin' }, + }), + }, }, - }), + }, }, expectedPluginPackages: [], expectedLogs: { errors: [ { - message: `failed to load dynamic plugin manifest from '${path.resolve( - '/backstageRoot/dist-dynamic/test-backend-plugin', + message: `failed to load dynamic plugin manifest from '${mockDir.resolve( + 'backstageRoot/dist-dynamic/test-backend-plugin', )}'`, meta: { name: 'Error', @@ -706,7 +616,7 @@ Please add '${path.resolve( }, ])('$name', async (tc: TestCase): Promise => { const logger = new MockedLogger(); - const backstageRoot = '/backstageRoot'; + const backstageRoot = mockDir.resolve('backstageRoot'); async function toTest(): Promise { const pluginScanner = new PluginScanner( new ConfigReader( @@ -725,7 +635,12 @@ Please add '${path.resolve( return await pluginScanner.scanRoot(); } if (tc.fileSystem) { - mockFs(tc.fileSystem); + mockDir.setContent(tc.fileSystem); + } + if (tc.symlinks) { + for (const { source, target } of tc.symlinks) { + fs.symlinkSync(target, source); + } } if (tc.expectedError) { /* eslint-disable-next-line jest/no-conditional-expect */ From 9358e965eccee64d0e80b7e06991824ccf697378 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 17:25:01 +0200 Subject: [PATCH 04/17] scaffolder-backend: refactor template action tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../actions/builtin/fetch/template.test.ts | 103 +++++++----------- 1 file changed, 41 insertions(+), 62 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 95f8c7b954..727eb5ce0d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -19,10 +19,8 @@ jest.mock('@backstage/plugin-scaffolder-node', () => { return { ...actual, fetchContents: jest.fn() }; }); -import os from 'os'; -import { join as joinPath, sep as pathSep } from 'path'; +import { join as joinPath, resolve as resolvePath, sep as pathSep } from 'path'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { getVoidLogger, resolvePackagePath, @@ -36,6 +34,7 @@ import { ActionContext, TemplateAction, } from '@backstage/plugin-scaffolder-node'; +import { createMockDirectory } from '@backstage/backend-test-utils'; type FetchTemplateInput = ReturnType< typeof createFetchTemplateAction @@ -43,16 +42,6 @@ type FetchTemplateInput = ReturnType< ? U : never; -const realFiles = Object.fromEntries( - [ - resolvePackagePath( - '@backstage/plugin-scaffolder-backend', - 'assets', - 'nunjucks.js.txt', - ), - ].map(k => [k, mockFs.load(k)]), -); - const aBinaryFile = fs.readFileSync( resolvePackagePath( '@backstage/plugin-scaffolder-backend', @@ -67,14 +56,8 @@ const mockFetchContents = fetchContents as jest.MockedFunction< describe('fetch:template', () => { let action: TemplateAction; - const workspacePath = os.tmpdir(); - const createTemporaryDirectory: jest.MockedFunction< - ActionContext['createTemporaryDirectory'] - > = jest.fn(() => - Promise.resolve( - joinPath(workspacePath, `${createTemporaryDirectory.mock.calls.length}`), - ), - ); + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); const logger = getVoidLogger(); @@ -95,24 +78,21 @@ describe('fetch:template', () => { logStream: new PassThrough(), logger, workspacePath, - createTemporaryDirectory, + async createTemporaryDirectory() { + return fs.mkdtemp(mockDir.resolve('tmp-')); + }, }); beforeEach(() => { - mockFs({ - ...realFiles, + mockDir.setContent({ + workspace: {}, }); - action = createFetchTemplateAction({ reader: Symbol('UrlReader') as unknown as UrlReader, integrations: Symbol('Integrations') as unknown as ScmIntegrations, }); }); - afterEach(() => { - mockFs.restore(); - }); - it(`returns a TemplateAction with the id 'fetch:template'`, () => { expect(action.id).toEqual('fetch:template'); }); @@ -190,8 +170,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { '{% if values.showDummyFile %}dummy-file.txt{% else %}{% endif %}': 'dummy file', @@ -282,13 +261,8 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { - 'an-executable.sh': mockFs.file({ - content: '#!/usr/bin/env bash', - mode: parseInt('100755', 8), - }), 'empty-dir-${{ values.count }}': {}, 'static.txt': 'static content', '${{ values.name }}.txt': 'static content', @@ -298,15 +272,27 @@ describe('fetch:template', () => { }, '.${{ values.name }}': '${{ values.itemList | dump }}', 'a-binary-file.png': aBinaryFile, - symlink: mockFs.symlink({ - path: 'a-binary-file.png', - }), - brokenSymlink: mockFs.symlink({ - path: './not-a-real-file.txt', - }), }, }); + fs.writeFileSync( + resolvePath(outputPath, 'an-executable.sh'), + '#!/usr/bin/env bash', + { + encoding: 'utf-8', + mode: parseInt('100755', 8), + }, + ); + + fs.symlinkSync( + 'a-binary-file.png', + resolvePath(outputPath, 'symlink'), + ); + fs.symlinkSync( + './not-a-real-file.txt', + resolvePath(outputPath, 'brokenSymlink'), + ); + return Promise.resolve(); }); @@ -378,7 +364,11 @@ describe('fetch:template', () => { await expect( fs.realpath(`${workspacePath}/target/symlink`), - ).resolves.toBe(joinPath(workspacePath, 'target', 'a-binary-file.png')); + ).resolves.toBe( + fs.realpathSync( + joinPath(workspacePath, 'target', 'a-binary-file.png'), + ), + ); }); it('copies broken symlinks as-is without processing them', async () => { @@ -408,8 +398,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { processed: { 'templated-content-${{ values.name }}.txt': '${{ values.count }}', @@ -458,8 +447,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { processed: { 'templated-content-${{ values.name }}.txt': '${{ values.count }}', @@ -509,8 +497,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { '{{ cookiecutter.name }}.txt': 'static content', subdir: { @@ -564,8 +551,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { 'empty-dir-${{ values.count }}': {}, 'static.txt': 'static content', @@ -646,8 +632,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { '${{ values.name }}.njk': '${{ values.name }}: ${{ values.count }}', '${{ values.name }}.txt.jinja2': @@ -687,8 +672,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [joinPath(workspacePath, 'target')]: { 'static-content.txt': 'static-content', }, @@ -703,10 +687,6 @@ describe('fetch:template', () => { await action.handler(context); }); - afterEach(() => { - mockFs.restore(); - }); - it('overwrites existing file', async () => { await expect( fs.readFile(`${workspacePath}/target/static-content.txt`, 'utf-8'), @@ -728,8 +708,7 @@ describe('fetch:template', () => { }); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [joinPath(workspacePath, 'target')]: { 'static-content.txt': 'static-content', }, From 688a69ad614b864d51106287ecad8df5506aae05 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 17:26:51 +0200 Subject: [PATCH 05/17] scaffolder-backend: refactor rename action tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../actions/builtin/filesystem/rename.test.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.test.ts index 6804c9f5a9..d37e967f43 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.test.ts @@ -14,20 +14,19 @@ * limitations under the License. */ -import * as os from 'os'; -import mockFs from 'mock-fs'; import { resolve as resolvePath } from 'path'; import { createFilesystemRenameAction } from './rename'; import { getVoidLogger } from '@backstage/backend-common'; import { PassThrough } from 'stream'; import fs from 'fs-extra'; - -const root = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; -const workspacePath = resolvePath(root, 'my-workspace'); +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('fs:rename', () => { const action = createFilesystemRenameAction(); + const mockDir = createMockDirectory(); + const workspacePath = resolvePath(mockDir.path, 'workspace'); + const mockInputFiles = [ { from: 'unit-test-a.js', @@ -56,7 +55,7 @@ describe('fs:rename', () => { beforeEach(() => { jest.restoreAllMocks(); - mockFs({ + mockDir.setContent({ [workspacePath]: { 'unit-test-a.js': 'hello', 'unit-test-b.js': 'world', @@ -68,10 +67,6 @@ describe('fs:rename', () => { }); }); - afterEach(() => { - mockFs.restore(); - }); - it('should throw an error when files is not an array', async () => { await expect( action.handler({ From 2d847c6ce93e2f2257d174f567e29add33d75d60 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 17:27:45 +0200 Subject: [PATCH 06/17] scaffolder-backend: refactor rename action example tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../builtin/filesystem/rename.examples.test.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.examples.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.examples.test.ts index 5139469959..cbb0fa0d0b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.examples.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/rename.examples.test.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import * as os from 'os'; -import mockFs from 'mock-fs'; import { resolve as resolvePath } from 'path'; import { createFilesystemRenameAction } from './rename'; import { getVoidLogger } from '@backstage/backend-common'; @@ -23,15 +21,16 @@ import { PassThrough } from 'stream'; import fs from 'fs-extra'; import yaml from 'yaml'; import { examples } from './rename.examples'; - -const root = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; -const workspacePath = resolvePath(root, 'my-workspace'); +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('fs:rename examples', () => { const action = createFilesystemRenameAction(); const files: { from: string; to: string }[] = yaml.parse(examples[0].example) .steps[0].input.files; + const mockDir = createMockDirectory(); + const workspacePath = resolvePath(mockDir.path, 'workspace'); + const mockContext = { input: { files: files, @@ -46,7 +45,7 @@ describe('fs:rename examples', () => { beforeEach(() => { jest.restoreAllMocks(); - mockFs({ + mockDir.setContent({ [workspacePath]: { [files[0].from]: 'hello', [files[1].from]: 'world', @@ -59,10 +58,6 @@ describe('fs:rename examples', () => { }); }); - afterEach(() => { - mockFs.restore(); - }); - it('should call fs.move with the correct values', async () => { mockContext.input.files.forEach(file => { const filePath = resolvePath(workspacePath, file.from); From d95f8092fadf4317f3120259296e7ba2519e4f99 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:06:53 +0200 Subject: [PATCH 07/17] scaffolder-backend: refactor delete action tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../builtin/filesystem/delete.examples.test.ts | 15 +++++---------- .../actions/builtin/filesystem/delete.test.ts | 15 +++++---------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.examples.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.examples.test.ts index 10994e3824..023f049ebc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.examples.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.examples.test.ts @@ -18,18 +18,17 @@ import { createFilesystemDeleteAction } from './delete'; import { getVoidLogger } from '@backstage/backend-common'; import { PassThrough } from 'stream'; import { resolve as resolvePath } from 'path'; -import * as os from 'os'; -import mockFs from 'mock-fs'; import fs from 'fs-extra'; import yaml from 'yaml'; import { examples } from './delete.examples'; - -const root = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; -const workspacePath = resolvePath(root, 'my-workspace'); +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('fs:delete examples', () => { const action = createFilesystemDeleteAction(); + const mockDir = createMockDirectory(); + const workspacePath = resolvePath(mockDir.path, 'workspace'); + const files: string[] = yaml.parse(examples[0].example).steps[0].input.files; const mockContext = { @@ -46,7 +45,7 @@ describe('fs:delete examples', () => { beforeEach(() => { jest.restoreAllMocks(); - mockFs({ + mockDir.setContent({ [workspacePath]: { [files[0]]: 'hello', [files[1]]: 'world', @@ -57,10 +56,6 @@ describe('fs:delete examples', () => { }); }); - afterEach(() => { - mockFs.restore(); - }); - it('should call fs.rm with the correct values', async () => { files.forEach(file => { const filePath = resolvePath(workspacePath, file); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.test.ts index 713de5b0ad..2a4f45b862 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.test.ts @@ -14,20 +14,19 @@ * limitations under the License. */ -import * as os from 'os'; -import mockFs from 'mock-fs'; import { resolve as resolvePath } from 'path'; import { createFilesystemDeleteAction } from './delete'; import { getVoidLogger } from '@backstage/backend-common'; import { PassThrough } from 'stream'; import fs from 'fs-extra'; - -const root = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; -const workspacePath = resolvePath(root, 'my-workspace'); +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('fs:delete', () => { const action = createFilesystemDeleteAction(); + const mockDir = createMockDirectory(); + const workspacePath = resolvePath(mockDir.path, 'workspace'); + const mockContext = { input: { files: ['unit-test-a.js', 'unit-test-b.js'], @@ -42,7 +41,7 @@ describe('fs:delete', () => { beforeEach(() => { jest.restoreAllMocks(); - mockFs({ + mockDir.setContent({ [workspacePath]: { 'unit-test-a.js': 'hello', 'unit-test-b.js': 'world', @@ -53,10 +52,6 @@ describe('fs:delete', () => { }); }); - afterEach(() => { - mockFs.restore(); - }); - it('should throw an error when files is not an array', async () => { await expect( action.handler({ From d49bd45d69efc7b825a7d9bb0ac4a2be34315ba1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:29:15 +0200 Subject: [PATCH 08/17] backend-test-utils: add MockDirectory content callback + default file modes Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 17 ++++++- .../src/filesystem/MockDirectory.test.ts | 14 ++++++ .../src/filesystem/MockDirectory.ts | 49 +++++++++++++++++-- .../src/filesystem/index.ts | 2 + 4 files changed, 77 insertions(+), 5 deletions(-) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index a477f70298..ac012e2f69 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -54,9 +54,24 @@ export interface MockDirectory { // @public export type MockDirectoryContent = { - [name in string]: MockDirectoryContent | string | Buffer; + [name in string]: + | MockDirectoryContent + | string + | Buffer + | MockDirectoryContentCallback; }; +// @public +export type MockDirectoryContentCallback = ( + ctx: MockDirectoryContentCallbackContext, +) => void; + +// @public +export interface MockDirectoryContentCallbackContext { + path: string; + symlink(target: string): void; +} + // @public export interface MockDirectoryContentOptions { path?: string; diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 8040d03f44..fd22f09432 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -119,6 +119,20 @@ describe('createMockDirectory', () => { }); }); + it('should be able to use callback for more detailed file system operations', () => { + mockDir.setContent({ + 'a.txt': 'a', + 'b.txt': ctx => ctx.symlink('./a.txt'), + 'c.txt': ctx => fs.copyFileSync(mockDir.resolve('a.txt'), ctx.path), + }); + + expect(mockDir.content()).toEqual({ + 'a.txt': 'a', + 'b.txt': 'a', + 'c.txt': 'a', + }); + }); + it('should read content from sub dirs', () => { mockDir.setContent({ 'a.txt': 'a', diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 61eb4fce24..b066b9acd5 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -30,6 +30,28 @@ import { const tmpdirMarker = Symbol('os-tmpdir-mock'); +/** + * A context that allows for more advanced file system operations when writing mock directory content. + * + * @public + */ +export interface MockDirectoryContentCallbackContext { + /** Absolute path to the location of this piece of content on the filesystem */ + path: string; + + /** Creates a symbolic link at the current location */ + symlink(target: string): void; +} + +/** + * A callback that allows for more advanced file system operations when writing mock directory content. + * + * @public + */ +export type MockDirectoryContentCallback = ( + ctx: MockDirectoryContentCallbackContext, +) => void; + /** * The content of a mock directory represented by a nested object structure. * @@ -54,7 +76,11 @@ const tmpdirMarker = Symbol('os-tmpdir-mock'); * @public */ export type MockDirectoryContent = { - [name in string]: MockDirectoryContent | string | Buffer; + [name in string]: + | MockDirectoryContent + | string + | Buffer + | MockDirectoryContentCallback; }; /** @@ -178,6 +204,11 @@ type MockEntry = | { type: 'dir'; path: string; + } + | { + type: 'callback'; + path: string; + callback: MockDirectoryContentCallback; }; /** @internal */ @@ -214,10 +245,18 @@ class MockDirectoryImpl { } if (entry.type === 'dir') { - fs.ensureDirSync(fullPath, { mode: 0o777 }); + fs.ensureDirSync(fullPath); } else if (entry.type === 'file') { - fs.ensureDirSync(dirname(fullPath), { mode: 0o777 }); - fs.writeFileSync(fullPath, entry.content, { mode: 0o666 }); + fs.ensureDirSync(dirname(fullPath)); + fs.writeFileSync(fullPath, entry.content); + } else if (entry.type === 'callback') { + fs.ensureDirSync(dirname(fullPath)); + entry.callback({ + path: fullPath, + symlink(target: string) { + fs.symlinkSync(target, fullPath); + }, + }); } } } @@ -287,6 +326,8 @@ class MockDirectoryImpl { }); } else if (node instanceof Buffer) { entries.push({ type: 'file', path, content: node }); + } else if (typeof node === 'function') { + entries.push({ type: 'callback', path, callback: node }); } else { entries.push({ type: 'dir', path }); for (const [name, child] of Object.entries(node)) { diff --git a/packages/backend-test-utils/src/filesystem/index.ts b/packages/backend-test-utils/src/filesystem/index.ts index 0a4d8c7d00..e18b0b55b8 100644 --- a/packages/backend-test-utils/src/filesystem/index.ts +++ b/packages/backend-test-utils/src/filesystem/index.ts @@ -20,4 +20,6 @@ export { type MockDirectoryOptions, type MockDirectoryContent, type MockDirectoryContentOptions, + type MockDirectoryContentCallback, + type MockDirectoryContentCallbackContext, } from './MockDirectory'; From 3bc42d968aa7f06b01ab41757ce11996efbef5ec Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:32:28 +0200 Subject: [PATCH 09/17] scaffolder-backend: refactor file serialization tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../deserializeDirectoryContents.test.ts | 20 ++- .../files/serializeDirectoryContents.test.ts | 114 ++++++++---------- 2 files changed, 55 insertions(+), 79 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts b/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts index c272e04205..e374e0d713 100644 --- a/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts +++ b/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts @@ -14,29 +14,25 @@ * limitations under the License. */ -import mockFs from 'mock-fs'; +import { createMockDirectory } from '@backstage/backend-test-utils'; import { deserializeDirectoryContents } from './deserializeDirectoryContents'; import { serializeDirectoryContents } from './serializeDirectoryContents'; describe('deserializeDirectoryContents', () => { - beforeEach(() => { - mockFs({ - root: {}, - }); - }); + const mockDir = createMockDirectory(); - afterEach(() => { - mockFs.restore(); + beforeEach(() => { + mockDir.clear(); }); it('deserializes contents into a directory', async () => { - await deserializeDirectoryContents('root', [ + await deserializeDirectoryContents(mockDir.path, [ { path: 'a.txt', content: Buffer.from('a', 'utf8'), }, ]); - await expect(serializeDirectoryContents('root')).resolves.toEqual([ + await expect(serializeDirectoryContents(mockDir.path)).resolves.toEqual([ { path: 'a.txt', content: Buffer.from('a', 'utf8'), @@ -47,7 +43,7 @@ describe('deserializeDirectoryContents', () => { }); it('deserializes contents into a deep directory structure', async () => { - await deserializeDirectoryContents('root', [ + await deserializeDirectoryContents(mockDir.path, [ { path: 'a.txt', content: Buffer.from('a', 'utf8'), @@ -61,7 +57,7 @@ describe('deserializeDirectoryContents', () => { content: Buffer.from('c', 'utf8'), }, ]); - await expect(serializeDirectoryContents('root')).resolves.toEqual([ + await expect(serializeDirectoryContents(mockDir.path)).resolves.toEqual([ { path: 'a.txt', content: Buffer.from('a', 'utf8'), diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts index a22ea85301..afc1b755ff 100644 --- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts +++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts @@ -14,13 +14,11 @@ * limitations under the License. */ +import { createMockDirectory } from '@backstage/backend-test-utils'; import { serializeDirectoryContents } from './serializeDirectoryContents'; -import mockFs from 'mock-fs'; describe('serializeDirectoryContents', () => { - afterEach(() => { - mockFs.restore(); - }); + const mockDir = createMockDirectory(); it('should list files in this directory', async () => { await expect(serializeDirectoryContents(__dirname)).resolves.toEqual( @@ -54,25 +52,23 @@ describe('serializeDirectoryContents', () => { }); it('should list files in a mock directory', async () => { - mockFs({ - root: { - 'a.txt': 'a', - b: { - 'b1.txt': 'b1', - 'b2.txt': 'b2', - }, - c: { - c1: { - 'c11.txt': 'c11', - c11: { - 'c111.txt': 'c111', - }, + mockDir.setContent({ + 'a.txt': 'a', + b: { + 'b1.txt': 'b1', + 'b2.txt': 'b2', + }, + c: { + c1: { + 'c11.txt': 'c11', + c11: { + 'c111.txt': 'c111', }, }, }, }); - await expect(serializeDirectoryContents('root')).resolves.toEqual([ + await expect(serializeDirectoryContents(mockDir.path)).resolves.toEqual([ { path: 'a.txt', executable: false, @@ -107,16 +103,12 @@ describe('serializeDirectoryContents', () => { }); it('should ignore symlinked files', async () => { - mockFs({ - root: { - 'a.txt': 'some text', - sym: mockFs.symlink({ - path: './a.txt', - }), - }, + mockDir.setContent({ + 'a.txt': 'some text', + sym: ctx => ctx.symlink('./a.txt'), }); - await expect(serializeDirectoryContents('root')).resolves.toEqual([ + await expect(serializeDirectoryContents(mockDir.path)).resolves.toEqual([ { path: 'a.txt', executable: false, @@ -127,18 +119,14 @@ describe('serializeDirectoryContents', () => { }); it('should pick up broken symlinks', async () => { - mockFs({ - root: { - 'b.txt': mockFs.symlink({ - path: './a.txt', - }), - }, + mockDir.setContent({ + 'b.txt': ctx => ctx.symlink('./a.txt'), }); - await expect(serializeDirectoryContents('root')).resolves.toEqual([ + await expect(serializeDirectoryContents(mockDir.path)).resolves.toEqual([ { path: 'b.txt', - executable: false, + executable: true, symlink: true, content: Buffer.from('./a.txt', 'utf8'), }, @@ -146,19 +134,15 @@ describe('serializeDirectoryContents', () => { }); it('should ignore symlinked folder files', async () => { - mockFs({ - root: { - 'a.txt': 'some text', - linkme: { - 'b.txt': 'lols', - }, - sym: mockFs.symlink({ - path: './linkme', - }), + mockDir.setContent({ + 'a.txt': 'some text', + linkme: { + 'b.txt': 'lols', }, + sym: ctx => ctx.symlink('./linkme'), }); - await expect(serializeDirectoryContents('root')).resolves.toEqual([ + await expect(serializeDirectoryContents(mockDir.path)).resolves.toEqual([ { path: 'a.txt', executable: false, @@ -175,16 +159,14 @@ describe('serializeDirectoryContents', () => { }); it('should ignore gitignored files', async () => { - mockFs({ - root: { - '.gitignore': '*.txt', - 'a.txt': 'a', - 'a.log': 'a', - }, + mockDir.setContent({ + '.gitignore': '*.txt', + 'a.txt': 'a', + 'a.log': 'a', }); await expect( - serializeDirectoryContents('root', { + serializeDirectoryContents(mockDir.path, { gitignore: true, }), ).resolves.toEqual([ @@ -204,26 +186,24 @@ describe('serializeDirectoryContents', () => { }); it('should use custom glob patterns', async () => { - mockFs({ - root: { - '.a': 'a', - 'a.log': 'a', - 'a.txt': 'a', - b: { - '.b': 'b', - 'b.log': 'b', - 'b.txt': 'b', - }, - c: { - '.c': 'c', - 'c.log': 'c', - 'c.txt': 'c', - }, + mockDir.setContent({ + '.a': 'a', + 'a.log': 'a', + 'a.txt': 'a', + b: { + '.b': 'b', + 'b.log': 'b', + 'b.txt': 'b', + }, + c: { + '.c': 'c', + 'c.log': 'c', + 'c.txt': 'c', }, }); await expect( - serializeDirectoryContents('root', { + serializeDirectoryContents(mockDir.path, { gitignore: true, globPatterns: ['**/*.txt', '*/.?', '*/*.log', '!c/**/.*', '!b/*.log'], }).then(files => files.sort((a, b) => a.path.localeCompare(b.path))), From 4c6d8cd1f367247e197dc67cef7618507a905bdf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:40:31 +0200 Subject: [PATCH 10/17] use new MockDirectory symlink helper Signed-off-by: Patrik Oldsberg --- .../src/manager/plugin-manager.test.ts | 7 +-- .../src/scanner/plugin-scanner.test.ts | 48 +++++++------------ .../actions/builtin/fetch/template.test.ts | 27 ++++------- 3 files changed, 27 insertions(+), 55 deletions(-) diff --git a/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts b/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts index f356163dea..4aabd2c072 100644 --- a/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts +++ b/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts @@ -452,16 +452,13 @@ describe('backend-plugin-manager', () => { findPaths(__dirname).resolveTargetRoot('package.json'), ), 'dynamic-plugins-root': {}, + 'dynamic-plugins-root/a-dynamic-plugin': ctx => + ctx.symlink(otherMockDir.resolve('a-dynamic-plugin')), }); otherMockDir.setContent({ 'a-dynamic-plugin': {}, }); - fs.symlinkSync( - otherMockDir.resolve('a-dynamic-plugin'), - mockDir.resolve('dynamic-plugins-root/a-dynamic-plugin'), - ); - const fromConfigSpier = jest.spyOn(PluginManager, 'fromConfig'); const applyConfigSpier = jest .spyOn(PluginScanner.prototype as any, 'applyConfig') diff --git a/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts b/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts index 04ce69ec98..d4ab8940c9 100644 --- a/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts +++ b/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts @@ -19,10 +19,12 @@ import { JsonObject } from '@backstage/types'; import { Logs, MockedLogger } from '../__testUtils__/testUtils'; import { ConfigReader } from '@backstage/config'; import path from 'path'; -import fs from 'fs'; import * as url from 'url'; import { ScannedPluginPackage } from './types'; -import { createMockDirectory } from '@backstage/backend-test-utils'; +import { + MockDirectoryContent, + createMockDirectory, +} from '@backstage/backend-test-utils'; const mockDir = createMockDirectory(); @@ -233,8 +235,7 @@ Please add '${mockDir.resolve( type TestCase = { name: string; preferAlpha?: boolean; - fileSystem?: any; - symlinks?: { source: string; target: string }[]; + fileSystem?: MockDirectoryContent; expectedLogs?: Logs; expectedPluginPackages?: ScannedPluginPackage[]; expectedError?: string; @@ -286,7 +287,12 @@ Please add '${mockDir.resolve( name: 'backend plugin found in symlink', fileSystem: { backstageRoot: { - 'dist-dynamic': {}, + 'dist-dynamic': { + 'test-backend-plugin': ctx => + ctx.symlink( + mockDir.resolve('somewhere-else/test-backend-plugin-target'), + ), + }, }, 'somewhere-else': { 'test-backend-plugin-target': { @@ -299,16 +305,6 @@ Please add '${mockDir.resolve( }, }, }, - symlinks: [ - { - source: mockDir.resolve( - 'backstageRoot/dist-dynamic/test-backend-plugin', - ), - target: mockDir.resolve( - 'somewhere-else/test-backend-plugin-target', - ), - }, - ], expectedPluginPackages: [ { location: url.pathToFileURL( @@ -347,22 +343,17 @@ Please add '${mockDir.resolve( name: 'ignored folder child symlink: target is not a directory', fileSystem: { backstageRoot: { - 'dist-dynamic': {}, + 'dist-dynamic': { + 'test-backend-plugin': ctx => + ctx.symlink( + mockDir.resolve('somewhere-else/test-backend-plugin-target'), + ), + }, }, 'somewhere-else': { 'test-backend-plugin-target': '', }, }, - symlinks: [ - { - source: mockDir.resolve( - 'backstageRoot/dist-dynamic/test-backend-plugin', - ), - target: mockDir.resolve( - 'somewhere-else/test-backend-plugin-target', - ), - }, - ], expectedPluginPackages: [], expectedLogs: { infos: [ @@ -637,11 +628,6 @@ Please add '${mockDir.resolve( if (tc.fileSystem) { mockDir.setContent(tc.fileSystem); } - if (tc.symlinks) { - for (const { source, target } of tc.symlinks) { - fs.symlinkSync(target, source); - } - } if (tc.expectedError) { /* eslint-disable-next-line jest/no-conditional-expect */ expect(toTest).toThrow(tc.expectedError); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 727eb5ce0d..d396a00c26 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -19,7 +19,7 @@ jest.mock('@backstage/plugin-scaffolder-node', () => { return { ...actual, fetchContents: jest.fn() }; }); -import { join as joinPath, resolve as resolvePath, sep as pathSep } from 'path'; +import { join as joinPath, sep as pathSep } from 'path'; import fs from 'fs-extra'; import { getVoidLogger, @@ -272,27 +272,16 @@ describe('fetch:template', () => { }, '.${{ values.name }}': '${{ values.itemList | dump }}', 'a-binary-file.png': aBinaryFile, + 'an-executable.sh': ctx => + fs.writeFileSync(ctx.path, '#!/usr/bin/env bash', { + encoding: 'utf-8', + mode: parseInt('100755', 8), + }), + symlink: ctx => ctx.symlink('a-binary-file.png'), + brokenSymlink: ctx => ctx.symlink('./not-a-real-file.txt'), }, }); - fs.writeFileSync( - resolvePath(outputPath, 'an-executable.sh'), - '#!/usr/bin/env bash', - { - encoding: 'utf-8', - mode: parseInt('100755', 8), - }, - ); - - fs.symlinkSync( - 'a-binary-file.png', - resolvePath(outputPath, 'symlink'), - ); - fs.symlinkSync( - './not-a-real-file.txt', - resolvePath(outputPath, 'brokenSymlink'), - ); - return Promise.resolve(); }); From aefca1e6d0a50f38fb9dfb909b225f6099c8eafd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:45:36 +0200 Subject: [PATCH 11/17] scaffolder-backend: refactor debug actions tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../builtin/debug/log.examples.test.ts | 21 ++++++++----------- .../actions/builtin/debug/log.test.ts | 17 +++++++-------- .../builtin/debug/wait.examples.test.ts | 15 ++++++------- .../actions/builtin/debug/wait.test.ts | 15 ++++++------- 4 files changed, 28 insertions(+), 40 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts index a7f5a1804a..901f3e5011 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts @@ -15,44 +15,41 @@ */ import { getVoidLogger } from '@backstage/backend-common'; -import mockFs from 'mock-fs'; -import os from 'os'; import { Writable } from 'stream'; import { createDebugLogAction } from './log'; import { join } from 'path'; import yaml from 'yaml'; import { examples } from './log.examples'; +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('debug:log examples', () => { const logStream = { write: jest.fn(), } as jest.Mocked> as jest.Mocked; - const mockTmpDir = os.tmpdir(); + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); + const mockContext = { input: {}, baseUrl: 'somebase', - workspacePath: mockTmpDir, + workspacePath, logger: getVoidLogger(), logStream, output: jest.fn(), - createTemporaryDirectory: jest.fn().mockResolvedValue(mockTmpDir), + createTemporaryDirectory: jest.fn(), }; const action = createDebugLogAction(); beforeEach(() => { - mockFs({ - [`${mockContext.workspacePath}/README.md`]: '', - [`${mockContext.workspacePath}/a-directory/index.md`]: '', + mockDir.setContent({ + [`${workspacePath}/README.md`]: '', + [`${workspacePath}/a-directory/index.md`]: '', }); jest.resetAllMocks(); }); - afterEach(() => { - mockFs.restore(); - }); - it('should log message', async () => { const context = { ...mockContext, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts index 6b7ad8816f..d6fc5174b3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts @@ -15,43 +15,40 @@ */ import { getVoidLogger } from '@backstage/backend-common'; -import mockFs from 'mock-fs'; -import os from 'os'; import { Writable } from 'stream'; import { createDebugLogAction } from './log'; import { join } from 'path'; import yaml from 'yaml'; +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('debug:log', () => { const logStream = { write: jest.fn(), } as jest.Mocked> as jest.Mocked; - const mockTmpDir = os.tmpdir(); + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); + const mockContext = { input: {}, baseUrl: 'somebase', - workspacePath: mockTmpDir, + workspacePath, logger: getVoidLogger(), logStream, output: jest.fn(), - createTemporaryDirectory: jest.fn().mockResolvedValue(mockTmpDir), + createTemporaryDirectory: jest.fn(), }; const action = createDebugLogAction(); beforeEach(() => { - mockFs({ + mockDir.setContent({ [`${mockContext.workspacePath}/README.md`]: '', [`${mockContext.workspacePath}/a-directory/index.md`]: '', }); jest.resetAllMocks(); }); - afterEach(() => { - mockFs.restore(); - }); - it('should do nothing', async () => { await action.handler(mockContext); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.examples.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.examples.test.ts index 595ae7c3b6..11cc83c696 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.examples.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.examples.test.ts @@ -15,12 +15,11 @@ */ import { getVoidLogger } from '@backstage/backend-common'; -import mockFs from 'mock-fs'; import { createWaitAction } from './wait'; import { Writable } from 'stream'; -import os from 'os'; import { examples } from './wait.examples'; import yaml from 'yaml'; +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('debug:wait examples', () => { const action = createWaitAction(); @@ -29,25 +28,23 @@ describe('debug:wait examples', () => { write: jest.fn(), } as jest.Mocked> as jest.Mocked; - const mockTmpDir = os.tmpdir(); + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); + const mockContext = { input: {}, baseUrl: 'somebase', - workspacePath: mockTmpDir, + workspacePath, logger: getVoidLogger(), logStream, output: jest.fn(), - createTemporaryDirectory: jest.fn().mockResolvedValue(mockTmpDir), + createTemporaryDirectory: jest.fn(), }; beforeEach(() => { jest.resetAllMocks(); }); - afterEach(() => { - mockFs.restore(); - }); - it('should wait for specified period of seconds', async () => { const context = { ...mockContext, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.test.ts index 0d4cdaba4b..6f80604a6d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/wait.test.ts @@ -15,10 +15,9 @@ */ import { getVoidLogger } from '@backstage/backend-common'; -import mockFs from 'mock-fs'; import { createWaitAction } from './wait'; import { Writable } from 'stream'; -import os from 'os'; +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('debug:wait', () => { const action = createWaitAction(); @@ -27,25 +26,23 @@ describe('debug:wait', () => { write: jest.fn(), } as jest.Mocked> as jest.Mocked; - const mockTmpDir = os.tmpdir(); + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); + const mockContext = { input: {}, baseUrl: 'somebase', - workspacePath: mockTmpDir, + workspacePath, logger: getVoidLogger(), logStream, output: jest.fn(), - createTemporaryDirectory: jest.fn().mockResolvedValue(mockTmpDir), + createTemporaryDirectory: jest.fn(), }; beforeEach(() => { jest.resetAllMocks(); }); - afterEach(() => { - mockFs.restore(); - }); - it('should wait for specified period of time', async () => { const context = { ...mockContext, From 82458b16ccc8e1bd173db051b15ac8ccc56852c3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:48:12 +0200 Subject: [PATCH 12/17] scaffolder-backend: refactor template action example tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../builtin/fetch/template.examples.test.ts | 63 +++++++------------ 1 file changed, 21 insertions(+), 42 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.examples.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.examples.test.ts index 43bed5a048..a1c8381178 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.examples.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.examples.test.ts @@ -14,10 +14,8 @@ * limitations under the License. */ -import os from 'os'; import { join as joinPath, sep as pathSep } from 'path'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { getVoidLogger, resolvePackagePath, @@ -33,6 +31,7 @@ import { } from '@backstage/plugin-scaffolder-node'; import { examples } from './template.examples'; import yaml from 'yaml'; +import { createMockDirectory } from '@backstage/backend-test-utils'; jest.mock('@backstage/plugin-scaffolder-node', () => ({ ...jest.requireActual('@backstage/plugin-scaffolder-node'), @@ -45,16 +44,6 @@ type FetchTemplateInput = ReturnType< ? U : never; -const realFiles = Object.fromEntries( - [ - resolvePackagePath( - '@backstage/plugin-scaffolder-backend', - 'assets', - 'nunjucks.js.txt', - ), - ].map(k => [k, mockFs.load(k)]), -); - const aBinaryFile = fs.readFileSync( resolvePackagePath( '@backstage/plugin-scaffolder-backend', @@ -69,14 +58,8 @@ const mockFetchContents = fetchContents as jest.MockedFunction< describe('fetch:template examples', () => { let action: TemplateAction; - const workspacePath = os.tmpdir(); - const createTemporaryDirectory: jest.MockedFunction< - ActionContext['createTemporaryDirectory'] - > = jest.fn(() => - Promise.resolve( - joinPath(workspacePath, `${createTemporaryDirectory.mock.calls.length}`), - ), - ); + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); const logger = getVoidLogger(); @@ -90,24 +73,20 @@ describe('fetch:template examples', () => { logStream: new PassThrough(), logger, workspacePath, - createTemporaryDirectory, + + async createTemporaryDirectory() { + return fs.mkdtemp(mockDir.resolve('tmp-')); + }, }); beforeEach(() => { - mockFs({ - ...realFiles, - }); - + mockDir.clear(); action = createFetchTemplateAction({ reader: Symbol('UrlReader') as unknown as UrlReader, integrations: Symbol('Integrations') as unknown as ScmIntegrations, }); }); - afterEach(() => { - mockFs.restore(); - }); - describe('handler', () => { describe('with valid input', () => { let context: ActionContext; @@ -116,13 +95,13 @@ describe('fetch:template examples', () => { context = mockContext(yaml.parse(examples[0].example).steps[0].input); mockFetchContents.mockImplementation(({ outputPath }) => { - mockFs({ - ...realFiles, + mockDir.setContent({ [outputPath]: { - 'an-executable.sh': mockFs.file({ - content: '#!/usr/bin/env bash', - mode: parseInt('100755', 8), - }), + 'an-executable.sh': ctx => + fs.writeFileSync(ctx.path, '#!/usr/bin/env bash', { + encoding: 'utf8', + mode: parseInt('100755', 8), + }), 'empty-dir-${{ values.count }}': {}, 'static.txt': 'static content', '${{ values.name }}.txt': 'static content', @@ -132,12 +111,8 @@ describe('fetch:template examples', () => { }, '.${{ values.name }}': '${{ values.itemList | dump }}', 'a-binary-file.png': aBinaryFile, - symlink: mockFs.symlink({ - path: 'a-binary-file.png', - }), - brokenSymlink: mockFs.symlink({ - path: './not-a-real-file.txt', - }), + symlink: ctx => ctx.symlink('a-binary-file.png'), + brokenSymlink: ctx => ctx.symlink('./not-a-real-file.txt'), }, }); @@ -212,7 +187,11 @@ describe('fetch:template examples', () => { await expect( fs.realpath(`${workspacePath}/target/symlink`), - ).resolves.toBe(joinPath(workspacePath, 'target', 'a-binary-file.png')); + ).resolves.toBe( + fs.realpathSync( + joinPath(workspacePath, 'target', 'a-binary-file.png'), + ), + ); }); it('copies broken symlinks as-is without processing them', async () => { From 53d110c055af1cfab2c004a35c1396da92032708 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:51:09 +0200 Subject: [PATCH 13/17] scaffolder-backend: refactor github action tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../builtin/publish/githubPullRequest.test.ts | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts index b6987335f9..a1f3712a9d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts @@ -24,21 +24,17 @@ import { ActionContext, TemplateAction, } from '@backstage/plugin-scaffolder-node'; -import mockFs from 'mock-fs'; -import os from 'os'; -import { resolve as resolvePath } from 'path'; +import fs from 'fs-extra'; import { Writable } from 'stream'; import { createPublishGithubPullRequestAction, OctokitWithPullRequestPluginClient, } from './githubPullRequest'; +import { createMockDirectory } from '@backstage/backend-test-utils'; // Make sure root logger is initialized ahead of FS mock createRootLogger(); -const root = os.platform() === 'win32' ? 'C:\\root' : '/root'; -const workspacePath = resolvePath(root, 'my-workspace'); - type GithubPullRequestActionInput = ReturnType< typeof createPublishGithubPullRequestAction > extends TemplateAction @@ -54,7 +50,12 @@ describe('createPublishGithubPullRequestAction', () => { }; }; + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); + beforeEach(() => { + mockDir.clear(); + const integrations = ScmIntegrations.fromConfig(new ConfigReader({})); fakeClient = { createPullRequest: jest.fn(async (_: any) => { @@ -92,7 +93,6 @@ describe('createPublishGithubPullRequestAction', () => { }); afterEach(() => { - mockFs.restore(); jest.resetAllMocks(); }); @@ -132,7 +132,7 @@ describe('createPublishGithubPullRequestAction', () => { draft: true, }; - mockFs({ + mockDir.setContent({ [workspacePath]: { 'file.txt': 'Hello there!' }, }); @@ -197,7 +197,7 @@ describe('createPublishGithubPullRequestAction', () => { draft: true, }; - mockFs({ + mockDir.setContent({ [workspacePath]: { 'file.txt': 'Hello there!' }, }); @@ -261,7 +261,7 @@ describe('createPublishGithubPullRequestAction', () => { sourcePath: 'source', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -323,7 +323,7 @@ describe('createPublishGithubPullRequestAction', () => { description: 'This PR is really good', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { 'file.txt': 'Hello there!' }, }); @@ -385,7 +385,7 @@ describe('createPublishGithubPullRequestAction', () => { teamReviewers: ['team-foo'], }; - mockFs({ [workspacePath]: {} }); + mockDir.setContent({ [workspacePath]: {} }); ctx = { createTemporaryDirectory: jest.fn(), @@ -437,7 +437,7 @@ describe('createPublishGithubPullRequestAction', () => { description: 'This PR is really good', }; - mockFs({ [workspacePath]: {} }); + mockDir.setContent({ [workspacePath]: {} }); ctx = { createTemporaryDirectory: jest.fn(), @@ -469,11 +469,9 @@ describe('createPublishGithubPullRequestAction', () => { description: 'This PR is really good', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { - Makefile: mockFs.symlink({ - path: '../../nothing/yet', - }), + Makefile: c => c.symlink('../../nothing/yet'), }, }); @@ -523,12 +521,13 @@ describe('createPublishGithubPullRequestAction', () => { description: 'This PR is really good', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { - 'hello.sh': mockFs.file({ - content: 'echo Hello there!', - mode: 0o100755, - }), + 'hello.sh': c => + fs.writeFileSync(c.path, 'echo Hello there!', { + encoding: 'utf8', + mode: 0o100755, + }), }, }); @@ -588,12 +587,13 @@ describe('createPublishGithubPullRequestAction', () => { description: 'This PR is really good', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { - 'hello.sh': mockFs.file({ - content: 'echo Hello there!', - mode: 0o100775, - }), + 'hello.sh': c => + fs.writeFileSync(c.path, 'echo Hello there!', { + encoding: 'utf8', + mode: 0o100775, + }), }, }); @@ -654,7 +654,7 @@ describe('createPublishGithubPullRequestAction', () => { commitMessage: 'Create my new app, but in the commit message', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { 'file.txt': 'Hello there!' }, }); From 7486c28163202955c740669dd00992b415e8a24a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:52:07 +0200 Subject: [PATCH 14/17] scaffolder-backend: refactor gitlab action tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../publish/gitlabMergeRequest.test.ts | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts index 6445d243b5..71a2273aed 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts @@ -17,18 +17,13 @@ import { createRootLogger, getRootLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { ScmIntegrations } from '@backstage/integration'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; -import mockFs from 'mock-fs'; -import os from 'os'; -import { resolve as resolvePath } from 'path'; import { Writable } from 'stream'; import { createPublishGitlabMergeRequestAction } from './gitlabMergeRequest'; +import { createMockDirectory } from '@backstage/backend-test-utils'; // Make sure root logger is initialized ahead of FS mock createRootLogger(); -const root = os.platform() === 'win32' ? 'C:\\root' : '/root'; -const workspacePath = resolvePath(root, 'my-workspace'); - const mockGitlabClient = { Namespaces: { show: jest.fn(), @@ -79,7 +74,12 @@ jest.mock('@gitbeaker/node', () => ({ describe('createGitLabMergeRequest', () => { let instance: TemplateAction; + const mockDir = createMockDirectory(); + const workspacePath = mockDir.resolve('workspace'); + beforeEach(() => { + mockDir.clear(); + const config = new ConfigReader({ integrations: { gitlab: [ @@ -100,10 +100,6 @@ describe('createGitLabMergeRequest', () => { instance = createPublishGitlabMergeRequestAction({ integrations }); }); - afterEach(() => { - mockFs.restore(); - }); - describe('createGitLabMergeRequestWithSpecifiedTargetBranch', () => { it('removeSourceBranch is false by default when not passed in options', async () => { const input = { @@ -114,7 +110,7 @@ describe('createGitLabMergeRequest', () => { description: 'This MR is really good', targetPath: 'Subdirectory', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -156,7 +152,7 @@ describe('createGitLabMergeRequest', () => { description: 'This MR is really good', targetPath: 'Subdirectory', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -200,7 +196,7 @@ describe('createGitLabMergeRequest', () => { removeSourceBranch: true, targetPath: 'Subdirectory', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -235,7 +231,7 @@ describe('createGitLabMergeRequest', () => { removeSourceBranch: false, targetPath: 'Subdirectory', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -276,7 +272,7 @@ describe('createGitLabMergeRequest', () => { targetPath: 'Subdirectory', assignee: 'John Smith', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -316,7 +312,7 @@ describe('createGitLabMergeRequest', () => { targetPath: 'Subdirectory', assingnee: 'John Doe', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -356,7 +352,7 @@ describe('createGitLabMergeRequest', () => { removeSourceBranch: false, targetPath: 'Subdirectory', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -395,7 +391,7 @@ describe('createGitLabMergeRequest', () => { targetPath: 'Subdirectory', assignee: 'Unknown', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -431,7 +427,7 @@ describe('createGitLabMergeRequest', () => { branchName: 'new-mr', description: 'This MR is really good', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -480,7 +476,7 @@ describe('createGitLabMergeRequest', () => { description: 'This MR is really good', targetPath: 'source', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -523,7 +519,7 @@ describe('createGitLabMergeRequest', () => { commitAction: 'create', targetPath: 'source', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -565,7 +561,7 @@ describe('createGitLabMergeRequest', () => { commitAction: 'update', targetPath: 'source', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -607,7 +603,7 @@ describe('createGitLabMergeRequest', () => { commitAction: 'delete', targetPath: 'source', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -652,7 +648,7 @@ describe('createGitLabMergeRequest', () => { commitAction: 'create', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, @@ -697,7 +693,7 @@ describe('createGitLabMergeRequest', () => { commitAction: 'create', }; - mockFs({ + mockDir.setContent({ [workspacePath]: { source: { 'foo.txt': 'Hello there!' }, irrelevant: { 'bar.txt': 'Nothing to see here' }, From d5d950a916df09c44fd81d2ca5d92589c0ef0327 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:54:27 +0200 Subject: [PATCH 15/17] scaffolder-backend: refactor NunjucksWorkflowRunner tests to avoid mock-fs Signed-off-by: Patrik Oldsberg --- .../tasks/NunjucksWorkflowRunner.test.ts | 33 ++++--------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index d94a8eac69..6907f8cc2c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -14,10 +14,7 @@ * limitations under the License. */ -import mockFs from 'mock-fs'; -import * as winston from 'winston'; - -import { getVoidLogger, resolvePackagePath } from '@backstage/backend-common'; +import { getVoidLogger } from '@backstage/backend-common'; import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; @@ -36,19 +33,7 @@ import { PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { RESOURCE_TYPE_SCAFFOLDER_ACTION } from '@backstage/plugin-scaffolder-common/alpha'; - -// The Stream module is lazy loaded, so make sure it's in the module cache before mocking fs -void winston.transports.Stream; - -const realFiles = Object.fromEntries( - [ - resolvePackagePath( - '@backstage/plugin-scaffolder-backend', - 'assets', - 'nunjucks.js.txt', - ), - ].map(k => [k, mockFs.load(k)]), -); +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('DefaultWorkflowRunner', () => { const logger = getVoidLogger(); @@ -56,6 +41,8 @@ describe('DefaultWorkflowRunner', () => { let runner: NunjucksWorkflowRunner; let fakeActionHandler: jest.Mock; + const mockDir = createMockDirectory(); + const mockedPermissionApi: jest.Mocked = { authorizeConditional: jest.fn(), } as unknown as jest.Mocked; @@ -84,11 +71,7 @@ describe('DefaultWorkflowRunner', () => { }); beforeEach(() => { - winston.format.simple(); // put logform in the require.cache before mocking fs - mockFs({ - '/tmp': mockFs.directory(), - ...realFiles, - }); + mockDir.clear(); jest.resetAllMocks(); actionRegistry = new TemplateActionRegistry(); @@ -148,16 +131,12 @@ describe('DefaultWorkflowRunner', () => { runner = new NunjucksWorkflowRunner({ actionRegistry, integrations, - workingDirectory: '/tmp', + workingDirectory: mockDir.path, logger, permissions: mockedPermissionApi, }); }); - afterEach(() => { - mockFs.restore(); - }); - it('should throw an error if the action does not exist', async () => { const task = createMockTaskWithSpec({ apiVersion: 'scaffolder.backstage.io/v1beta3', From 7d077ae835c72bb485f5bc192295dee9fc841f5f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:56:23 +0200 Subject: [PATCH 16/17] scaffolder-backend: remove mock-fs dependency Signed-off-by: Patrik Oldsberg --- plugins/scaffolder-backend/package.json | 2 -- yarn.lock | 2 -- 2 files changed, 4 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index cab9abae96..4ff894cd61 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -107,13 +107,11 @@ "@types/fs-extra": "^9.0.1", "@types/git-url-parse": "^9.0.0", "@types/libsodium-wrappers": "^0.7.10", - "@types/mock-fs": "^4.13.0", "@types/nunjucks": "^3.1.4", "@types/supertest": "^2.0.8", "@types/zen-observable": "^0.8.0", "esbuild": "^0.19.0", "jest-when": "^3.1.0", - "mock-fs": "^5.2.0", "msw": "^1.0.0", "supertest": "^6.1.3", "wait-for-expect": "^3.0.2", diff --git a/yarn.lock b/yarn.lock index 70690186aa..96df4629a6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8722,7 +8722,6 @@ __metadata: "@types/git-url-parse": ^9.0.0 "@types/libsodium-wrappers": ^0.7.10 "@types/luxon": ^3.0.0 - "@types/mock-fs": ^4.13.0 "@types/nunjucks": ^3.1.4 "@types/supertest": ^2.0.8 "@types/zen-observable": ^0.8.0 @@ -8745,7 +8744,6 @@ __metadata: libsodium-wrappers: ^0.7.11 lodash: ^4.17.21 luxon: ^3.0.0 - mock-fs: ^5.2.0 morgan: ^1.10.0 msw: ^1.0.0 node-fetch: ^2.6.7 From 4515c10ca288de65ece1cb6625385087599bf389 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 6 Oct 2023 13:08:37 +0200 Subject: [PATCH 17/17] Revert ".github/workflows: disable Node 20 tests" This reverts commit 79c3febee739705daa78cfd0c359a25a48bb60da. Signed-off-by: Patrik Oldsberg --- .github/workflows/ci.yml | 6 +++--- .github/workflows/deploy_packages.yml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dbf2dc6705..12c19075a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: strategy: fail-fast: false matrix: - node-version: [18.x] + node-version: [18.x, 20.x] env: CI: true @@ -49,7 +49,7 @@ jobs: strategy: fail-fast: false matrix: - node-version: [18.x] + node-version: [18.x, 20.x] env: CI: true @@ -139,7 +139,7 @@ jobs: strategy: fail-fast: false matrix: - node-version: [18.x] + node-version: [18.x, 20.x] name: Test ${{ matrix.node-version }} services: diff --git a/.github/workflows/deploy_packages.yml b/.github/workflows/deploy_packages.yml index 91b25fb323..2329101eb0 100644 --- a/.github/workflows/deploy_packages.yml +++ b/.github/workflows/deploy_packages.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - node-version: [18.x] + node-version: [18.x, 20.x] services: postgres13: