From eb42e005cb1521a11add47c996cd7b0841eed2c7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 17:25:01 +0200 Subject: [PATCH 01/13] 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 0fca639626c22425ad3becc074a3cfff216e1a5a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 17:26:51 +0200 Subject: [PATCH 02/13] 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 b95b05baeb3c3fcc496a1cad6ce8f2de9f80a6a7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 17:27:45 +0200 Subject: [PATCH 03/13] 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 12da2feccb9b5d678a919255d997849c711ee310 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:06:53 +0200 Subject: [PATCH 04/13] 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 45f9748e436ef19bbc5121edfb5bfcd795eb3fef Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:29:15 +0200 Subject: [PATCH 05/13] 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 5fd3d627233b8281ed3256eda36aee88ca4b19c0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:32:28 +0200 Subject: [PATCH 06/13] 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 e68b9bf85062538111caf4bdda864b655922595d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:40:31 +0200 Subject: [PATCH 07/13] 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 ea14da07779c8852ed20522433ccedb80c3fc9d6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:45:36 +0200 Subject: [PATCH 08/13] 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 22f18a91e2651dc1129082d670ae213a03ffec90 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:48:12 +0200 Subject: [PATCH 09/13] 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 9b41a43089ae86f293b07d6156e5f36337fe4cd6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:51:09 +0200 Subject: [PATCH 10/13] 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 98407789cb1cdb1084f32159bf29020d0ba88611 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:52:07 +0200 Subject: [PATCH 11/13] 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 1159058cb164aefca95c12e9cd9ad7d4354ad339 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:54:27 +0200 Subject: [PATCH 12/13] 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 94e3cb2d6d901875158782bf5440ab96ab7cccdc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Oct 2023 21:56:23 +0200 Subject: [PATCH 13/13] 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 3bd1e9d44e..013950f540 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