From ee318017e71fea9f2ee6999065dba3c5e98cae00 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 Sep 2023 19:21:30 +0200 Subject: [PATCH 01/35] backend-test-utils: initial directory mock implementation Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/package.json | 1 + .../src/files/DirectoryMocker.ts | 83 +++++++++++++++++++ .../backend-test-utils/src/files/index.ts | 17 ++++ packages/backend-test-utils/src/index.ts | 1 + yarn.lock | 1 + 5 files changed, 103 insertions(+) create mode 100644 packages/backend-test-utils/src/files/DirectoryMocker.ts create mode 100644 packages/backend-test-utils/src/files/index.ts diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index 5274b49ce2..bc78e2b4e0 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -50,6 +50,7 @@ "@backstage/types": "workspace:^", "better-sqlite3": "^8.0.0", "express": "^4.17.1", + "fs-extra": "^10.0.1", "knex": "^2.0.0", "msw": "^1.0.0", "mysql2": "^2.2.5", diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts new file mode 100644 index 0000000000..6b67f8f9c0 --- /dev/null +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -0,0 +1,83 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { resolveSafeChildPath } from '@backstage/backend-common'; +import fs from 'fs-extra'; +import { tmpdir as getTmpDir } from 'os'; +import { join as joinPath } from 'path'; + +type FileNode = string | Buffer; + +type DirectoryNode = { + [name in string]: MockDirectoryNode; +}; + +type MockDirectoryNode = DirectoryNode | FileNode; + +export class DirectoryMocker { + static create() { + const root = fs.mkdtempSync( + joinPath(getTmpDir(), 'backstage-tmp-test-dir-'), + ); + + const mocker = new DirectoryMocker(root); + + process.on('beforeExit', mocker.#cleanupSync); + + afterAll(async () => { + await mocker.cleanup(); + }); + + return mocker; + } + + readonly #root: string; + + private constructor(root: string) { + this.#root = root; + } + + get dir() { + return this.#root; + } + + async setContent(root: MockDirectoryNode) { + await this.cleanup(); + + async function createFiles(node: MockDirectoryNode, path: string) { + if (typeof node === 'string' || node instanceof Buffer) { + await fs.writeFile(path, node, 'utf8'); + return; + } + + await fs.ensureDir(path); + + for (const [name, child] of Object.entries(node)) { + await createFiles(child, resolveSafeChildPath(path, name)); + } + } + + await createFiles(root, this.#root); + } + + async cleanup() { + await fs.rm(this.#root, { recursive: true, force: true }); + } + + #cleanupSync() { + fs.rmSync(this.#root, { recursive: true, force: true }); + } +} diff --git a/packages/backend-test-utils/src/files/index.ts b/packages/backend-test-utils/src/files/index.ts new file mode 100644 index 0000000000..f6ed111069 --- /dev/null +++ b/packages/backend-test-utils/src/files/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { DirectoryMocker } from './DirectoryMocker'; diff --git a/packages/backend-test-utils/src/index.ts b/packages/backend-test-utils/src/index.ts index ae937d2fc8..5609dbbc2b 100644 --- a/packages/backend-test-utils/src/index.ts +++ b/packages/backend-test-utils/src/index.ts @@ -22,5 +22,6 @@ export * from './database'; export * from './msw'; +export * from './files'; export * from './next'; export * from './util'; diff --git a/yarn.lock b/yarn.lock index 9419064a2e..83d9128534 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3649,6 +3649,7 @@ __metadata: "@types/supertest": ^2.0.8 better-sqlite3: ^8.0.0 express: ^4.17.1 + fs-extra: ^10.0.1 knex: ^2.0.0 msw: ^1.0.0 mysql2: ^2.2.5 From cee06b595291411383658612ee24f0cd059b5841 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 Sep 2023 19:31:00 +0200 Subject: [PATCH 02/35] backend-test-utils: added DirectoryMocker root option Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 6b67f8f9c0..7fe7a1d1f5 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -27,19 +27,33 @@ type DirectoryNode = { type MockDirectoryNode = DirectoryNode | FileNode; +interface DirectoryMockerOptions { + /** + * The root path to create the directory in. Defaults to a temporary directory. + * + * If an existing directory is provided, it will not be cleaned up after the test. + */ + root?: string; +} + export class DirectoryMocker { - static create() { - const root = fs.mkdtempSync( - joinPath(getTmpDir(), 'backstage-tmp-test-dir-'), - ); + static create(options?: DirectoryMockerOptions) { + const root = + options?.root ?? + fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); const mocker = new DirectoryMocker(root); - process.on('beforeExit', mocker.#cleanupSync); + const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); + if (shouldCleanup) { + process.on('beforeExit', mocker.#cleanupSync); - afterAll(async () => { - await mocker.cleanup(); - }); + try { + afterAll(mocker.cleanup); + } catch { + /* ignore */ + } + } return mocker; } @@ -73,11 +87,11 @@ export class DirectoryMocker { await createFiles(root, this.#root); } - async cleanup() { + cleanup = async () => { await fs.rm(this.#root, { recursive: true, force: true }); - } + }; - #cleanupSync() { + #cleanupSync = () => { fs.rmSync(this.#root, { recursive: true, force: true }); - } + }; } From c09cd27904f3dc077d4e7590d893841d3c7f69d1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 11:35:56 +0200 Subject: [PATCH 03/35] backend-test-utils: support key path expansion in DirectoryMocker Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 7fe7a1d1f5..7e99f30086 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -17,15 +17,16 @@ import { resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import { tmpdir as getTmpDir } from 'os'; -import { join as joinPath } from 'path'; +import { dirname, join as joinPath, resolve as resolvePath } from 'path'; -type FileNode = string | Buffer; - -type DirectoryNode = { - [name in string]: MockDirectoryNode; +type MockEntry = { + path: string; + content: Buffer; }; -type MockDirectoryNode = DirectoryNode | FileNode; +export type MockDirectory = { + [name in string]: MockDirectory | string | Buffer; +}; interface DirectoryMockerOptions { /** @@ -68,23 +69,36 @@ export class DirectoryMocker { return this.#root; } - async setContent(root: MockDirectoryNode) { + async setContent(root: MockDirectory) { + const entries = this.#transformInput(root); + await this.cleanup(); - async function createFiles(node: MockDirectoryNode, path: string) { - if (typeof node === 'string' || node instanceof Buffer) { - await fs.writeFile(path, node, 'utf8'); - return; - } + for (const { path, content } of entries) { + const fullPath = resolveSafeChildPath(this.#root, path.slice(1)); // trim leading slash + await fs.ensureDir(dirname(fullPath)); + await fs.writeFile(fullPath, content); + } + } - await fs.ensureDir(path); + #transformInput(input: MockDirectory[string]): MockEntry[] { + const entries: MockEntry[] = []; - for (const [name, child] of Object.entries(node)) { - await createFiles(child, resolveSafeChildPath(path, name)); + function traverse(node: MockDirectory[string], path: string) { + if (typeof node === 'string') { + entries.push({ path, content: Buffer.from(node, 'utf8') }); + } else if (node instanceof Buffer) { + entries.push({ path, content: node }); + } else { + for (const [name, child] of Object.entries(node)) { + traverse(child, `${path}/${name}`); + } } } - await createFiles(root, this.#root); + traverse(input, ''); + + return entries; } cleanup = async () => { From 8f6194ff18a6d3dc163621028478eaafd79aec8d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 11:55:49 +0200 Subject: [PATCH 04/35] backend-test-utils: add directoryMocker.getContent() Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 7e99f30086..66a6823a2f 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -81,6 +81,32 @@ export class DirectoryMocker { } } + async getContent(): Promise { + async function read(path: string): Promise { + if (!(await fs.pathExists(path))) { + return undefined; + } + const entries = await fs.readdir(path); + + return Object.fromEntries( + await Promise.all( + entries.map(async entry => { + const fullPath = resolvePath(path, entry); + const stat = await fs.stat(fullPath); + + if (stat.isDirectory()) { + return [entry, await read(fullPath)]; + } + const content = await fs.readFile(fullPath); + return [entry, content.toString('utf8')]; + }), + ), + ); + } + + return read(this.#root); + } + #transformInput(input: MockDirectory[string]): MockEntry[] { const entries: MockEntry[] = []; From 8a1d570218a8c91002571ae32cfa1062f58983b6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 15:06:23 +0200 Subject: [PATCH 05/35] backend-test-utils: add DirectoryMocker shouldReadAsText option Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/package.json | 1 + .../src/files/DirectoryMocker.ts | 32 +++++++++++++++++-- yarn.lock | 9 +++--- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index bc78e2b4e0..da626d3786 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -56,6 +56,7 @@ "mysql2": "^2.2.5", "pg": "^8.3.0", "testcontainers": "^8.1.2", + "textextensions": "^5.16.0", "uuid": "^8.0.0" }, "peerDependencies": { diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 66a6823a2f..7bf23aa2de 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -16,8 +16,14 @@ import { resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; +import textextensions from 'textextensions'; import { tmpdir as getTmpDir } from 'os'; -import { dirname, join as joinPath, resolve as resolvePath } from 'path'; +import { + dirname, + extname, + join as joinPath, + resolve as resolvePath, +} from 'path'; type MockEntry = { path: string; @@ -37,6 +43,15 @@ interface DirectoryMockerOptions { root?: string; } +interface DirectoryMockerGetContentOptions { + /** + * Whether or not to return files as text rather than buffers. + * + * Defaults to checking the file extension against a list of known text extensions. + */ + shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); +} + export class DirectoryMocker { static create(options?: DirectoryMockerOptions) { const root = @@ -81,7 +96,14 @@ export class DirectoryMocker { } } - async getContent(): Promise { + async getContent( + options?: DirectoryMockerGetContentOptions, + ): Promise { + const shouldReadAsText = + (typeof options?.shouldReadAsText === 'boolean' + ? () => options?.shouldReadAsText + : options?.shouldReadAsText) ?? + ((path: string) => textextensions.includes(extname(path).slice(1))); async function read(path: string): Promise { if (!(await fs.pathExists(path))) { return undefined; @@ -98,7 +120,11 @@ export class DirectoryMocker { return [entry, await read(fullPath)]; } const content = await fs.readFile(fullPath); - return [entry, content.toString('utf8')]; + + if (shouldReadAsText(fullPath, content)) { + return [entry, content.toString('utf8')]; + } + return [entry, content]; }), ), ); diff --git a/yarn.lock b/yarn.lock index 83d9128534..86a011100e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3656,6 +3656,7 @@ __metadata: pg: ^8.3.0 supertest: ^6.1.3 testcontainers: ^8.1.2 + textextensions: ^5.16.0 uuid: ^8.0.0 peerDependencies: "@types/jest": "*" @@ -40458,10 +40459,10 @@ __metadata: languageName: node linkType: hard -"textextensions@npm:^5.12.0, textextensions@npm:^5.13.0": - version: 5.14.0 - resolution: "textextensions@npm:5.14.0" - checksum: 1f610ccf2a2c1445fb7156c23b5c8defd608c74e8047df18abd4b9ee44c74d29b453ba104d14c53c91f9026670f7c923114cd12200ee5ec458cc518fd0798c74 +"textextensions@npm:^5.12.0, textextensions@npm:^5.13.0, textextensions@npm:^5.16.0": + version: 5.16.0 + resolution: "textextensions@npm:5.16.0" + checksum: d2abd5c962760046aa85d9ca542bd8bdb451370fc0a5e5f807aa80dd2f50175ec10d5ce9d28ae96968aaf6a1b1bea254cf4715f24852d0dcf29c6a60af7f793c languageName: node linkType: hard From c6e5eec85c2f3f032a8a020745f45a81d419b71b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 15:06:39 +0200 Subject: [PATCH 06/35] backend-test-utils: initial tests for DirectoryMocker Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.test.ts | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 packages/backend-test-utils/src/files/DirectoryMocker.test.ts diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts new file mode 100644 index 0000000000..cb0e52125c --- /dev/null +++ b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts @@ -0,0 +1,83 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { DirectoryMocker } from './DirectoryMocker'; + +describe('DirectoryMocker', () => { + const mocker = DirectoryMocker.create(); + + it('should populate a directory with text files', async () => { + await mocker.setContent({ + 'a.txt': 'a', + 'a/b.txt': 'b', + 'a/b/c.txt': 'c', + 'a/b/d.txt': 'd', + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + a: { + 'b.txt': 'b', + b: { + 'c.txt': 'c', + 'd.txt': 'd', + }, + }, + }); + }); + + it('should mix text and binary files', async () => { + await mocker.setContent({ + 'a.txt': 'a', + 'a/b.txt': 'b', + 'a/b/c.bin': Buffer.from([0xc]), + 'a/b/d.bin': Buffer.from([0xd]), + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + a: { + 'b.txt': 'b', + b: { + 'c.bin': Buffer.from([0xc]), + 'd.bin': Buffer.from([0xd]), + }, + }, + }); + }); + + describe('cleanup', () => { + let cleanupMocker: DirectoryMocker; + + describe('inner', () => { + cleanupMocker = DirectoryMocker.create(); + + it('should populate a directory', async () => { + await cleanupMocker.setContent({ + 'a.txt': 'a', + }); + + await expect(cleanupMocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + }); + }); + }); + + it('should clean up after itself automatically', async () => { + await expect(cleanupMocker.getContent()).resolves.toBeUndefined(); + }); + }); +}); From dbad03ac47aeacb7d621f8e4312e6b00cccf3c5e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 19:12:19 +0200 Subject: [PATCH 07/35] backend-test-utils: added directoryMocker.addContent Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.test.ts | 60 +++++++++++++++++ .../src/files/DirectoryMocker.ts | 66 ++++++++++++------- 2 files changed, 103 insertions(+), 23 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts index cb0e52125c..1a50e38b47 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts @@ -59,6 +59,66 @@ describe('DirectoryMocker', () => { }); }); + it('should be able to add content', async () => { + await mocker.setContent({ + 'a.txt': 'a', + b: {}, + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + b: {}, + }); + + await mocker.addContent({ + 'b.txt': 'b', + b: { + 'c.txt': 'c', + }, + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + 'b.txt': 'b', + b: { + 'c.txt': 'c', + }, + }); + }); + + it('should replace existing files', async () => { + await mocker.setContent({ + 'a.txt': 'a', + }); + + await mocker.addContent({ + 'a.txt': 'a2', + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a2', + }); + }); + + it('should not override directories', async () => { + await mocker.setContent({ + 'a.txt': 'a', + b: {}, + }); + + await expect( + mocker.addContent({ + 'a.txt': {}, + }), + ).rejects.toThrow('EEXIST'); + + await expect( + mocker.addContent({ + b: 'b', + }), + ).rejects.toThrow('EISDIR'); + }); + describe('cleanup', () => { let cleanupMocker: DirectoryMocker; diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 7bf23aa2de..87538183ec 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -25,10 +25,16 @@ import { resolve as resolvePath, } from 'path'; -type MockEntry = { - path: string; - content: Buffer; -}; +type MockEntry = + | { + type: 'file'; + path: string; + content: Buffer; + } + | { + type: 'dir'; + path: string; + }; export type MockDirectory = { [name in string]: MockDirectory | string | Buffer; @@ -65,7 +71,7 @@ export class DirectoryMocker { process.on('beforeExit', mocker.#cleanupSync); try { - afterAll(mocker.cleanup); + afterAll(mocker.removeContent); } catch { /* ignore */ } @@ -85,14 +91,23 @@ export class DirectoryMocker { } async setContent(root: MockDirectory) { + await this.removeContent(); + + return this.addContent(root); + } + + async addContent(root: MockDirectory) { const entries = this.#transformInput(root); - await this.cleanup(); + for (const entry of entries) { + const fullPath = resolveSafeChildPath(this.#root, entry.path.slice(1)); // trim leading slash - for (const { path, content } of entries) { - const fullPath = resolveSafeChildPath(this.#root, path.slice(1)); // trim leading slash - await fs.ensureDir(dirname(fullPath)); - await fs.writeFile(fullPath, content); + if (entry.type === 'dir') { + await fs.ensureDir(fullPath); + } else if (entry.type === 'file') { + await fs.ensureDir(dirname(fullPath)); + await fs.writeFile(fullPath, entry.content); + } } } @@ -104,27 +119,27 @@ export class DirectoryMocker { ? () => options?.shouldReadAsText : options?.shouldReadAsText) ?? ((path: string) => textextensions.includes(extname(path).slice(1))); + async function read(path: string): Promise { if (!(await fs.pathExists(path))) { return undefined; } - const entries = await fs.readdir(path); + const entries = await fs.readdir(path, { withFileTypes: true }); return Object.fromEntries( await Promise.all( entries.map(async entry => { - const fullPath = resolvePath(path, entry); - const stat = await fs.stat(fullPath); + const fullPath = resolvePath(path, entry.name); - if (stat.isDirectory()) { - return [entry, await read(fullPath)]; + if (entry.isDirectory()) { + return [entry.name, await read(fullPath)]; } const content = await fs.readFile(fullPath); if (shouldReadAsText(fullPath, content)) { - return [entry, content.toString('utf8')]; + return [entry.name, content.toString('utf8')]; } - return [entry, content]; + return [entry.name, content]; }), ), ); @@ -133,15 +148,24 @@ export class DirectoryMocker { return read(this.#root); } + removeContent = async () => { + await fs.rm(this.#root, { recursive: true, force: true }); + }; + #transformInput(input: MockDirectory[string]): MockEntry[] { const entries: MockEntry[] = []; function traverse(node: MockDirectory[string], path: string) { if (typeof node === 'string') { - entries.push({ path, content: Buffer.from(node, 'utf8') }); + entries.push({ + type: 'file', + path, + content: Buffer.from(node, 'utf8'), + }); } else if (node instanceof Buffer) { - entries.push({ path, content: node }); + entries.push({ type: 'file', path, content: node }); } else { + entries.push({ type: 'dir', path }); for (const [name, child] of Object.entries(node)) { traverse(child, `${path}/${name}`); } @@ -153,10 +177,6 @@ export class DirectoryMocker { return entries; } - cleanup = async () => { - await fs.rm(this.#root, { recursive: true, force: true }); - }; - #cleanupSync = () => { fs.rmSync(this.#root, { recursive: true, force: true }); }; From 24f1dfa58fb1a13991ec3e541c60feb85acfb5f2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:45:13 +0200 Subject: [PATCH 08/35] backend-test-utils: path option for DirectoryMocker.getContent + better child path check Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 87538183ec..593b2790c4 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { resolveSafeChildPath } from '@backstage/backend-common'; +import { isChildPath, resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; import { tmpdir as getTmpDir } from 'os'; @@ -23,6 +23,7 @@ import { extname, join as joinPath, resolve as resolvePath, + relative as relativePath, } from 'path'; type MockEntry = @@ -50,6 +51,8 @@ interface DirectoryMockerOptions { } interface DirectoryMockerGetContentOptions { + path?: string; + /** * Whether or not to return files as text rather than buffers. * @@ -100,7 +103,12 @@ export class DirectoryMocker { const entries = this.#transformInput(root); for (const entry of entries) { - const fullPath = resolveSafeChildPath(this.#root, entry.path.slice(1)); // trim leading slash + const fullPath = resolveSafeChildPath(this.#root, entry.path); + if (!isChildPath(this.#root, fullPath)) { + throw new Error( + `Provided path must resolve to a child path of the mock directory, got ${entry.path}`, + ); + } if (entry.type === 'dir') { await fs.ensureDir(fullPath); @@ -120,6 +128,16 @@ export class DirectoryMocker { : options?.shouldReadAsText) ?? ((path: string) => textextensions.includes(extname(path).slice(1))); + const root = resolvePath(this.#root, options?.path ?? ''); + if (!isChildPath(this.#root, root)) { + throw new Error( + `Provided path must resolve to a child path of the mock directory, got ${relativePath( + this.#root, + root, + )}`, + ); + } + async function read(path: string): Promise { if (!(await fs.pathExists(path))) { return undefined; @@ -145,7 +163,7 @@ export class DirectoryMocker { ); } - return read(this.#root); + return read(root); } removeContent = async () => { @@ -156,18 +174,19 @@ export class DirectoryMocker { const entries: MockEntry[] = []; function traverse(node: MockDirectory[string], path: string) { + const trimmedPath = path.startsWith('/') ? path.slice(1) : path; // trim leading slash if (typeof node === 'string') { entries.push({ type: 'file', - path, + path: trimmedPath, content: Buffer.from(node, 'utf8'), }); } else if (node instanceof Buffer) { - entries.push({ type: 'file', path, content: node }); + entries.push({ type: 'file', path: trimmedPath, content: node }); } else { - entries.push({ type: 'dir', path }); + entries.push({ type: 'dir', path: trimmedPath }); for (const [name, child] of Object.entries(node)) { - traverse(child, `${path}/${name}`); + traverse(child, `${trimmedPath}/${name}`); } } } From d96fb0642fa89991d75ef685db3d9baacd653366 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:53:23 +0200 Subject: [PATCH 09/35] backend-test-utils: add DirectoryMocker.mockOsTmpDir Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 593b2790c4..0422969762 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import os from 'os'; import { isChildPath, resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; @@ -83,6 +84,21 @@ export class DirectoryMocker { return mocker; } + static mockOsTmpDir() { + const mocker = DirectoryMocker.create(); + const origTmpdir = os.tmpdir; + os.tmpdir = () => mocker.path; + + try { + afterAll(() => { + os.tmpdir = origTmpdir; + }); + } catch { + /* ignore */ + } + return mocker; + } + readonly #root: string; private constructor(root: string) { From 4c5d3940cb86e743a5e0c6f8be49324a8e8edce4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:55:14 +0200 Subject: [PATCH 10/35] backend-test-utils: add resolve and clear for DirectoryMocker + renames Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 0422969762..473504b6c2 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -75,7 +75,7 @@ export class DirectoryMocker { process.on('beforeExit', mocker.#cleanupSync); try { - afterAll(mocker.removeContent); + afterAll(mocker.cleanup); } catch { /* ignore */ } @@ -105,12 +105,16 @@ export class DirectoryMocker { this.#root = root; } - get dir() { + get path() { return this.#root; } + resolve(...paths: string[]) { + return resolvePath(this.#root, ...paths); + } + async setContent(root: MockDirectory) { - await this.removeContent(); + await this.cleanup(); return this.addContent(root); } @@ -182,7 +186,11 @@ export class DirectoryMocker { return read(root); } - removeContent = async () => { + clear = async () => { + await this.setContent({}); + }; + + cleanup = async () => { await fs.rm(this.#root, { recursive: true, force: true }); }; From f3bd2700ccbb7db71eee60cb4c945510bab5f494 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:57:03 +0200 Subject: [PATCH 11/35] backend-test-utils: rename DirectoryMocker.getContent -> .content Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.test.ts | 14 +++++++------- .../src/files/DirectoryMocker.ts | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts index 1a50e38b47..6ebf74b2a4 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts @@ -27,7 +27,7 @@ describe('DirectoryMocker', () => { 'a/b/d.txt': 'd', }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -47,7 +47,7 @@ describe('DirectoryMocker', () => { 'a/b/d.bin': Buffer.from([0xd]), }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -65,7 +65,7 @@ describe('DirectoryMocker', () => { b: {}, }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', b: {}, }); @@ -77,7 +77,7 @@ describe('DirectoryMocker', () => { }, }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', 'b.txt': 'b', b: { @@ -95,7 +95,7 @@ describe('DirectoryMocker', () => { 'a.txt': 'a2', }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a2', }); }); @@ -130,14 +130,14 @@ describe('DirectoryMocker', () => { 'a.txt': 'a', }); - await expect(cleanupMocker.getContent()).resolves.toEqual({ + await expect(cleanupMocker.content()).resolves.toEqual({ 'a.txt': 'a', }); }); }); it('should clean up after itself automatically', async () => { - await expect(cleanupMocker.getContent()).resolves.toBeUndefined(); + await expect(cleanupMocker.content()).resolves.toBeUndefined(); }); }); }); diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 473504b6c2..bf741f6ce2 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -51,7 +51,7 @@ interface DirectoryMockerOptions { root?: string; } -interface DirectoryMockerGetContentOptions { +interface DirectoryMockerContentOptions { path?: string; /** @@ -139,8 +139,8 @@ export class DirectoryMocker { } } - async getContent( - options?: DirectoryMockerGetContentOptions, + async content( + options?: DirectoryMockerContentOptions, ): Promise { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' From a51c1f46aa60df4042881caf621e8141cd96dc77 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 11:11:48 +0200 Subject: [PATCH 12/35] backend-test-utils: rename DirectoryMocker -> MockDirectory Signed-off-by: Patrik Oldsberg --- .../MockDirectory.test.ts} | 10 ++++---- .../MockDirectory.ts} | 24 ++++++++++--------- .../src/{files => filesystem}/index.ts | 2 +- packages/backend-test-utils/src/index.ts | 2 +- 4 files changed, 20 insertions(+), 18 deletions(-) rename packages/backend-test-utils/src/{files/DirectoryMocker.test.ts => filesystem/MockDirectory.test.ts} (93%) rename packages/backend-test-utils/src/{files/DirectoryMocker.ts => filesystem/MockDirectory.ts} (90%) rename packages/backend-test-utils/src/{files => filesystem}/index.ts (91%) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts similarity index 93% rename from packages/backend-test-utils/src/files/DirectoryMocker.test.ts rename to packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 6ebf74b2a4..48b8dcd83d 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -14,10 +14,10 @@ * limitations under the License. */ -import { DirectoryMocker } from './DirectoryMocker'; +import { MockDirectory } from './MockDirectory'; -describe('DirectoryMocker', () => { - const mocker = DirectoryMocker.create(); +describe('MockDirectory', () => { + const mocker = MockDirectory.create(); it('should populate a directory with text files', async () => { await mocker.setContent({ @@ -120,10 +120,10 @@ describe('DirectoryMocker', () => { }); describe('cleanup', () => { - let cleanupMocker: DirectoryMocker; + let cleanupMocker: MockDirectory; describe('inner', () => { - cleanupMocker = DirectoryMocker.create(); + cleanupMocker = MockDirectory.create(); it('should populate a directory', async () => { await cleanupMocker.setContent({ diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts similarity index 90% rename from packages/backend-test-utils/src/files/DirectoryMocker.ts rename to packages/backend-test-utils/src/filesystem/MockDirectory.ts index bf741f6ce2..d7e7d2c2e1 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -38,8 +38,8 @@ type MockEntry = path: string; }; -export type MockDirectory = { - [name in string]: MockDirectory | string | Buffer; +export type MockDirectoryContent = { + [name in string]: MockDirectoryContent | string | Buffer; }; interface DirectoryMockerOptions { @@ -62,13 +62,13 @@ interface DirectoryMockerContentOptions { shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); } -export class DirectoryMocker { +export class MockDirectory { static create(options?: DirectoryMockerOptions) { const root = options?.root ?? fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); - const mocker = new DirectoryMocker(root); + const mocker = new MockDirectory(root); const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); if (shouldCleanup) { @@ -85,7 +85,7 @@ export class DirectoryMocker { } static mockOsTmpDir() { - const mocker = DirectoryMocker.create(); + const mocker = MockDirectory.create(); const origTmpdir = os.tmpdir; os.tmpdir = () => mocker.path; @@ -113,13 +113,13 @@ export class DirectoryMocker { return resolvePath(this.#root, ...paths); } - async setContent(root: MockDirectory) { + async setContent(root: MockDirectoryContent) { await this.cleanup(); return this.addContent(root); } - async addContent(root: MockDirectory) { + async addContent(root: MockDirectoryContent) { const entries = this.#transformInput(root); for (const entry of entries) { @@ -141,7 +141,7 @@ export class DirectoryMocker { async content( options?: DirectoryMockerContentOptions, - ): Promise { + ): Promise { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' ? () => options?.shouldReadAsText @@ -158,7 +158,9 @@ export class DirectoryMocker { ); } - async function read(path: string): Promise { + async function read( + path: string, + ): Promise { if (!(await fs.pathExists(path))) { return undefined; } @@ -194,10 +196,10 @@ export class DirectoryMocker { await fs.rm(this.#root, { recursive: true, force: true }); }; - #transformInput(input: MockDirectory[string]): MockEntry[] { + #transformInput(input: MockDirectoryContent[string]): MockEntry[] { const entries: MockEntry[] = []; - function traverse(node: MockDirectory[string], path: string) { + function traverse(node: MockDirectoryContent[string], path: string) { const trimmedPath = path.startsWith('/') ? path.slice(1) : path; // trim leading slash if (typeof node === 'string') { entries.push({ diff --git a/packages/backend-test-utils/src/files/index.ts b/packages/backend-test-utils/src/filesystem/index.ts similarity index 91% rename from packages/backend-test-utils/src/files/index.ts rename to packages/backend-test-utils/src/filesystem/index.ts index f6ed111069..f0b05641f5 100644 --- a/packages/backend-test-utils/src/files/index.ts +++ b/packages/backend-test-utils/src/filesystem/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { DirectoryMocker } from './DirectoryMocker'; +export { MockDirectory } from './MockDirectory'; diff --git a/packages/backend-test-utils/src/index.ts b/packages/backend-test-utils/src/index.ts index 5609dbbc2b..ff1f2ee460 100644 --- a/packages/backend-test-utils/src/index.ts +++ b/packages/backend-test-utils/src/index.ts @@ -22,6 +22,6 @@ export * from './database'; export * from './msw'; -export * from './files'; +export * from './filesystem'; export * from './next'; export * from './util'; From 3eab720cd210b0dc41f731c48e9e2fd2eff77263 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 11:18:05 +0200 Subject: [PATCH 13/35] backend-common: remove usage of mock-fs, use MockDirectory instead Signed-off-by: Patrik Oldsberg --- packages/backend-common/package.json | 2 - .../src/reading/AzureUrlReader.test.ts | 23 +-- .../reading/BitbucketCloudUrlReader.test.ts | 25 ++- .../reading/BitbucketServerUrlReader.test.ts | 21 +-- .../src/reading/BitbucketUrlReader.test.ts | 27 ++-- .../src/reading/GerritUrlReader.test.ts | 27 ++-- .../src/reading/GithubUrlReader.test.ts | 25 ++- .../src/reading/GitlabUrlReader.test.ts | 25 ++- .../tree/ReadableArrayResponse.test.ts | 45 +++--- .../reading/tree/TarArchiveResponse.test.ts | 151 +++++++++++------- .../reading/tree/ZipArchiveResponse.test.ts | 116 ++++++++------ .../src/reading/tree/ZipArchiveResponse.ts | 11 +- .../src/util/DockerContainerRunner.test.ts | 30 ++-- yarn.lock | 2 - 14 files changed, 267 insertions(+), 263 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index a887bdc8e7..ee43b74c16 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -125,7 +125,6 @@ "@types/fs-extra": "^9.0.3", "@types/http-errors": "^2.0.0", "@types/minimist": "^1.2.0", - "@types/mock-fs": "^4.13.0", "@types/morgan": "^1.9.0", "@types/node-forge": "^1.3.0", "@types/pg": "^8.6.6", @@ -138,7 +137,6 @@ "aws-sdk-client-mock": "^2.0.0", "better-sqlite3": "^8.0.0", "http-errors": "^2.0.0", - "mock-fs": "^5.2.0", "msw": "^1.0.0", "mysql2": "^2.2.5", "recursive-readdir": "^2.2.2", diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index 6b2f41a2bf..b6338a1c70 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -23,12 +23,13 @@ import { AzureDevOpsCredentialLike, AzureIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import * as os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { getVoidLogger } from '../logging'; @@ -43,6 +44,8 @@ type AzureIntegrationConfigLike = Partial< const logger = getVoidLogger(); +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -69,18 +72,8 @@ const urlReaderFactory = (azureIntegration: AzureIntegrationConfigLike) => { ); }; -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('AzureUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); const worker = setupServer(); setupRequestMockHandlers(worker); @@ -270,7 +263,7 @@ describe('AzureUrlReader', () => { 'https://dev.azure.com/organization/project/_git/repository', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts index c00aeba124..18a186f793 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts @@ -19,18 +19,21 @@ import { BitbucketCloudIntegration, readBitbucketCloudIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { BitbucketCloudUrlReader } from './BitbucketCloudUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; import getRawBody from 'raw-body'; +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -49,18 +52,8 @@ const reader = new BitbucketCloudUrlReader( { treeResponseFactory }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('BitbucketCloudUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); const worker = setupServer(); setupRequestMockHandlers(worker); @@ -316,7 +309,7 @@ describe('BitbucketCloudUrlReader', () => { 'https://bitbucket.org/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -346,7 +339,7 @@ describe('BitbucketCloudUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts index 9f231e2ba7..82b467f70f 100644 --- a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts @@ -19,17 +19,20 @@ import { BitbucketServerIntegration, readBitbucketServerIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { BitbucketServerUrlReader } from './BitbucketServerUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; +MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -46,19 +49,7 @@ const reader = new BitbucketServerUrlReader( { treeResponseFactory }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('BitbucketServerUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); - const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 15e007d516..986619d161 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -19,12 +19,13 @@ import { BitbucketIntegration, readBitbucketIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { BitbucketUrlReader } from './BitbucketUrlReader'; @@ -68,6 +69,10 @@ describe('BitbucketUrlReader.factory', () => { }); describe('BitbucketUrlReader', () => { + const mockDir = MockDirectory.mockOsTmpDir(); + + beforeEach(() => mockDir.clear()); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -98,18 +103,6 @@ describe('BitbucketUrlReader', () => { { treeResponseFactory }, ); - const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); - const worker = setupServer(); setupRequestMockHandlers(worker); @@ -391,7 +384,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -436,7 +429,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index 19c67ffd56..d3c336ee93 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -14,7 +14,10 @@ * limitations under the License. */ -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; import { NotModifiedError, NotFoundError } from '@backstage/errors'; import { @@ -24,7 +27,6 @@ import { import { JsonObject } from '@backstage/types'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import mockFs from 'mock-fs'; import fs from 'fs-extra'; import path from 'path'; import { getVoidLogger } from '../logging'; @@ -33,6 +35,8 @@ import { DefaultReadTreeResponseFactory } from './tree'; import { GerritUrlReader } from './GerritUrlReader'; import getRawBody from 'raw-body'; +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -82,13 +86,12 @@ const createReader = (config: JsonObject): UrlReaderPredicateTuple[] => { }); }; -// TODO(Rugvip): These tests seem to be a direct or indirect cause of the TaskWorker test flakiness -// We're not sure why at this point, but while investigating these tests are disabled -// eslint-disable-next-line jest/no-disabled-tests -describe.skip('GerritUrlReader', () => { +describe('GerritUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); + beforeEach(() => mockDir.clear()); + afterAll(() => { jest.clearAllMocks(); }); @@ -249,14 +252,13 @@ describe.skip('GerritUrlReader', () => { path.resolve(__dirname, '__fixtures__/gerrit/gerrit-master-docs.tar.gz'), ); - beforeEach(() => { - mockFs({ - '/tmp/': mockFs.directory(), - '/tmp/gerrit-clone-123abc/repo/mkdocs.yml': mkdocsContent, - '/tmp/gerrit-clone-123abc/repo/docs/first.md': mdContent, + beforeEach(async () => { + await mockDir.setContent({ + 'repo/mkdocs.yml': mkdocsContent, + 'repo/docs/first.md': mdContent, }); const spy = jest.spyOn(fs, 'mkdtemp'); - spy.mockImplementation(() => '/tmp/gerrit-clone-123abc'); + spy.mockImplementation(() => mockDir.path); worker.use( rest.get( @@ -289,7 +291,6 @@ describe.skip('GerritUrlReader', () => { }); afterEach(() => { - mockFs.restore(); jest.clearAllMocks(); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 499f97d6ec..326d3632a4 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -20,12 +20,13 @@ import { GithubIntegration, readGithubIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotFoundError, NotModifiedError } from '@backstage/errors'; import { @@ -37,6 +38,8 @@ import { } from './GithubUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -69,21 +72,11 @@ const gheProcessor = new GithubUrlReader( { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('GithubUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); beforeEach(() => { jest.clearAllMocks(); @@ -420,7 +413,7 @@ describe('GithubUrlReader', () => { 'https://github.com/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -501,7 +494,7 @@ describe('GithubUrlReader', () => { 'https://github.com/backstage/mock/tree/main/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 5af4faf7e5..6500f207f0 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -15,12 +15,13 @@ */ import { ConfigReader } from '@backstage/config'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { getVoidLogger } from '../logging'; import { GitlabUrlReader } from './GitlabUrlReader'; @@ -33,6 +34,8 @@ import { const logger = getVoidLogger(); +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -65,18 +68,8 @@ const hostedGitlabProcessor = new GitlabUrlReader( { treeResponseFactory }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('GitlabUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); const worker = setupServer(); setupRequestMockHandlers(worker); @@ -400,7 +393,7 @@ describe('GitlabUrlReader', () => { 'https://gitlab.com/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -457,7 +450,7 @@ describe('GitlabUrlReader', () => { 'https://gitlab.com/backstage/mock/tree/main/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index a5728a6169..9f0fd5377e 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -15,33 +15,35 @@ */ import fs from 'fs-extra'; -import mockFs from 'mock-fs'; -import path, { resolve as resolvePath } from 'path'; +import path from 'path'; import { FromReadableArrayOptions } from '../types'; import { ReadableArrayResponse } from './ReadableArrayResponse'; +import { MockDirectory } from '@backstage/backend-test-utils'; -const path1 = '/file1.yaml'; +const name1 = 'file1.yaml'; const file1 = fs.readFileSync( path.resolve(__filename, '../../__fixtures__/awsS3/awsS3-mock-object.yaml'), ); -const path2 = '/file2.yaml'; +const name2 = 'file2.yaml'; const file2 = fs.readFileSync( path.resolve(__filename, '../../__fixtures__/awsS3/awsS3-mock-object2.yaml'), ); describe('ReadableArrayResponse', () => { - beforeEach(() => { - mockFs({ - [path1]: file1, - [path2]: file2, - '/tmp': mockFs.directory(), + const sourceDir = MockDirectory.create(); + const targetDir = MockDirectory.create(); + + beforeEach(async () => { + await sourceDir.setContent({ + [name1]: file1, + [name2]: file2, }); + await targetDir.clear(); }); - afterEach(() => { - mockFs.restore(); - }); + const path1 = sourceDir.resolve(name1); + const path2 = sourceDir.resolve(name2); it('should read files', async () => { const arr: FromReadableArrayOptions = [ @@ -49,7 +51,7 @@ describe('ReadableArrayResponse', () => { { data: fs.createReadStream(path2), path: path2 }, ]; - const res = new ReadableArrayResponse(arr, '/tmp', 'etag'); + const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -57,10 +59,7 @@ describe('ReadableArrayResponse', () => { { path: path2, content: expect.any(Function) }, ]); const contents = await Promise.all(files.map(f => f.content())); - expect(contents.map(c => c.toString('utf8').trim())).toEqual([ - 'site_name: Test', - 'site_name: Test2', - ]); + expect(contents).toEqual([file1, file2]); }); it('should extract entire archive into directory', async () => { @@ -69,14 +68,12 @@ describe('ReadableArrayResponse', () => { { data: fs.createReadStream(path2), path: path2 }, ]; - const res = new ReadableArrayResponse(arr, '/tmp', 'etag'); + const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); const dir = await res.dir(); - await expect( - fs.readFile(resolvePath(dir, 'file1.yaml'), 'utf8'), - ).resolves.toMatch(/site_name: Test/); - await expect( - fs.readFile(resolvePath(dir, 'file2.yaml'), 'utf8'), - ).resolves.toMatch(/site_name: Test2/); + await expect(targetDir.content({ path: dir })).resolves.toEqual({ + [name1]: file1.toString('utf8'), + [name2]: file2.toString('utf8'), + }); }); }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index 1765a41156..ce126eb345 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -15,30 +15,31 @@ */ import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { resolve as resolvePath } from 'path'; import { TarArchiveResponse } from './TarArchiveResponse'; +import { MockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'), ); describe('TarArchiveResponse', () => { - beforeEach(() => { - mockFs({ - '/test-archive.tar.gz': archiveData, - '/tmp': mockFs.directory(), - }); - }); + const sourceDir = MockDirectory.create(); + const targetDir = MockDirectory.create(); - afterEach(() => { - mockFs.restore(); + beforeAll(async () => { + await sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); + }); + beforeEach(async () => { + await targetDir.clear(); }); it('should read files', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -61,10 +62,16 @@ describe('TarArchiveResponse', () => { }); it('should read files with filter', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -80,16 +87,18 @@ describe('TarArchiveResponse', () => { }); it('should read as archive and files', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new TarArchiveResponse(buffer, '', '/tmp', 'etag'); + const res2 = new TarArchiveResponse(buffer, '', targetDir.path, 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -112,9 +121,11 @@ describe('TarArchiveResponse', () => { }); it('should extract entire archive into directory', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag'); const dir = await res.dir(); await expect( fs.readFile(resolvePath(dir, 'mkdocs.yml'), 'utf8'), @@ -125,67 +136,91 @@ describe('TarArchiveResponse', () => { }); it('should extract archive into directory with a subpath', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, 'docs', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, 'docs', targetDir.path, 'etag'); const dir = await res.dir(); - expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); - await expect( - fs.readFile(resolvePath(dir, 'index.md'), 'utf8'), - ).resolves.toBe('# Test\n'); + await expect(targetDir.content({ path: dir })).resolves.toEqual({ + 'index.md': '# Test\n', + }); }); it('should extract archive into directory with a subpath and filter', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); - - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), ); - const dir = await res.dir({ targetDir: '/tmp' }); - expect(dir).toBe('/tmp'); - await expect(fs.pathExists(resolvePath(dir, 'mkdocs.yml'))).resolves.toBe( - true, + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); - await expect( - fs.pathExists(resolvePath(dir, 'docs/index.md')), - ).resolves.toBe(false); + + await targetDir.addContent({ sub: {} }); + const dir = await res.dir({ targetDir: targetDir.resolve('sub') }); + + expect(dir).toBe(targetDir.resolve('sub')); + await expect(targetDir.content()).resolves.toEqual({ + sub: { + 'mkdocs.yml': 'site_name: Test\n', + }, + }); }); - it('should leave temporary directories in place in the case of an error', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + it('should clean up temporary directories in place in the case of an error', async () => { + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', () => { - throw new Error('NOPE'); - }); + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + () => { + throw new Error('NOPE'); + }, + ); - const tmpDir = await fs.mkdtemp('/tmp/test'); - // selects the wrong overload by default - const mkdtemp = jest.spyOn(fs, 'mkdtemp') as unknown as jest.SpyInstance< - Promise, - [] - >; - mkdtemp.mockResolvedValue(tmpDir); + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(true); + const mkdtemp = jest + .spyOn(fs, 'mkdtemp') + .mockImplementation(async () => sub); + + await expect(fs.pathExists(sub)).resolves.toBe(true); await expect(res.dir()).rejects.toThrow('NOPE'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(false); + await expect(fs.pathExists(sub)).resolves.toBe(false); mkdtemp.mockRestore(); }); it('should leave directory in place if provided in the case of an error', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', () => { - throw new Error('NOPE'); - }); + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + () => { + throw new Error('NOPE'); + }, + ); - const tmpDir = await fs.mkdtemp('/tmp/test'); + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(true); - await expect(res.dir({ targetDir: tmpDir })).rejects.toThrow('NOPE'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(true); + await expect(fs.pathExists(sub)).resolves.toBe(true); + await expect(res.dir({ targetDir: sub })).rejects.toThrow('NOPE'); + await expect(fs.pathExists(sub)).resolves.toBe(true); }); }); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 0b010565dd..20cf33241b 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -15,11 +15,11 @@ */ import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { Readable } from 'stream'; import { create as createArchive } from 'archiver'; import { resolve as resolvePath } from 'path'; import { ZipArchiveResponse } from './ZipArchiveResponse'; +import { MockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.zip'), @@ -35,24 +35,25 @@ const archiveWithMaliciousEntry = fs.readFileSync( ); describe('ZipArchiveResponse', () => { - beforeEach(() => { - mockFs({ - '/test-archive.zip': archiveData, - '/test-archive-with-extra-root-dir.zip': archiveDataWithExtraDir, - '/test-archive-corrupted.zip': archiveDataCorrupted, - '/test-archive-malicious.zip': archiveWithMaliciousEntry, - '/tmp': mockFs.directory(), + const sourceDir = MockDirectory.create(); + const targetDir = MockDirectory.create(); + + beforeAll(async () => { + await sourceDir.setContent({ + 'test-archive.zip': archiveData, + 'test-archive-with-extra-root-dir.zip': archiveDataWithExtraDir, + 'test-archive-corrupted.zip': archiveDataCorrupted, + 'test-archive-malicious.zip': archiveWithMaliciousEntry, }); }); - - afterEach(() => { - mockFs.restore(); + beforeEach(async () => { + await targetDir.clear(); }); it('should read files', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -76,10 +77,14 @@ describe('ZipArchiveResponse', () => { }); it('should read files with filter', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -95,16 +100,16 @@ describe('ZipArchiveResponse', () => { }); it('should read as archive and files', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new ZipArchiveResponse(buffer, '', '/tmp', 'etag'); + const res2 = new ZipArchiveResponse(buffer, '', targetDir.path, 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -127,9 +132,9 @@ describe('ZipArchiveResponse', () => { }); it('should extract entire archive into directory', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const dir = await res.dir(); await expect( @@ -141,38 +146,45 @@ describe('ZipArchiveResponse', () => { }); it('should extract archive into directory with a subpath', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + + const res = new ZipArchiveResponse(stream, 'docs/', targetDir.path, 'etag'); - const res = new ZipArchiveResponse(stream, 'docs/', '/tmp', 'etag'); const dir = await res.dir(); - expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); - await expect( - fs.readFile(resolvePath(dir, 'index.md'), 'utf8'), - ).resolves.toBe('# Test\n'); + await expect(targetDir.content({ path: dir })).resolves.toEqual({ + 'index.md': '# Test\n', + }); }); it('should extract archive into directory with a subpath and filter', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); - const dir = await res.dir({ targetDir: '/tmp' }); - expect(dir).toBe('/tmp'); - await expect(fs.pathExists(resolvePath(dir, 'mkdocs.yml'))).resolves.toBe( - true, - ); - await expect( - fs.pathExists(resolvePath(dir, 'docs/index.md')), - ).resolves.toBe(false); + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); + const dir = await res.dir({ targetDir: sub }); + + expect(dir).toBe(sub); + + await expect(targetDir.content()).resolves.toEqual({ + sub: { + 'mkdocs.yml': 'site_name: Test\n', + }, + }); }); it('should extract a large archive', async () => { const fileCount = 10; const fileSize = 1000 * 1000; const filePath = await new Promise((resolve, reject) => { - const outFile = '/large-archive.zip'; + const outFile = targetDir.resolve('large-archive.zip'); const archive = createArchive('zip'); archive.on('error', reject); @@ -195,12 +207,14 @@ describe('ZipArchiveResponse', () => { const stream = fs.createReadStream(filePath); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); + + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); const dir = await res.dir({ - targetDir: '/out', + targetDir: sub, }); - expect(dir).toBe('/out'); const files = await fs.readdir(dir); expect(files).toHaveLength(fileCount); @@ -211,9 +225,11 @@ describe('ZipArchiveResponse', () => { }); it('should throw on invalid archive', async () => { - const stream = fs.createReadStream('/test-archive-corrupted.zip'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive-corrupted.zip'), + ); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const filesPromise = res.files(); await expect(filesPromise).rejects.toThrow( @@ -222,18 +238,22 @@ describe('ZipArchiveResponse', () => { }); it('should throw on entries with a path outside the destination dir', async () => { - const stream = fs.createReadStream('/test-archive-malicious.zip'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive-malicious.zip'), + ); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); await expect(res.files()).rejects.toThrow( 'invalid relative path: ../side.txt', ); }); it('should throw on entries that attempt to write outside destination dir', async () => { - const stream = fs.createReadStream('/test-archive-malicious.zip'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive-malicious.zip'), + ); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); await expect(res.dir()).rejects.toThrow( 'invalid relative path: ../side.txt', ); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 1e55150310..cd57788743 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -94,7 +94,10 @@ export class ZipArchiveResponse implements ReadTreeResponse { return new Promise((resolve, reject) => { writeStream.on('error', reject); writeStream.on('finish', () => - resolve({ fileName: tmpFile, cleanup: () => fs.remove(tmpFile) }), + resolve({ + fileName: tmpFile, + cleanup: () => fs.rm(tmpDir, { recursive: true }), + }), ); stream.pipe(writeStream); }); @@ -152,7 +155,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { }); }); - temporary.cleanup(); + await temporary.cleanup(); return files; } @@ -175,7 +178,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { archive.finalize(); - temporary.cleanup(); + await temporary.cleanup(); return archive; } @@ -204,7 +207,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { }); }); - temporary.cleanup(); + await temporary.cleanup(); return dir; } diff --git a/packages/backend-common/src/util/DockerContainerRunner.test.ts b/packages/backend-common/src/util/DockerContainerRunner.test.ts index 3208679d1f..6b4150540a 100644 --- a/packages/backend-common/src/util/DockerContainerRunner.test.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.test.ts @@ -14,27 +14,24 @@ * limitations under the License. */ +import fs from 'fs-extra'; import Docker from 'dockerode'; -import mockFs from 'mock-fs'; -import os from 'os'; -import path from 'path'; import Stream, { PassThrough } from 'stream'; import { ContainerRunner } from './ContainerRunner'; import { DockerContainerRunner, UserOptions } from './DockerContainerRunner'; +import { MockDirectory } from '@backstage/backend-test-utils'; const mockDocker = new Docker() as jest.Mocked; -const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; describe('DockerContainerRunner', () => { let containerTaskApi: ContainerRunner; - beforeEach(() => { - mockFs({ - [rootDir]: { - input: mockFs.directory(), - output: mockFs.directory(), - }, - }); + const inputDir = MockDirectory.create(); + const outputDir = MockDirectory.create(); + + beforeEach(async () => { + await inputDir.clear(); + await outputDir.clear(); jest.spyOn(mockDocker, 'pull').mockImplementation((async ( _image: string, @@ -59,16 +56,15 @@ describe('DockerContainerRunner', () => { afterEach(() => { jest.clearAllMocks(); - mockFs.restore(); }); const imageName = 'dockerOrg/image'; const args = ['bash', '-c', 'echo test']; const mountDirs = { - [path.join(rootDir, 'input')]: '/input', - [path.join(rootDir, 'output')]: '/output', + [inputDir.path]: '/input', + [outputDir.path]: '/output', }; - const workingDir = path.join(rootDir, 'input'); + const workingDir = inputDir.path; const envVars = { HOME: '/tmp', LOG_LEVEL: 'debug' }; const envVarsArray = ['HOME=/tmp', 'LOG_LEVEL=debug']; @@ -117,8 +113,8 @@ describe('DockerContainerRunner', () => { HostConfig: { AutoRemove: true, Binds: expect.arrayContaining([ - `${path.join(rootDir, 'input')}:/input`, - `${path.join(rootDir, 'output')}:/output`, + `${await fs.realpath(inputDir.path)}:/input`, + `${await fs.realpath(outputDir.path)}:/output`, ]), }, Volumes: { diff --git a/yarn.lock b/yarn.lock index 86a011100e..dc8e58da96 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3464,7 +3464,6 @@ __metadata: "@types/http-errors": ^2.0.0 "@types/luxon": ^3.0.0 "@types/minimist": ^1.2.0 - "@types/mock-fs": ^4.13.0 "@types/morgan": ^1.9.0 "@types/node-forge": ^1.3.0 "@types/pg": ^8.6.6 @@ -3497,7 +3496,6 @@ __metadata: luxon: ^3.0.0 minimatch: ^5.0.0 minimist: ^1.2.5 - mock-fs: ^5.2.0 morgan: ^1.10.0 msw: ^1.0.0 mysql2: ^2.2.5 From 5b20107206650412798013c976ba5b533267812f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:12:36 +0200 Subject: [PATCH 14/35] backend-test-utils: document MockDirectory + more naming tweaks Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 74 +++++-- .../src/filesystem/MockDirectory.ts | 191 +++++++++++++++--- 2 files changed, 219 insertions(+), 46 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 48b8dcd83d..22563b7a59 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -17,17 +17,17 @@ import { MockDirectory } from './MockDirectory'; describe('MockDirectory', () => { - const mocker = MockDirectory.create(); + const mockDir = MockDirectory.create(); it('should populate a directory with text files', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.txt': 'c', 'a/b/d.txt': 'd', }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -40,14 +40,14 @@ describe('MockDirectory', () => { }); it('should mix text and binary files', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.bin': Buffer.from([0xc]), 'a/b/d.bin': Buffer.from([0xd]), }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -60,24 +60,24 @@ describe('MockDirectory', () => { }); it('should be able to add content', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', b: {}, }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', b: {}, }); - await mocker.addContent({ + await mockDir.addContent({ 'b.txt': 'b', b: { 'c.txt': 'c', }, }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', 'b.txt': 'b', b: { @@ -87,57 +87,91 @@ describe('MockDirectory', () => { }); it('should replace existing files', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', }); - await mocker.addContent({ + await mockDir.addContent({ 'a.txt': 'a2', }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a2', }); }); it('should not override directories', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', b: {}, }); await expect( - mocker.addContent({ + mockDir.addContent({ 'a.txt': {}, }), ).rejects.toThrow('EEXIST'); await expect( - mocker.addContent({ + mockDir.addContent({ b: 'b', }), ).rejects.toThrow('EISDIR'); }); + it('examples should work', async () => { + await mockDir.setContent({ + 'test.txt': 'content', + 'sub-dir': { + 'file.txt': 'content', + 'nested-dir/file.txt': 'content', + }, + 'empty-dir': {}, + 'binary-file': Buffer.from([0, 1, 2]), + }); + + await mockDir.addContent({ + 'test.txt': 'content', + 'sub-dir': { + 'file.txt': 'content', + 'nested-dir/file.txt': 'content', + }, + 'empty-dir': {}, + 'binary-file': Buffer.from([0, 1, 2]), + }); + + await expect(mockDir.content()).resolves.toEqual({ + 'test.txt': 'content', + 'sub-dir': { + 'file.txt': 'content', + 'nested-dir': { + 'file.txt': 'content', + }, + }, + 'empty-dir': {}, + 'binary-file': Buffer.from([0, 1, 2]), + }); + }); + describe('cleanup', () => { - let cleanupMocker: MockDirectory; + let cleanupMockDir: MockDirectory; describe('inner', () => { - cleanupMocker = MockDirectory.create(); + cleanupMockDir = MockDirectory.create(); it('should populate a directory', async () => { - await cleanupMocker.setContent({ + await cleanupMockDir.setContent({ 'a.txt': 'a', }); - await expect(cleanupMocker.content()).resolves.toEqual({ + await expect(cleanupMockDir.content()).resolves.toEqual({ 'a.txt': 'a', }); }); }); it('should clean up after itself automatically', async () => { - await expect(cleanupMocker.content()).resolves.toBeUndefined(); + await expect(cleanupMockDir.content()).resolves.toBeUndefined(); }); }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index d7e7d2c2e1..2ef61b3d0c 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -27,6 +27,69 @@ import { relative as relativePath, } from 'path'; +/** + * The content of a mock directory represented by a nested object structure. + * + * @remarks + * + * When used as input, the keys may contain forward slashes to indicate nested directories. + * Then returned as output, each directory will always be represented as a separate object. + * + * @example + * ```ts + * { + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir/file.txt': 'content', + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * } + * ``` + * + * @public + */ +export type MockDirectoryContent = { + [name in string]: MockDirectoryContent | string | Buffer; +}; + +/** + * Options for {@link MockDirectory.create}. + * + * @public + */ +export interface MockDirectoryCreateOptions { + /** + * The root path to create the directory in. Defaults to a temporary directory. + * + * If an existing directory is provided, it will not be cleaned up after the test. + */ + root?: string; +} + +/** + * Options for {@link MockDirectory.content}. + * + * @public + */ +export interface MockDirectoryContentOptions { + /** + * The path to read content from. Defaults to the root of the mock directory. + * + * An absolute path can also be provided, as long as it is a child path of the mock directory. + */ + path?: string; + + /** + * Whether or not to return files as text rather than buffers. + * + * Defaults to checking the file extension against a list of known text extensions. + */ + shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); +} + +/** @internal */ type MockEntry = | { type: 'file'; @@ -38,32 +101,35 @@ type MockEntry = path: string; }; -export type MockDirectoryContent = { - [name in string]: MockDirectoryContent | string | Buffer; -}; - -interface DirectoryMockerOptions { - /** - * The root path to create the directory in. Defaults to a temporary directory. - * - * If an existing directory is provided, it will not be cleaned up after the test. - */ - root?: string; -} - -interface DirectoryMockerContentOptions { - path?: string; - - /** - * Whether or not to return files as text rather than buffers. - * - * Defaults to checking the file extension against a list of known text extensions. - */ - shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); -} - +/** + * A utility for creating a mock directory that is automatically cleaned up. + * + * @public + */ export class MockDirectory { - static create(options?: DirectoryMockerOptions) { + /** + * Creates a new temporary mock directory that will be removed after the tests have completed. + * + * @remarks + * + * This method is intended to be called outside of any test, either at top-level or + * within a `describe` block. It will call `afterAll` to make sure that the mock directory + * is removed after the tests have run. + * + * @example + * ```ts + * describe('MySubject', () => { + * const mockDir = MockDirectory.create(); + * + * beforeEach(() => mockDir.clear()); + * + * it('should work', async () => { + * // ... use mockDir + * }) + * }) + * ``` + */ + static create(options?: MockDirectoryCreateOptions) { const root = options?.root ?? fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); @@ -84,6 +150,12 @@ export class MockDirectory { return mocker; } + /** + * Like {@link MockDirectory.create}, but also mocks `os.tmpdir()` to return the + * mock directory path until the end of the test suite. + * + * @returns + */ static mockOsTmpDir() { const mocker = MockDirectory.create(); const origTmpdir = os.tmpdir; @@ -105,20 +177,58 @@ export class MockDirectory { this.#root = root; } + /** + * The path to the root of the mock directory + */ get path() { return this.#root; } + /** + * Resolves a path relative to the root of the mock directory. + */ resolve(...paths: string[]) { return resolvePath(this.#root, ...paths); } + /** + * Sets the content of the mock directory. This will remove any existing content. + * + * @example + * ```ts + * await mockDir.setContent({ + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir/file.txt': 'content', + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * }); + * ``` + */ async setContent(root: MockDirectoryContent) { await this.cleanup(); return this.addContent(root); } + /** + * Adds content of the mock directory. This will overwrite existing files. + * + * @example + * ```ts + * await mockDir.addContent({ + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir/file.txt': 'content', + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * }); + * ``` + */ async addContent(root: MockDirectoryContent) { const entries = this.#transformInput(root); @@ -139,8 +249,31 @@ export class MockDirectory { } } + /** + * Reads the content of the mock directory. + * + * @remarks + * + * Text files will be returned as strings, while binary files will be returned as buffers. + * By default the file extension is used to determine whether a file should be read as text. + * + * @example + * ```ts + * await expect(mockDir.content()).resolves.toEqual({ + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir': { + * 'file.txt': 'content', + * }, + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * }); + * ``` + */ async content( - options?: DirectoryMockerContentOptions, + options?: MockDirectoryContentOptions, ): Promise { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' @@ -188,10 +321,16 @@ export class MockDirectory { return read(root); } + /** + * Clears the content of the mock directory, ensuring that the directory itself exists. + */ clear = async () => { await this.setContent({}); }; + /** + * Removes the mock directory and all its contents. + */ cleanup = async () => { await fs.rm(this.#root, { recursive: true, force: true }); }; From 4606abef326ec47ceed07cd2e284e3f8712288f7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:17:37 +0200 Subject: [PATCH 15/35] backend-test-utils: MockDirectory API report updates Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 32 +++++++++++++++++++ .../src/filesystem/MockDirectory.ts | 16 +++++----- .../src/filesystem/index.ts | 7 +++- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index 4a27ac21a9..03df9de264 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -4,6 +4,7 @@ ```ts /// +/// import { Backend } from '@backstage/backend-app-api'; import { BackendFeature } from '@backstage/backend-plugin-api'; @@ -33,6 +34,37 @@ import { UrlReaderService } from '@backstage/backend-plugin-api'; // @public (undocumented) export function isDockerDisabledForTests(): boolean; +// @public +export class MockDirectory { + addContent(root: MockDirectoryContent): Promise; + cleanup: () => Promise; + clear: () => Promise; + content( + options?: MockDirectoryContentOptions, + ): Promise; + static create(options?: MockDirectoryCreateOptions): MockDirectory; + static mockOsTmpDir(): MockDirectory; + get path(): string; + resolve(...paths: string[]): string; + setContent(root: MockDirectoryContent): Promise; +} + +// @public +export type MockDirectoryContent = { + [name in string]: MockDirectoryContent | string | Buffer; +}; + +// @public +export interface MockDirectoryContentOptions { + path?: string; + shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); +} + +// @public +export interface MockDirectoryCreateOptions { + root?: string; +} + // @public (undocumented) export namespace mockServices { // (undocumented) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 2ef61b3d0c..81a65783fb 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -129,7 +129,7 @@ export class MockDirectory { * }) * ``` */ - static create(options?: MockDirectoryCreateOptions) { + static create(options?: MockDirectoryCreateOptions): MockDirectory { const root = options?.root ?? fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); @@ -156,7 +156,7 @@ export class MockDirectory { * * @returns */ - static mockOsTmpDir() { + static mockOsTmpDir(): MockDirectory { const mocker = MockDirectory.create(); const origTmpdir = os.tmpdir; os.tmpdir = () => mocker.path; @@ -180,14 +180,14 @@ export class MockDirectory { /** * The path to the root of the mock directory */ - get path() { + get path(): string { return this.#root; } /** * Resolves a path relative to the root of the mock directory. */ - resolve(...paths: string[]) { + resolve(...paths: string[]): string { return resolvePath(this.#root, ...paths); } @@ -207,7 +207,7 @@ export class MockDirectory { * }); * ``` */ - async setContent(root: MockDirectoryContent) { + async setContent(root: MockDirectoryContent): Promise { await this.cleanup(); return this.addContent(root); @@ -229,7 +229,7 @@ export class MockDirectory { * }); * ``` */ - async addContent(root: MockDirectoryContent) { + async addContent(root: MockDirectoryContent): Promise { const entries = this.#transformInput(root); for (const entry of entries) { @@ -324,14 +324,14 @@ export class MockDirectory { /** * Clears the content of the mock directory, ensuring that the directory itself exists. */ - clear = async () => { + clear = async (): Promise => { await this.setContent({}); }; /** * Removes the mock directory and all its contents. */ - cleanup = async () => { + cleanup = async (): Promise => { await fs.rm(this.#root, { recursive: true, force: true }); }; diff --git a/packages/backend-test-utils/src/filesystem/index.ts b/packages/backend-test-utils/src/filesystem/index.ts index f0b05641f5..d1077db980 100644 --- a/packages/backend-test-utils/src/filesystem/index.ts +++ b/packages/backend-test-utils/src/filesystem/index.ts @@ -14,4 +14,9 @@ * limitations under the License. */ -export { MockDirectory } from './MockDirectory'; +export { + MockDirectory, + type MockDirectoryContent, + type MockDirectoryContentOptions, + type MockDirectoryCreateOptions, +} from './MockDirectory'; From a250ad775f0bd48eaf1ce09b959664f1562cee09 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:55:02 +0200 Subject: [PATCH 16/35] changesets: add changeset for MockDirectory Signed-off-by: Patrik Oldsberg --- .changeset/flat-pandas-tan.md | 5 +++++ .changeset/short-terms-check.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/flat-pandas-tan.md create mode 100644 .changeset/short-terms-check.md diff --git a/.changeset/flat-pandas-tan.md b/.changeset/flat-pandas-tan.md new file mode 100644 index 0000000000..8461c9beb8 --- /dev/null +++ b/.changeset/flat-pandas-tan.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': patch +--- + +Added `MockDirectory` to help out with file system mocking in tests. diff --git a/.changeset/short-terms-check.md b/.changeset/short-terms-check.md new file mode 100644 index 0000000000..69d85fcb9c --- /dev/null +++ b/.changeset/short-terms-check.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Removed `mock-fs` dev dependency. From 3f3714ab4170913c51a5c6b77bfb04d503b3cbad Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:55:27 +0200 Subject: [PATCH 17/35] backend-test-utils: rename MockDirectory.cleanup -> .remove Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 2 +- .../backend-test-utils/src/filesystem/MockDirectory.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index 03df9de264..0262ef339d 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -37,7 +37,6 @@ export function isDockerDisabledForTests(): boolean; // @public export class MockDirectory { addContent(root: MockDirectoryContent): Promise; - cleanup: () => Promise; clear: () => Promise; content( options?: MockDirectoryContentOptions, @@ -45,6 +44,7 @@ export class MockDirectory { static create(options?: MockDirectoryCreateOptions): MockDirectory; static mockOsTmpDir(): MockDirectory; get path(): string; + remove: () => Promise; resolve(...paths: string[]): string; setContent(root: MockDirectoryContent): Promise; } diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 81a65783fb..cd85a4c347 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -138,10 +138,10 @@ export class MockDirectory { const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); if (shouldCleanup) { - process.on('beforeExit', mocker.#cleanupSync); + process.on('beforeExit', mocker.#removeSync); try { - afterAll(mocker.cleanup); + afterAll(mocker.remove); } catch { /* ignore */ } @@ -208,7 +208,7 @@ export class MockDirectory { * ``` */ async setContent(root: MockDirectoryContent): Promise { - await this.cleanup(); + await this.remove(); return this.addContent(root); } @@ -331,7 +331,7 @@ export class MockDirectory { /** * Removes the mock directory and all its contents. */ - cleanup = async (): Promise => { + remove = async (): Promise => { await fs.rm(this.#root, { recursive: true, force: true }); }; @@ -361,7 +361,7 @@ export class MockDirectory { return entries; } - #cleanupSync = () => { + #removeSync = () => { fs.rmSync(this.#root, { recursive: true, force: true }); }; } From b516ebc8e70919ffdf208ad9c41b82cacb5384fd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 13:17:22 +0200 Subject: [PATCH 18/35] backend-test-utils: MockDirectory tests + fixes Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 159 ++++++++++++++++++ .../src/filesystem/MockDirectory.ts | 18 +- 2 files changed, 169 insertions(+), 8 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 22563b7a59..562acb3a8a 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -14,11 +14,28 @@ * limitations under the License. */ +import fs from 'fs-extra'; +import os from 'os'; +import { relative as relativePath } from 'path'; import { MockDirectory } from './MockDirectory'; describe('MockDirectory', () => { const mockDir = MockDirectory.create(); + beforeEach(mockDir.clear); + + it('should resolve paths', () => { + expect(mockDir.path).toEqual(expect.any(String)); + expect(relativePath(mockDir.path, mockDir.resolve('a'))).toBe('a'); + expect(relativePath(mockDir.path, mockDir.resolve('a/b/c'))).toBe('a/b/c'); + }); + + it('should remove itself', async () => { + await expect(fs.pathExists(mockDir.path)).resolves.toBe(true); + await mockDir.remove(); + await expect(fs.pathExists(mockDir.path)).resolves.toBe(false); + }); + it('should populate a directory with text files', async () => { await mockDir.setContent({ 'a.txt': 'a', @@ -100,6 +117,102 @@ describe('MockDirectory', () => { }); }); + it('should read content from sub dirs', async () => { + await mockDir.setContent({ + 'a.txt': 'a', + 'b/b.txt': 'b', + 'b/c/c.txt': 'c', + }); + + const expected = { + 'a.txt': 'a', + b: { + 'b.txt': 'b', + c: { + 'c.txt': 'c', + }, + }, + }; + + await expect(mockDir.content()).resolves.toEqual(expected); + await expect(mockDir.content({ path: mockDir.path })).resolves.toEqual( + expected, + ); + await expect( + mockDir.content({ path: mockDir.resolve('.') }), + ).resolves.toEqual(expected); + await expect(mockDir.content({ path: 'b' })).resolves.toEqual(expected.b); + await expect(mockDir.content({ path: './b' })).resolves.toEqual(expected.b); + await expect( + mockDir.content({ path: mockDir.resolve('b') }), + ).resolves.toEqual(expected.b); + await expect(mockDir.content({ path: 'b/c' })).resolves.toEqual( + expected.b.c, + ); + await expect(mockDir.content({ path: './b/c' })).resolves.toEqual( + expected.b.c, + ); + await expect( + mockDir.content({ path: mockDir.resolve('b/c') }), + ).resolves.toEqual(expected.b.c); + await expect( + mockDir.content({ path: mockDir.resolve('b', 'c') }), + ).resolves.toEqual(expected.b.c); + }); + + it('should allow text reading to be configured', async () => { + const text = 'a'; + const binary = Buffer.from('a', 'utf8'); + + await mockDir.setContent({ + a: binary, + 'a.txt': text, + 'a.bin': binary, + }); + + await expect(mockDir.content()).resolves.toEqual({ + a: binary, + 'a.txt': text, + 'a.bin': binary, + }); + + await expect(mockDir.content({ shouldReadAsText: false })).resolves.toEqual( + { + a: binary, + 'a.txt': binary, + 'a.bin': binary, + }, + ); + + await expect(mockDir.content({ shouldReadAsText: true })).resolves.toEqual({ + a: text, + 'a.txt': text, + 'a.bin': text, + }); + + await expect( + mockDir.content({ shouldReadAsText: path => path.length > 3 }), + ).resolves.toEqual({ + a: binary, + 'a.txt': text, + 'a.bin': text, + }); + }); + + it('should provide a posix path to shouldReadAsText', async () => { + const shouldReadAsText = jest.fn().mockReturnValue(true); + + await mockDir.setContent({ 'a/b/c': 'c' }); + + await expect(mockDir.content({ shouldReadAsText })).resolves.toEqual({ + a: { b: { c: 'c' } }, + }); + expect(shouldReadAsText).toHaveBeenCalledWith( + 'a/b/c', + Buffer.from('c', 'utf8'), + ); + }); + it('should not override directories', async () => { await mockDir.setContent({ 'a.txt': 'a', @@ -153,6 +266,18 @@ describe('MockDirectory', () => { }); }); + it('should reject non-child paths', async () => { + await expect(mockDir.setContent({ '/root/a.txt': 'a' })).rejects.toThrow( + "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + ); + await expect(mockDir.addContent({ '/root/a.txt': 'a' })).rejects.toThrow( + "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + ); + await expect(mockDir.content({ path: '/root/a.txt' })).rejects.toThrow( + "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + ); + }); + describe('cleanup', () => { let cleanupMockDir: MockDirectory; @@ -174,4 +299,38 @@ describe('MockDirectory', () => { await expect(cleanupMockDir.content()).resolves.toBeUndefined(); }); }); + + describe('tmpdir mock', () => { + let tmpDirMock: MockDirectory; + + describe('inner', () => { + tmpDirMock = MockDirectory.mockOsTmpDir(); + + it('should mock os.tmpdir()', async () => { + expect(os.tmpdir()).toBe(tmpDirMock.path); + }); + }); + + it('should restore os.tmpdir()', async () => { + expect(os.tmpdir()).not.toBe(tmpDirMock.path); + }); + }); + + describe('existing directory', () => { + let existingMockDir: MockDirectory; + + describe('inner', () => { + existingMockDir = MockDirectory.create({ root: __dirname }); // hardcore mode + + it('should read existing directory', async () => { + await expect(existingMockDir.content()).resolves.toMatchObject({ + 'index.ts': expect.any(String), + }); + }); + }); + + it('should remove existing directory', async () => { + await expect(fs.pathExists(__dirname)).resolves.toBe(true); + }); + }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index cd85a4c347..5553982484 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -15,7 +15,7 @@ */ import os from 'os'; -import { isChildPath, resolveSafeChildPath } from '@backstage/backend-common'; +import { isChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; import { tmpdir as getTmpDir } from 'os'; @@ -25,6 +25,8 @@ import { join as joinPath, resolve as resolvePath, relative as relativePath, + win32, + posix, } from 'path'; /** @@ -233,10 +235,10 @@ export class MockDirectory { const entries = this.#transformInput(root); for (const entry of entries) { - const fullPath = resolveSafeChildPath(this.#root, entry.path); + const fullPath = resolvePath(this.#root, entry.path); if (!isChildPath(this.#root, fullPath)) { throw new Error( - `Provided path must resolve to a child path of the mock directory, got ${entry.path}`, + `Provided path must resolve to a child path of the mock directory, got '${entry.path}'`, ); } @@ -284,10 +286,7 @@ export class MockDirectory { const root = resolvePath(this.#root, options?.path ?? ''); if (!isChildPath(this.#root, root)) { throw new Error( - `Provided path must resolve to a child path of the mock directory, got ${relativePath( - this.#root, - root, - )}`, + `Provided path must resolve to a child path of the mock directory, got '${root}'`, ); } @@ -308,8 +307,11 @@ export class MockDirectory { return [entry.name, await read(fullPath)]; } const content = await fs.readFile(fullPath); + const relativePosixPath = relativePath(root, fullPath) + .split(win32.sep) + .join(posix.sep); - if (shouldReadAsText(fullPath, content)) { + if (shouldReadAsText(relativePosixPath, content)) { return [entry.name, content.toString('utf8')]; } return [entry.name, content]; From 71b63f6ad129fd9bcef56b0fec8e976f99ca6a86 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 17:33:21 +0200 Subject: [PATCH 19/35] backend-test-utils: MockDirectory fixes for windows Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/package.json | 1 + .../src/filesystem/MockDirectory.test.ts | 17 ++++++++++++----- .../src/filesystem/MockDirectory.ts | 12 +++++++++++- yarn.lock | 1 + 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index da626d3786..0a91508400 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -46,6 +46,7 @@ "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/config": "workspace:^", + "@backstage/errors": "workspace:^", "@backstage/plugin-auth-node": "workspace:^", "@backstage/types": "workspace:^", "better-sqlite3": "^8.0.0", diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 562acb3a8a..a7fe1815b2 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -16,7 +16,11 @@ import fs from 'fs-extra'; import os from 'os'; -import { relative as relativePath } from 'path'; +import { + join as joinPath, + resolve as resolvePath, + relative as relativePath, +} from 'path'; import { MockDirectory } from './MockDirectory'; describe('MockDirectory', () => { @@ -27,7 +31,9 @@ describe('MockDirectory', () => { it('should resolve paths', () => { expect(mockDir.path).toEqual(expect.any(String)); expect(relativePath(mockDir.path, mockDir.resolve('a'))).toBe('a'); - expect(relativePath(mockDir.path, mockDir.resolve('a/b/c'))).toBe('a/b/c'); + expect(relativePath(mockDir.path, mockDir.resolve('a/b/c'))).toBe( + joinPath('a', 'b', 'c'), + ); }); it('should remove itself', async () => { @@ -267,14 +273,15 @@ describe('MockDirectory', () => { }); it('should reject non-child paths', async () => { + const path = resolvePath('/root/a.txt'); await expect(mockDir.setContent({ '/root/a.txt': 'a' })).rejects.toThrow( - "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); await expect(mockDir.addContent({ '/root/a.txt': 'a' })).rejects.toThrow( - "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); await expect(mockDir.content({ path: '/root/a.txt' })).rejects.toThrow( - "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 5553982484..371a315f22 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -28,6 +28,7 @@ import { win32, posix, } from 'path'; +import { isError } from '@backstage/errors'; /** * The content of a mock directory represented by a nested object structure. @@ -334,7 +335,16 @@ export class MockDirectory { * Removes the mock directory and all its contents. */ remove = async (): Promise => { - await fs.rm(this.#root, { recursive: true, force: true }); + try { + await fs.rm(this.#root, { recursive: true, force: true }); + } catch (error: unknown) { + if (isError(error) && error.code === 'ENOTEMPTY') { + // Windows can be a bit flaky, give it another go + await fs.rm(this.#root, { recursive: true, force: true }); + } else { + throw error; + } + } }; #transformInput(input: MockDirectoryContent[string]): MockEntry[] { diff --git a/yarn.lock b/yarn.lock index dc8e58da96..20922a9959 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3642,6 +3642,7 @@ __metadata: "@backstage/backend-plugin-api": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" + "@backstage/errors": "workspace:^" "@backstage/plugin-auth-node": "workspace:^" "@backstage/types": "workspace:^" "@types/supertest": ^2.0.8 From 95dab5827e50c25f78d81a4d5aaaaed06674f229 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 17:36:53 +0200 Subject: [PATCH 20/35] backend-common: disable GerritUrlReader tests again Signed-off-by: Patrik Oldsberg --- packages/backend-common/src/reading/GerritUrlReader.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index d3c336ee93..e5a299d0b8 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -86,7 +86,10 @@ const createReader = (config: JsonObject): UrlReaderPredicateTuple[] => { }); }; -describe('GerritUrlReader', () => { +// TODO(Rugvip): These tests seem to be a direct or indirect cause of the TaskWorker test flakiness +// We're not sure why at this point, but while investigating these tests are disabled +// eslint-disable-next-line jest/no-disabled-tests +describe.skip('GerritUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); From 7d275f4587a641e13d8a9d9c503a55c6cdc25078 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 11:48:12 +0200 Subject: [PATCH 21/35] backend-test-utils: refactor MockDirectory to be synchronous Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 10 +- .../src/filesystem/MockDirectory.test.ts | 146 ++++++++---------- .../src/filesystem/MockDirectory.ts | 80 +++++----- 3 files changed, 108 insertions(+), 128 deletions(-) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index 0262ef339d..dff218edb1 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -36,17 +36,17 @@ export function isDockerDisabledForTests(): boolean; // @public export class MockDirectory { - addContent(root: MockDirectoryContent): Promise; - clear: () => Promise; + addContent(root: MockDirectoryContent): void; + clear: () => void; content( options?: MockDirectoryContentOptions, - ): Promise; + ): MockDirectoryContent | undefined; static create(options?: MockDirectoryCreateOptions): MockDirectory; static mockOsTmpDir(): MockDirectory; get path(): string; - remove: () => Promise; + remove: () => void; resolve(...paths: string[]): string; - setContent(root: MockDirectoryContent): Promise; + setContent(root: MockDirectoryContent): void; } // @public diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index a7fe1815b2..19c56119f0 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -38,19 +38,19 @@ describe('MockDirectory', () => { it('should remove itself', async () => { await expect(fs.pathExists(mockDir.path)).resolves.toBe(true); - await mockDir.remove(); + mockDir.remove(); await expect(fs.pathExists(mockDir.path)).resolves.toBe(false); }); - it('should populate a directory with text files', async () => { - await mockDir.setContent({ + it('should populate a directory with text files', () => { + mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.txt': 'c', 'a/b/d.txt': 'd', }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -62,15 +62,15 @@ describe('MockDirectory', () => { }); }); - it('should mix text and binary files', async () => { - await mockDir.setContent({ + it('should mix text and binary files', () => { + mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.bin': Buffer.from([0xc]), 'a/b/d.bin': Buffer.from([0xd]), }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -82,25 +82,25 @@ describe('MockDirectory', () => { }); }); - it('should be able to add content', async () => { - await mockDir.setContent({ + it('should be able to add content', () => { + mockDir.setContent({ 'a.txt': 'a', b: {}, }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', b: {}, }); - await mockDir.addContent({ + mockDir.addContent({ 'b.txt': 'b', b: { 'c.txt': 'c', }, }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', 'b.txt': 'b', b: { @@ -109,22 +109,22 @@ describe('MockDirectory', () => { }); }); - it('should replace existing files', async () => { - await mockDir.setContent({ + it('should replace existing files', () => { + mockDir.setContent({ 'a.txt': 'a', }); - await mockDir.addContent({ + mockDir.addContent({ 'a.txt': 'a2', }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a2', }); }); - it('should read content from sub dirs', async () => { - await mockDir.setContent({ + it('should read content from sub dirs', () => { + mockDir.setContent({ 'a.txt': 'a', 'b/b.txt': 'b', 'b/c/c.txt': 'c', @@ -140,77 +140,65 @@ describe('MockDirectory', () => { }, }; - await expect(mockDir.content()).resolves.toEqual(expected); - await expect(mockDir.content({ path: mockDir.path })).resolves.toEqual( - expected, - ); - await expect( - mockDir.content({ path: mockDir.resolve('.') }), - ).resolves.toEqual(expected); - await expect(mockDir.content({ path: 'b' })).resolves.toEqual(expected.b); - await expect(mockDir.content({ path: './b' })).resolves.toEqual(expected.b); - await expect( - mockDir.content({ path: mockDir.resolve('b') }), - ).resolves.toEqual(expected.b); - await expect(mockDir.content({ path: 'b/c' })).resolves.toEqual( + expect(mockDir.content()).toEqual(expected); + expect(mockDir.content({ path: mockDir.path })).toEqual(expected); + expect(mockDir.content({ path: mockDir.resolve('.') })).toEqual(expected); + expect(mockDir.content({ path: 'b' })).toEqual(expected.b); + expect(mockDir.content({ path: './b' })).toEqual(expected.b); + expect(mockDir.content({ path: mockDir.resolve('b') })).toEqual(expected.b); + expect(mockDir.content({ path: 'b/c' })).toEqual(expected.b.c); + expect(mockDir.content({ path: './b/c' })).toEqual(expected.b.c); + expect(mockDir.content({ path: mockDir.resolve('b/c') })).toEqual( expected.b.c, ); - await expect(mockDir.content({ path: './b/c' })).resolves.toEqual( + expect(mockDir.content({ path: mockDir.resolve('b', 'c') })).toEqual( expected.b.c, ); - await expect( - mockDir.content({ path: mockDir.resolve('b/c') }), - ).resolves.toEqual(expected.b.c); - await expect( - mockDir.content({ path: mockDir.resolve('b', 'c') }), - ).resolves.toEqual(expected.b.c); }); - it('should allow text reading to be configured', async () => { + it('should allow text reading to be configured', () => { const text = 'a'; const binary = Buffer.from('a', 'utf8'); - await mockDir.setContent({ + mockDir.setContent({ a: binary, 'a.txt': text, 'a.bin': binary, }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ a: binary, 'a.txt': text, 'a.bin': binary, }); - await expect(mockDir.content({ shouldReadAsText: false })).resolves.toEqual( - { - a: binary, - 'a.txt': binary, - 'a.bin': binary, - }, - ); + expect(mockDir.content({ shouldReadAsText: false })).toEqual({ + a: binary, + 'a.txt': binary, + 'a.bin': binary, + }); - await expect(mockDir.content({ shouldReadAsText: true })).resolves.toEqual({ + expect(mockDir.content({ shouldReadAsText: true })).toEqual({ a: text, 'a.txt': text, 'a.bin': text, }); - await expect( + expect( mockDir.content({ shouldReadAsText: path => path.length > 3 }), - ).resolves.toEqual({ + ).toEqual({ a: binary, 'a.txt': text, 'a.bin': text, }); }); - it('should provide a posix path to shouldReadAsText', async () => { + it('should provide a posix path to shouldReadAsText', () => { const shouldReadAsText = jest.fn().mockReturnValue(true); - await mockDir.setContent({ 'a/b/c': 'c' }); + mockDir.setContent({ 'a/b/c': 'c' }); - await expect(mockDir.content({ shouldReadAsText })).resolves.toEqual({ + expect(mockDir.content({ shouldReadAsText })).toEqual({ a: { b: { c: 'c' } }, }); expect(shouldReadAsText).toHaveBeenCalledWith( @@ -219,27 +207,27 @@ describe('MockDirectory', () => { ); }); - it('should not override directories', async () => { - await mockDir.setContent({ + it('should not override directories', () => { + mockDir.setContent({ 'a.txt': 'a', b: {}, }); - await expect( + expect(() => mockDir.addContent({ 'a.txt': {}, }), - ).rejects.toThrow('EEXIST'); + ).toThrow('EEXIST'); - await expect( + expect(() => mockDir.addContent({ b: 'b', }), - ).rejects.toThrow('EISDIR'); + ).toThrow('EISDIR'); }); - it('examples should work', async () => { - await mockDir.setContent({ + it('examples should work', () => { + mockDir.setContent({ 'test.txt': 'content', 'sub-dir': { 'file.txt': 'content', @@ -249,7 +237,7 @@ describe('MockDirectory', () => { 'binary-file': Buffer.from([0, 1, 2]), }); - await mockDir.addContent({ + mockDir.addContent({ 'test.txt': 'content', 'sub-dir': { 'file.txt': 'content', @@ -259,7 +247,7 @@ describe('MockDirectory', () => { 'binary-file': Buffer.from([0, 1, 2]), }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'test.txt': 'content', 'sub-dir': { 'file.txt': 'content', @@ -272,15 +260,15 @@ describe('MockDirectory', () => { }); }); - it('should reject non-child paths', async () => { + it('should reject non-child paths', () => { const path = resolvePath('/root/a.txt'); - await expect(mockDir.setContent({ '/root/a.txt': 'a' })).rejects.toThrow( + expect(() => mockDir.setContent({ '/root/a.txt': 'a' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); - await expect(mockDir.addContent({ '/root/a.txt': 'a' })).rejects.toThrow( + expect(() => mockDir.addContent({ '/root/a.txt': 'a' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); - await expect(mockDir.content({ path: '/root/a.txt' })).rejects.toThrow( + expect(() => mockDir.content({ path: '/root/a.txt' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); }); @@ -291,19 +279,19 @@ describe('MockDirectory', () => { describe('inner', () => { cleanupMockDir = MockDirectory.create(); - it('should populate a directory', async () => { - await cleanupMockDir.setContent({ + it('should populate a directory', () => { + cleanupMockDir.setContent({ 'a.txt': 'a', }); - await expect(cleanupMockDir.content()).resolves.toEqual({ + expect(cleanupMockDir.content()).toEqual({ 'a.txt': 'a', }); }); }); - it('should clean up after itself automatically', async () => { - await expect(cleanupMockDir.content()).resolves.toBeUndefined(); + it('should clean up after itself automatically', () => { + expect(cleanupMockDir.content()).toBeUndefined(); }); }); @@ -313,12 +301,12 @@ describe('MockDirectory', () => { describe('inner', () => { tmpDirMock = MockDirectory.mockOsTmpDir(); - it('should mock os.tmpdir()', async () => { + it('should mock os.tmpdir()', () => { expect(os.tmpdir()).toBe(tmpDirMock.path); }); }); - it('should restore os.tmpdir()', async () => { + it('should restore os.tmpdir()', () => { expect(os.tmpdir()).not.toBe(tmpDirMock.path); }); }); @@ -329,15 +317,15 @@ describe('MockDirectory', () => { describe('inner', () => { existingMockDir = MockDirectory.create({ root: __dirname }); // hardcore mode - it('should read existing directory', async () => { - await expect(existingMockDir.content()).resolves.toMatchObject({ + it('should read existing directory', () => { + expect(existingMockDir.content()).toMatchObject({ 'index.ts': expect.any(String), }); }); }); - it('should remove existing directory', async () => { - await expect(fs.pathExists(__dirname)).resolves.toBe(true); + it('should remove existing directory', () => { + expect(fs.pathExistsSync(__dirname)).toBe(true); }); }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 371a315f22..b452fdcc08 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -124,9 +124,9 @@ export class MockDirectory { * describe('MySubject', () => { * const mockDir = MockDirectory.create(); * - * beforeEach(() => mockDir.clear()); + * beforeEach(mockDir.clear); * - * it('should work', async () => { + * it('should work', () => { * // ... use mockDir * }) * }) @@ -141,7 +141,7 @@ export class MockDirectory { const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); if (shouldCleanup) { - process.on('beforeExit', mocker.#removeSync); + process.on('beforeExit', mocker.remove); try { afterAll(mocker.remove); @@ -199,7 +199,7 @@ export class MockDirectory { * * @example * ```ts - * await mockDir.setContent({ + * mockDir.setContent({ * 'test.txt': 'content', * 'sub-dir': { * 'file.txt': 'content', @@ -210,8 +210,8 @@ export class MockDirectory { * }); * ``` */ - async setContent(root: MockDirectoryContent): Promise { - await this.remove(); + setContent(root: MockDirectoryContent): void { + this.remove(); return this.addContent(root); } @@ -221,7 +221,7 @@ export class MockDirectory { * * @example * ```ts - * await mockDir.addContent({ + * mockDir.addContent({ * 'test.txt': 'content', * 'sub-dir': { * 'file.txt': 'content', @@ -232,7 +232,7 @@ export class MockDirectory { * }); * ``` */ - async addContent(root: MockDirectoryContent): Promise { + addContent(root: MockDirectoryContent): void { const entries = this.#transformInput(root); for (const entry of entries) { @@ -244,10 +244,10 @@ export class MockDirectory { } if (entry.type === 'dir') { - await fs.ensureDir(fullPath); + fs.ensureDirSync(fullPath); } else if (entry.type === 'file') { - await fs.ensureDir(dirname(fullPath)); - await fs.writeFile(fullPath, entry.content); + fs.ensureDirSync(dirname(fullPath)); + fs.writeFileSync(fullPath, entry.content); } } } @@ -262,7 +262,7 @@ export class MockDirectory { * * @example * ```ts - * await expect(mockDir.content()).resolves.toEqual({ + * expect(mockDir.content()).toEqual({ * 'test.txt': 'content', * 'sub-dir': { * 'file.txt': 'content', @@ -275,9 +275,9 @@ export class MockDirectory { * }); * ``` */ - async content( + content( options?: MockDirectoryContentOptions, - ): Promise { + ): MockDirectoryContent | undefined { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' ? () => options?.shouldReadAsText @@ -291,33 +291,29 @@ export class MockDirectory { ); } - async function read( - path: string, - ): Promise { - if (!(await fs.pathExists(path))) { + function read(path: string): MockDirectoryContent | undefined { + if (!fs.pathExistsSync(path)) { return undefined; } - const entries = await fs.readdir(path, { withFileTypes: true }); + const entries = fs.readdirSync(path, { withFileTypes: true }); return Object.fromEntries( - await Promise.all( - entries.map(async entry => { - const fullPath = resolvePath(path, entry.name); + entries.map(entry => { + const fullPath = resolvePath(path, entry.name); - if (entry.isDirectory()) { - return [entry.name, await read(fullPath)]; - } - const content = await fs.readFile(fullPath); - const relativePosixPath = relativePath(root, fullPath) - .split(win32.sep) - .join(posix.sep); + if (entry.isDirectory()) { + return [entry.name, read(fullPath)]; + } + const content = fs.readFileSync(fullPath); + const relativePosixPath = relativePath(root, fullPath) + .split(win32.sep) + .join(posix.sep); - if (shouldReadAsText(relativePosixPath, content)) { - return [entry.name, content.toString('utf8')]; - } - return [entry.name, content]; - }), - ), + if (shouldReadAsText(relativePosixPath, content)) { + return [entry.name, content.toString('utf8')]; + } + return [entry.name, content]; + }), ); } @@ -327,20 +323,20 @@ export class MockDirectory { /** * Clears the content of the mock directory, ensuring that the directory itself exists. */ - clear = async (): Promise => { - await this.setContent({}); + clear = (): void => { + this.setContent({}); }; /** * Removes the mock directory and all its contents. */ - remove = async (): Promise => { + remove = (): void => { try { - await fs.rm(this.#root, { recursive: true, force: true }); + fs.rmSync(this.#root, { recursive: true, force: true }); } catch (error: unknown) { if (isError(error) && error.code === 'ENOTEMPTY') { // Windows can be a bit flaky, give it another go - await fs.rm(this.#root, { recursive: true, force: true }); + fs.rmSync(this.#root, { recursive: true, force: true }); } else { throw error; } @@ -372,8 +368,4 @@ export class MockDirectory { return entries; } - - #removeSync = () => { - fs.rmSync(this.#root, { recursive: true, force: true }); - }; } From 587644c044b938f2ead04af945faa76aa88e9109 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 11:53:57 +0200 Subject: [PATCH 22/35] backend-common: update MockDirectory usage Signed-off-by: Patrik Oldsberg --- .../src/reading/AzureUrlReader.test.ts | 2 +- .../reading/BitbucketCloudUrlReader.test.ts | 2 +- .../src/reading/BitbucketUrlReader.test.ts | 2 +- .../src/reading/GerritUrlReader.test.ts | 4 ++-- .../src/reading/GithubUrlReader.test.ts | 2 +- .../src/reading/GitlabUrlReader.test.ts | 2 +- .../reading/tree/ReadableArrayResponse.test.ts | 8 ++++---- .../reading/tree/TarArchiveResponse.test.ts | 18 +++++++++--------- .../reading/tree/ZipArchiveResponse.test.ts | 16 ++++++++-------- .../src/util/DockerContainerRunner.test.ts | 6 +++--- 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index b6338a1c70..45d64c5b96 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -73,7 +73,7 @@ const urlReaderFactory = (azureIntegration: AzureIntegrationConfigLike) => { }; describe('AzureUrlReader', () => { - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts index 18a186f793..7b1e0f9fb3 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts @@ -53,7 +53,7 @@ const reader = new BitbucketCloudUrlReader( ); describe('BitbucketCloudUrlReader', () => { - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 986619d161..5e3884cf8e 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -71,7 +71,7 @@ describe('BitbucketUrlReader.factory', () => { describe('BitbucketUrlReader', () => { const mockDir = MockDirectory.mockOsTmpDir(); - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index e5a299d0b8..d91e49e533 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -93,7 +93,7 @@ describe.skip('GerritUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); afterAll(() => { jest.clearAllMocks(); @@ -256,7 +256,7 @@ describe.skip('GerritUrlReader', () => { ); beforeEach(async () => { - await mockDir.setContent({ + mockDir.setContent({ 'repo/mkdocs.yml': mkdocsContent, 'repo/docs/first.md': mdContent, }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 326d3632a4..82ce07a3a9 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -76,7 +76,7 @@ describe('GithubUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); beforeEach(() => { jest.clearAllMocks(); diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 6500f207f0..098d48458d 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -69,7 +69,7 @@ const hostedGitlabProcessor = new GitlabUrlReader( ); describe('GitlabUrlReader', () => { - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index 9f0fd5377e..1c95f653c1 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -34,12 +34,12 @@ describe('ReadableArrayResponse', () => { const sourceDir = MockDirectory.create(); const targetDir = MockDirectory.create(); - beforeEach(async () => { - await sourceDir.setContent({ + beforeEach(() => { + sourceDir.setContent({ [name1]: file1, [name2]: file2, }); - await targetDir.clear(); + targetDir.clear(); }); const path1 = sourceDir.resolve(name1); @@ -71,7 +71,7 @@ describe('ReadableArrayResponse', () => { const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); const dir = await res.dir(); - await expect(targetDir.content({ path: dir })).resolves.toEqual({ + expect(targetDir.content({ path: dir })).toEqual({ [name1]: file1.toString('utf8'), [name2]: file2.toString('utf8'), }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index ce126eb345..784a4fb9bf 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -27,11 +27,11 @@ describe('TarArchiveResponse', () => { const sourceDir = MockDirectory.create(); const targetDir = MockDirectory.create(); - beforeAll(async () => { - await sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); + beforeAll(() => { + sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); }); - beforeEach(async () => { - await targetDir.clear(); + beforeEach(() => { + targetDir.clear(); }); it('should read files', async () => { @@ -143,7 +143,7 @@ describe('TarArchiveResponse', () => { const res = new TarArchiveResponse(stream, 'docs', targetDir.path, 'etag'); const dir = await res.dir(); - await expect(targetDir.content({ path: dir })).resolves.toEqual({ + expect(targetDir.content({ path: dir })).toEqual({ 'index.md': '# Test\n', }); }); @@ -161,11 +161,11 @@ describe('TarArchiveResponse', () => { path => path.endsWith('.yml'), ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const dir = await res.dir({ targetDir: targetDir.resolve('sub') }); expect(dir).toBe(targetDir.resolve('sub')); - await expect(targetDir.content()).resolves.toEqual({ + expect(targetDir.content()).toEqual({ sub: { 'mkdocs.yml': 'site_name: Test\n', }, @@ -187,7 +187,7 @@ describe('TarArchiveResponse', () => { }, ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); const mkdtemp = jest @@ -216,7 +216,7 @@ describe('TarArchiveResponse', () => { }, ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); await expect(fs.pathExists(sub)).resolves.toBe(true); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 20cf33241b..c92ee46aad 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -38,16 +38,16 @@ describe('ZipArchiveResponse', () => { const sourceDir = MockDirectory.create(); const targetDir = MockDirectory.create(); - beforeAll(async () => { - await sourceDir.setContent({ + beforeAll(() => { + sourceDir.setContent({ 'test-archive.zip': archiveData, 'test-archive-with-extra-root-dir.zip': archiveDataWithExtraDir, 'test-archive-corrupted.zip': archiveDataCorrupted, 'test-archive-malicious.zip': archiveWithMaliciousEntry, }); }); - beforeEach(async () => { - await targetDir.clear(); + beforeEach(() => { + targetDir.clear(); }); it('should read files', async () => { @@ -151,7 +151,7 @@ describe('ZipArchiveResponse', () => { const res = new ZipArchiveResponse(stream, 'docs/', targetDir.path, 'etag'); const dir = await res.dir(); - await expect(targetDir.content({ path: dir })).resolves.toEqual({ + expect(targetDir.content({ path: dir })).toEqual({ 'index.md': '# Test\n', }); }); @@ -167,13 +167,13 @@ describe('ZipArchiveResponse', () => { path => path.endsWith('.yml'), ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); const dir = await res.dir({ targetDir: sub }); expect(dir).toBe(sub); - await expect(targetDir.content()).resolves.toEqual({ + expect(targetDir.content()).toEqual({ sub: { 'mkdocs.yml': 'site_name: Test\n', }, @@ -209,7 +209,7 @@ describe('ZipArchiveResponse', () => { const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); const dir = await res.dir({ targetDir: sub, diff --git a/packages/backend-common/src/util/DockerContainerRunner.test.ts b/packages/backend-common/src/util/DockerContainerRunner.test.ts index 6b4150540a..60f457a670 100644 --- a/packages/backend-common/src/util/DockerContainerRunner.test.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.test.ts @@ -29,9 +29,9 @@ describe('DockerContainerRunner', () => { const inputDir = MockDirectory.create(); const outputDir = MockDirectory.create(); - beforeEach(async () => { - await inputDir.clear(); - await outputDir.clear(); + beforeEach(() => { + inputDir.clear(); + outputDir.clear(); jest.spyOn(mockDocker, 'pull').mockImplementation((async ( _image: string, From a71dbf4b34e12cd4e09ebdc00a918898d6b4dfbc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 13:53:20 +0200 Subject: [PATCH 23/35] backend-test-utils: make sure MockDirectory contents are created with full permissions Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index b452fdcc08..809fd5ddf7 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -244,10 +244,10 @@ export class MockDirectory { } if (entry.type === 'dir') { - fs.ensureDirSync(fullPath); + fs.ensureDirSync(fullPath, { mode: 0o777 }); } else if (entry.type === 'file') { - fs.ensureDirSync(dirname(fullPath)); - fs.writeFileSync(fullPath, entry.content); + fs.ensureDirSync(dirname(fullPath), { mode: 0o777 }); + fs.writeFileSync(fullPath, entry.content, { mode: 0o666 }); } } } From 7c6141f54d82ff68bcf223936b1488d67eaa67d0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 13:53:47 +0200 Subject: [PATCH 24/35] backend-test-utils: use built-in rmSync retries in MockDirectory Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 809fd5ddf7..499c7f29fb 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -28,7 +28,6 @@ import { win32, posix, } from 'path'; -import { isError } from '@backstage/errors'; /** * The content of a mock directory represented by a nested object structure. @@ -331,16 +330,7 @@ export class MockDirectory { * Removes the mock directory and all its contents. */ remove = (): void => { - try { - fs.rmSync(this.#root, { recursive: true, force: true }); - } catch (error: unknown) { - if (isError(error) && error.code === 'ENOTEMPTY') { - // Windows can be a bit flaky, give it another go - fs.rmSync(this.#root, { recursive: true, force: true }); - } else { - throw error; - } - } + fs.rmSync(this.#root, { recursive: true, force: true, maxRetries: 3 }); }; #transformInput(input: MockDirectoryContent[string]): MockEntry[] { From 82752ab5cbfde00e56da948af2f42ff666fce5ff Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Sep 2023 16:41:04 +0200 Subject: [PATCH 25/35] backend-test-utils: throw resolved path when trying to write outside dir in MockDirectory Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 499c7f29fb..2f949987f5 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -238,7 +238,7 @@ export class MockDirectory { const fullPath = resolvePath(this.#root, entry.path); if (!isChildPath(this.#root, fullPath)) { throw new Error( - `Provided path must resolve to a child path of the mock directory, got '${entry.path}'`, + `Provided path must resolve to a child path of the mock directory, got '${fullPath}'`, ); } From 0f58df31d6d5f9c226b10d225f5122437774fbad Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Sep 2023 16:42:44 +0200 Subject: [PATCH 26/35] backend-test-utils: use remove instead of rm in MockDirectory Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 2f949987f5..cb7ca5e726 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -330,7 +330,7 @@ export class MockDirectory { * Removes the mock directory and all its contents. */ remove = (): void => { - fs.rmSync(this.#root, { recursive: true, force: true, maxRetries: 3 }); + fs.removeSync(this.#root); }; #transformInput(input: MockDirectoryContent[string]): MockEntry[] { From 81726cd2d45a504e373baff347d8cfe3f7c7ef01 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 16:54:44 +0200 Subject: [PATCH 27/35] backend-test-utils: refactor to createMockDirectory + remove root option Signed-off-by: Patrik Oldsberg --- .changeset/flat-pandas-tan.md | 2 +- .../src/reading/AzureUrlReader.test.ts | 4 +- .../reading/BitbucketCloudUrlReader.test.ts | 4 +- .../reading/BitbucketServerUrlReader.test.ts | 4 +- .../src/reading/BitbucketUrlReader.test.ts | 4 +- .../src/reading/GerritUrlReader.test.ts | 4 +- .../src/reading/GithubUrlReader.test.ts | 4 +- .../src/reading/GitlabUrlReader.test.ts | 4 +- .../tree/ReadableArrayResponse.test.ts | 6 +- .../reading/tree/TarArchiveResponse.test.ts | 6 +- .../reading/tree/ZipArchiveResponse.test.ts | 6 +- .../src/util/DockerContainerRunner.test.ts | 6 +- packages/backend-test-utils/api-report.md | 19 +- .../src/filesystem/MockDirectory.test.ts | 28 +- .../src/filesystem/MockDirectory.ts | 273 +++++++++--------- .../src/filesystem/index.ts | 5 +- 16 files changed, 186 insertions(+), 193 deletions(-) diff --git a/.changeset/flat-pandas-tan.md b/.changeset/flat-pandas-tan.md index 8461c9beb8..dfe7965bec 100644 --- a/.changeset/flat-pandas-tan.md +++ b/.changeset/flat-pandas-tan.md @@ -2,4 +2,4 @@ '@backstage/backend-test-utils': patch --- -Added `MockDirectory` to help out with file system mocking in tests. +Added `createMockDirectory()` to help out with file system mocking in tests. diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index 45d64c5b96..2b4bc84be7 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -24,7 +24,7 @@ import { AzureIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -44,7 +44,7 @@ type AzureIntegrationConfigLike = Partial< const logger = getVoidLogger(); -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts index 7b1e0f9fb3..dc182af619 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts @@ -20,7 +20,7 @@ import { readBitbucketCloudIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -32,7 +32,7 @@ import { BitbucketCloudUrlReader } from './BitbucketCloudUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; import getRawBody from 'raw-body'; -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts index 82b467f70f..2870138535 100644 --- a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts @@ -20,7 +20,7 @@ import { readBitbucketServerIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -31,7 +31,7 @@ import { NotModifiedError } from '@backstage/errors'; import { BitbucketServerUrlReader } from './BitbucketServerUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; -MockDirectory.mockOsTmpDir(); +createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 5e3884cf8e..4258ac9d5b 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -20,7 +20,7 @@ import { readBitbucketIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -69,7 +69,7 @@ describe('BitbucketUrlReader.factory', () => { }); describe('BitbucketUrlReader', () => { - const mockDir = MockDirectory.mockOsTmpDir(); + const mockDir = createMockDirectory({ mockOsTmpDir: true }); beforeEach(mockDir.clear); diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index d91e49e533..4a3b0a5de6 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -15,7 +15,7 @@ */ import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; @@ -35,7 +35,7 @@ import { DefaultReadTreeResponseFactory } from './tree'; import { GerritUrlReader } from './GerritUrlReader'; import getRawBody from 'raw-body'; -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 82ce07a3a9..5e6d30f519 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -21,7 +21,7 @@ import { readGithubIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -38,7 +38,7 @@ import { } from './GithubUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 098d48458d..3c065e8b81 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -16,7 +16,7 @@ import { ConfigReader } from '@backstage/config'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -34,7 +34,7 @@ import { const logger = getVoidLogger(); -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index 1c95f653c1..4108697bce 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -18,7 +18,7 @@ import fs from 'fs-extra'; import path from 'path'; import { FromReadableArrayOptions } from '../types'; import { ReadableArrayResponse } from './ReadableArrayResponse'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const name1 = 'file1.yaml'; const file1 = fs.readFileSync( @@ -31,8 +31,8 @@ const file2 = fs.readFileSync( ); describe('ReadableArrayResponse', () => { - const sourceDir = MockDirectory.create(); - const targetDir = MockDirectory.create(); + const sourceDir = createMockDirectory(); + const targetDir = createMockDirectory(); beforeEach(() => { sourceDir.setContent({ diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index 784a4fb9bf..f69fb2e0e1 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -17,15 +17,15 @@ import fs from 'fs-extra'; import { resolve as resolvePath } from 'path'; import { TarArchiveResponse } from './TarArchiveResponse'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'), ); describe('TarArchiveResponse', () => { - const sourceDir = MockDirectory.create(); - const targetDir = MockDirectory.create(); + const sourceDir = createMockDirectory(); + const targetDir = createMockDirectory(); beforeAll(() => { sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index c92ee46aad..0eecad2098 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -19,7 +19,7 @@ import { Readable } from 'stream'; import { create as createArchive } from 'archiver'; import { resolve as resolvePath } from 'path'; import { ZipArchiveResponse } from './ZipArchiveResponse'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.zip'), @@ -35,8 +35,8 @@ const archiveWithMaliciousEntry = fs.readFileSync( ); describe('ZipArchiveResponse', () => { - const sourceDir = MockDirectory.create(); - const targetDir = MockDirectory.create(); + const sourceDir = createMockDirectory(); + const targetDir = createMockDirectory(); beforeAll(() => { sourceDir.setContent({ diff --git a/packages/backend-common/src/util/DockerContainerRunner.test.ts b/packages/backend-common/src/util/DockerContainerRunner.test.ts index 60f457a670..737bf3d178 100644 --- a/packages/backend-common/src/util/DockerContainerRunner.test.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.test.ts @@ -19,15 +19,15 @@ import Docker from 'dockerode'; import Stream, { PassThrough } from 'stream'; import { ContainerRunner } from './ContainerRunner'; import { DockerContainerRunner, UserOptions } from './DockerContainerRunner'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const mockDocker = new Docker() as jest.Mocked; describe('DockerContainerRunner', () => { let containerTaskApi: ContainerRunner; - const inputDir = MockDirectory.create(); - const outputDir = MockDirectory.create(); + const inputDir = createMockDirectory(); + const outputDir = createMockDirectory(); beforeEach(() => { inputDir.clear(); diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index dff218edb1..ac4c5ffe85 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -31,20 +31,23 @@ import { ServiceRef } from '@backstage/backend-plugin-api'; import { TokenManagerService } from '@backstage/backend-plugin-api'; import { UrlReaderService } from '@backstage/backend-plugin-api'; +// @public +export function createMockDirectory( + options?: MockDirectoryOptions, +): MockDirectory; + // @public (undocumented) export function isDockerDisabledForTests(): boolean; // @public -export class MockDirectory { +export interface MockDirectory { addContent(root: MockDirectoryContent): void; - clear: () => void; + clear(): void; content( options?: MockDirectoryContentOptions, ): MockDirectoryContent | undefined; - static create(options?: MockDirectoryCreateOptions): MockDirectory; - static mockOsTmpDir(): MockDirectory; - get path(): string; - remove: () => void; + readonly path: string; + remove(): void; resolve(...paths: string[]): string; setContent(root: MockDirectoryContent): void; } @@ -61,8 +64,8 @@ export interface MockDirectoryContentOptions { } // @public -export interface MockDirectoryCreateOptions { - root?: string; +export interface MockDirectoryOptions { + mockOsTmpDir?: boolean; } // @public (undocumented) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 19c56119f0..8c4ff49e64 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -21,10 +21,10 @@ import { resolve as resolvePath, relative as relativePath, } from 'path'; -import { MockDirectory } from './MockDirectory'; +import { createMockDirectory, MockDirectory } from './MockDirectory'; -describe('MockDirectory', () => { - const mockDir = MockDirectory.create(); +describe('createMockDirectory', () => { + const mockDir = createMockDirectory(); beforeEach(mockDir.clear); @@ -277,7 +277,7 @@ describe('MockDirectory', () => { let cleanupMockDir: MockDirectory; describe('inner', () => { - cleanupMockDir = MockDirectory.create(); + cleanupMockDir = createMockDirectory(); it('should populate a directory', () => { cleanupMockDir.setContent({ @@ -299,7 +299,7 @@ describe('MockDirectory', () => { let tmpDirMock: MockDirectory; describe('inner', () => { - tmpDirMock = MockDirectory.mockOsTmpDir(); + tmpDirMock = createMockDirectory({ mockOsTmpDir: true }); it('should mock os.tmpdir()', () => { expect(os.tmpdir()).toBe(tmpDirMock.path); @@ -310,22 +310,4 @@ describe('MockDirectory', () => { expect(os.tmpdir()).not.toBe(tmpDirMock.path); }); }); - - describe('existing directory', () => { - let existingMockDir: MockDirectory; - - describe('inner', () => { - existingMockDir = MockDirectory.create({ root: __dirname }); // hardcore mode - - it('should read existing directory', () => { - expect(existingMockDir.content()).toMatchObject({ - 'index.ts': expect.any(String), - }); - }); - }); - - it('should remove existing directory', () => { - expect(fs.pathExistsSync(__dirname)).toBe(true); - }); - }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index cb7ca5e726..43df843946 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -56,20 +56,6 @@ export type MockDirectoryContent = { [name in string]: MockDirectoryContent | string | Buffer; }; -/** - * Options for {@link MockDirectory.create}. - * - * @public - */ -export interface MockDirectoryCreateOptions { - /** - * The root path to create the directory in. Defaults to a temporary directory. - * - * If an existing directory is provided, it will not be cleaned up after the test. - */ - root?: string; -} - /** * Options for {@link MockDirectory.content}. * @@ -91,107 +77,21 @@ export interface MockDirectoryContentOptions { shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); } -/** @internal */ -type MockEntry = - | { - type: 'file'; - path: string; - content: Buffer; - } - | { - type: 'dir'; - path: string; - }; - /** * A utility for creating a mock directory that is automatically cleaned up. * * @public */ -export class MockDirectory { - /** - * Creates a new temporary mock directory that will be removed after the tests have completed. - * - * @remarks - * - * This method is intended to be called outside of any test, either at top-level or - * within a `describe` block. It will call `afterAll` to make sure that the mock directory - * is removed after the tests have run. - * - * @example - * ```ts - * describe('MySubject', () => { - * const mockDir = MockDirectory.create(); - * - * beforeEach(mockDir.clear); - * - * it('should work', () => { - * // ... use mockDir - * }) - * }) - * ``` - */ - static create(options?: MockDirectoryCreateOptions): MockDirectory { - const root = - options?.root ?? - fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); - - const mocker = new MockDirectory(root); - - const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); - if (shouldCleanup) { - process.on('beforeExit', mocker.remove); - - try { - afterAll(mocker.remove); - } catch { - /* ignore */ - } - } - - return mocker; - } - - /** - * Like {@link MockDirectory.create}, but also mocks `os.tmpdir()` to return the - * mock directory path until the end of the test suite. - * - * @returns - */ - static mockOsTmpDir(): MockDirectory { - const mocker = MockDirectory.create(); - const origTmpdir = os.tmpdir; - os.tmpdir = () => mocker.path; - - try { - afterAll(() => { - os.tmpdir = origTmpdir; - }); - } catch { - /* ignore */ - } - return mocker; - } - - readonly #root: string; - - private constructor(root: string) { - this.#root = root; - } - +export interface MockDirectory { /** * The path to the root of the mock directory */ - get path(): string { - return this.#root; - } + readonly path: string; /** * Resolves a path relative to the root of the mock directory. */ - resolve(...paths: string[]): string { - return resolvePath(this.#root, ...paths); - } + resolve(...paths: string[]): string; /** * Sets the content of the mock directory. This will remove any existing content. @@ -209,11 +109,7 @@ export class MockDirectory { * }); * ``` */ - setContent(root: MockDirectoryContent): void { - this.remove(); - - return this.addContent(root); - } + setContent(root: MockDirectoryContent): void; /** * Adds content of the mock directory. This will overwrite existing files. @@ -231,25 +127,7 @@ export class MockDirectory { * }); * ``` */ - addContent(root: MockDirectoryContent): void { - const entries = this.#transformInput(root); - - for (const entry of entries) { - const fullPath = resolvePath(this.#root, entry.path); - if (!isChildPath(this.#root, fullPath)) { - throw new Error( - `Provided path must resolve to a child path of the mock directory, got '${fullPath}'`, - ); - } - - if (entry.type === 'dir') { - fs.ensureDirSync(fullPath, { mode: 0o777 }); - } else if (entry.type === 'file') { - fs.ensureDirSync(dirname(fullPath), { mode: 0o777 }); - fs.writeFileSync(fullPath, entry.content, { mode: 0o666 }); - } - } - } + addContent(root: MockDirectoryContent): void; /** * Reads the content of the mock directory. @@ -274,6 +152,75 @@ export class MockDirectory { * }); * ``` */ + content( + options?: MockDirectoryContentOptions, + ): MockDirectoryContent | undefined; + + /** + * Clears the content of the mock directory, ensuring that the directory itself exists. + */ + clear(): void; + + /** + * Removes the mock directory and all its contents. + */ + remove(): void; +} + +/** @internal */ +type MockEntry = + | { + type: 'file'; + path: string; + content: Buffer; + } + | { + type: 'dir'; + path: string; + }; + +/** @internal */ +class MockDirectoryImpl { + readonly #root: string; + + constructor(root: string) { + this.#root = root; + } + + get path(): string { + return this.#root; + } + + resolve(...paths: string[]): string { + return resolvePath(this.#root, ...paths); + } + + setContent(root: MockDirectoryContent): void { + this.remove(); + + return this.addContent(root); + } + + addContent(root: MockDirectoryContent): void { + const entries = this.#transformInput(root); + + for (const entry of entries) { + const fullPath = resolvePath(this.#root, entry.path); + if (!isChildPath(this.#root, fullPath)) { + throw new Error( + `Provided path must resolve to a child path of the mock directory, got '${fullPath}'`, + ); + } + + if (entry.type === 'dir') { + fs.ensureDirSync(fullPath, { mode: 0o777 }); + } else if (entry.type === 'file') { + fs.ensureDirSync(dirname(fullPath), { mode: 0o777 }); + fs.writeFileSync(fullPath, entry.content, { mode: 0o666 }); + } + } + } + content( options?: MockDirectoryContentOptions, ): MockDirectoryContent | undefined { @@ -319,16 +266,10 @@ export class MockDirectory { return read(root); } - /** - * Clears the content of the mock directory, ensuring that the directory itself exists. - */ clear = (): void => { this.setContent({}); }; - /** - * Removes the mock directory and all its contents. - */ remove = (): void => { fs.removeSync(this.#root); }; @@ -359,3 +300,69 @@ export class MockDirectory { return entries; } } + +/** + * Options for {@link createMockDirectory}. + * + * @public + */ +export interface MockDirectoryOptions { + /** + * In addition to creating a temporary directory, also mock `os.tmpdir()` to return the + * mock directory path until the end of the test suite. + * + * @returns + */ + mockOsTmpDir?: boolean; +} + +/** + * Creates a new temporary mock directory that will be removed after the tests have completed. + * + * @public + * @remarks + * + * This method is intended to be called outside of any test, either at top-level or + * within a `describe` block. It will call `afterAll` to make sure that the mock directory + * is removed after the tests have run. + * + * @example + * ```ts + * describe('MySubject', () => { + * const mockDir = createMockDirectory(); + * + * beforeEach(mockDir.clear); + * + * it('should work', () => { + * // ... use mockDir + * }) + * }) + * ``` + */ +export function createMockDirectory( + options?: MockDirectoryOptions, +): MockDirectory { + const root = fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); + + const mocker = new MockDirectoryImpl(root); + + const origTmpdir = options?.mockOsTmpDir ? os.tmpdir : undefined; + if (origTmpdir) { + os.tmpdir = () => mocker.path; + } + + process.on('beforeExit', mocker.remove); + + try { + afterAll(() => { + if (origTmpdir) { + os.tmpdir = origTmpdir; + } + mocker.remove(); + }); + } catch { + /* ignore */ + } + + return mocker; +} diff --git a/packages/backend-test-utils/src/filesystem/index.ts b/packages/backend-test-utils/src/filesystem/index.ts index d1077db980..0a4d8c7d00 100644 --- a/packages/backend-test-utils/src/filesystem/index.ts +++ b/packages/backend-test-utils/src/filesystem/index.ts @@ -15,8 +15,9 @@ */ export { - MockDirectory, + createMockDirectory, + type MockDirectory, + type MockDirectoryOptions, type MockDirectoryContent, type MockDirectoryContentOptions, - type MockDirectoryCreateOptions, } from './MockDirectory'; From b7ae05d5b8b9d40d07016f7d9ceb2a80c5f9e4fe Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 18:59:00 +0200 Subject: [PATCH 28/35] backend-test-utils: win32 fix Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 8c4ff49e64..54d32f47d7 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -16,11 +16,7 @@ import fs from 'fs-extra'; import os from 'os'; -import { - join as joinPath, - resolve as resolvePath, - relative as relativePath, -} from 'path'; +import { join as joinPath, relative as relativePath } from 'path'; import { createMockDirectory, MockDirectory } from './MockDirectory'; describe('createMockDirectory', () => { @@ -261,7 +257,7 @@ describe('createMockDirectory', () => { }); it('should reject non-child paths', () => { - const path = resolvePath('/root/a.txt'); + const path = mockDir.resolve('/root/a.txt'); expect(() => mockDir.setContent({ '/root/a.txt': 'a' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); From da0ef5a98f0cbf26e35e81025f52dde7b8754be0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 19:01:10 +0200 Subject: [PATCH 29/35] backend-test-utils: skip mock directory cleanup in CI Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 22 ------------------- .../src/filesystem/MockDirectory.ts | 10 +++++++-- 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 54d32f47d7..45a206d7df 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -269,28 +269,6 @@ describe('createMockDirectory', () => { ); }); - describe('cleanup', () => { - let cleanupMockDir: MockDirectory; - - describe('inner', () => { - cleanupMockDir = createMockDirectory(); - - it('should populate a directory', () => { - cleanupMockDir.setContent({ - 'a.txt': 'a', - }); - - expect(cleanupMockDir.content()).toEqual({ - 'a.txt': 'a', - }); - }); - }); - - it('should clean up after itself automatically', () => { - expect(cleanupMockDir.content()).toBeUndefined(); - }); - }); - describe('tmpdir mock', () => { let tmpDirMock: MockDirectory; diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 43df843946..1d48d157df 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -351,14 +351,20 @@ export function createMockDirectory( os.tmpdir = () => mocker.path; } - process.on('beforeExit', mocker.remove); + // In CI we expect there to be no need to clean up temporary directories + const needsCleanup = !process.env.CI; + if (needsCleanup) { + process.on('beforeExit', mocker.remove); + } try { afterAll(() => { if (origTmpdir) { os.tmpdir = origTmpdir; } - mocker.remove(); + if (needsCleanup) { + mocker.remove(); + } }); } catch { /* ignore */ From 8770ca25182daacb7140b652782e8a236afbf39e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 19:14:50 +0200 Subject: [PATCH 30/35] backend-test-utils: added content option for createMockDirectory Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 1 + .../backend-test-utils/src/filesystem/MockDirectory.ts | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index ac4c5ffe85..a477f70298 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -65,6 +65,7 @@ export interface MockDirectoryContentOptions { // @public export interface MockDirectoryOptions { + content?: MockDirectoryContent; mockOsTmpDir?: boolean; } diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 1d48d157df..7619f191ec 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -314,6 +314,11 @@ export interface MockDirectoryOptions { * @returns */ mockOsTmpDir?: boolean; + + /** + * Initializes the directory with the given content, see {@link MockDirectory.setContent}. + */ + content?: MockDirectoryContent; } /** @@ -370,5 +375,9 @@ export function createMockDirectory( /* ignore */ } + if (options?.content) { + mocker.setContent(options.content); + } + return mocker; } From e2c854a398def32e01b77865d5f0b17c798eeacd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 10:39:40 +0200 Subject: [PATCH 31/35] backend-test-utils: make directory mocker use RUNNER_TEMP if available Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 7619f191ec..4ca66f2b68 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -347,7 +347,10 @@ export interface MockDirectoryOptions { export function createMockDirectory( options?: MockDirectoryOptions, ): MockDirectory { - const root = fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); + const tmpDir = process.env.RUNNER_TEMP; // GitHub Actions + const root = fs.mkdtempSync( + joinPath(tmpDir || getTmpDir(), 'backstage-tmp-test-dir-'), + ); const mocker = new MockDirectoryImpl(root); From 41866257190d58148d73d6ca1041035aa237d2a3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 10:33:39 +0200 Subject: [PATCH 32/35] .github/workflows: switch windows verification to windows-2022 Signed-off-by: Patrik Oldsberg --- .github/workflows/verify_windows.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/verify_windows.yml b/.github/workflows/verify_windows.yml index e5d3adf013..312c662b5a 100644 --- a/.github/workflows/verify_windows.yml +++ b/.github/workflows/verify_windows.yml @@ -9,9 +9,10 @@ on: jobs: build: - runs-on: windows-2019 + runs-on: windows-2022 strategy: + fail-fast: false matrix: node-version: [18.x, 20.x] From 3b7ec34b4a49db8ed64cdae474f7c416e7f3293a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 11:58:49 +0200 Subject: [PATCH 33/35] backend-test-utils: make MockDirectory refuse duplicate mock of os.tmpdir() Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 6 ++++++ .../src/filesystem/MockDirectory.ts | 17 +++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 45a206d7df..9e3efdaec0 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -278,6 +278,12 @@ describe('createMockDirectory', () => { it('should mock os.tmpdir()', () => { expect(os.tmpdir()).toBe(tmpDirMock.path); }); + + it('should refuce to mock os.tmpdir() again', () => { + expect(() => createMockDirectory({ mockOsTmpDir: true })).toThrow( + 'Cannot mock os.tmpdir() when it has already been mocked', + ); + }); }); it('should restore os.tmpdir()', () => { diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 4ca66f2b68..20345e4ff9 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -18,7 +18,6 @@ import os from 'os'; import { isChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; -import { tmpdir as getTmpDir } from 'os'; import { dirname, extname, @@ -29,6 +28,8 @@ import { posix, } from 'path'; +const tmpdirMarker = Symbol('os-tmpdir-mock'); + /** * The content of a mock directory represented by a nested object structure. * @@ -347,16 +348,20 @@ export interface MockDirectoryOptions { export function createMockDirectory( options?: MockDirectoryOptions, ): MockDirectory { - const tmpDir = process.env.RUNNER_TEMP; // GitHub Actions - const root = fs.mkdtempSync( - joinPath(tmpDir || getTmpDir(), 'backstage-tmp-test-dir-'), - ); + const tmpDir = process.env.RUNNER_TEMP || os.tmpdir(); // GitHub Actions + const root = fs.mkdtempSync(joinPath(tmpDir, 'backstage-tmp-test-dir-')); const mocker = new MockDirectoryImpl(root); const origTmpdir = options?.mockOsTmpDir ? os.tmpdir : undefined; if (origTmpdir) { - os.tmpdir = () => mocker.path; + if (Object.hasOwn(origTmpdir, tmpdirMarker)) { + throw new Error( + 'Cannot mock os.tmpdir() when it has already been mocked', + ); + } + const mock = Object.assign(() => mocker.path, { [tmpdirMarker]: true }); + os.tmpdir = mock; } // In CI we expect there to be no need to clean up temporary directories From b95d66d4ea74d56edec96f8d893a925a6ed2e83f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Oct 2023 16:08:13 +0200 Subject: [PATCH 34/35] backend-common: end write stream after use in ZipArchiveResponse Signed-off-by: Patrik Oldsberg --- .changeset/great-walls-brake.md | 5 +++++ .../backend-common/src/reading/tree/ZipArchiveResponse.ts | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 .changeset/great-walls-brake.md diff --git a/.changeset/great-walls-brake.md b/.changeset/great-walls-brake.md new file mode 100644 index 0000000000..dd71c81a07 --- /dev/null +++ b/.changeset/great-walls-brake.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Properly close write stream when writing temporary archive for processing zip-based `.readTree()` responses. diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index cd57788743..a191163517 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -93,12 +93,13 @@ export class ZipArchiveResponse implements ReadTreeResponse { return new Promise((resolve, reject) => { writeStream.on('error', reject); - writeStream.on('finish', () => + writeStream.on('finish', () => { + writeStream.end(); resolve({ fileName: tmpFile, cleanup: () => fs.rm(tmpDir, { recursive: true }), - }), - ); + }); + }); stream.pipe(writeStream); }); } From 527142b06bc064f17920f706535eddbb0f8f0350 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Oct 2023 16:10:03 +0200 Subject: [PATCH 35/35] backend-common: close write streams after use in tests Signed-off-by: Patrik Oldsberg --- .../tree/ReadableArrayResponse.test.ts | 19 +++++++++--- .../reading/tree/ZipArchiveResponse.test.ts | 31 +++++++++++++------ 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index 4108697bce..0a27bdf09c 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -42,13 +42,24 @@ describe('ReadableArrayResponse', () => { targetDir.clear(); }); + const openStreams = new Array(); + function createReadStream(filePath: string) { + const stream = fs.createReadStream(filePath); + openStreams.push(stream); + return stream; + } + afterEach(() => { + openStreams.forEach(stream => stream.destroy()); + openStreams.length = 0; + }); + const path1 = sourceDir.resolve(name1); const path2 = sourceDir.resolve(name2); it('should read files', async () => { const arr: FromReadableArrayOptions = [ - { data: fs.createReadStream(path1), path: path1 }, - { data: fs.createReadStream(path2), path: path2 }, + { data: createReadStream(path1), path: path1 }, + { data: createReadStream(path2), path: path2 }, ]; const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); @@ -64,8 +75,8 @@ describe('ReadableArrayResponse', () => { it('should extract entire archive into directory', async () => { const arr: FromReadableArrayOptions = [ - { data: fs.createReadStream(path1), path: path1 }, - { data: fs.createReadStream(path2), path: path2 }, + { data: createReadStream(path1), path: path1 }, + { data: createReadStream(path2), path: path2 }, ]; const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 0eecad2098..ccf53e7cff 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -50,8 +50,19 @@ describe('ZipArchiveResponse', () => { targetDir.clear(); }); + const openStreams = new Array(); + function createReadStream(filePath: string) { + const stream = fs.createReadStream(filePath); + openStreams.push(stream); + return stream; + } + afterEach(() => { + openStreams.forEach(stream => stream.destroy()); + openStreams.length = 0; + }); + it('should read files', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const files = await res.files(); @@ -77,7 +88,7 @@ describe('ZipArchiveResponse', () => { }); it('should read files with filter', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse( stream, @@ -100,7 +111,7 @@ describe('ZipArchiveResponse', () => { }); it('should read as archive and files', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const buffer = await res.archive(); @@ -132,7 +143,7 @@ describe('ZipArchiveResponse', () => { }); it('should extract entire archive into directory', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const dir = await res.dir(); @@ -146,7 +157,7 @@ describe('ZipArchiveResponse', () => { }); it('should extract archive into directory with a subpath', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, 'docs/', targetDir.path, 'etag'); @@ -157,7 +168,7 @@ describe('ZipArchiveResponse', () => { }); it('should extract archive into directory with a subpath and filter', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse( stream, @@ -205,7 +216,7 @@ describe('ZipArchiveResponse', () => { archive.finalize(); }); - const stream = fs.createReadStream(filePath); + const stream = createReadStream(filePath); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); @@ -225,7 +236,7 @@ describe('ZipArchiveResponse', () => { }); it('should throw on invalid archive', async () => { - const stream = fs.createReadStream( + const stream = createReadStream( sourceDir.resolve('test-archive-corrupted.zip'), ); @@ -238,7 +249,7 @@ describe('ZipArchiveResponse', () => { }); it('should throw on entries with a path outside the destination dir', async () => { - const stream = fs.createReadStream( + const stream = createReadStream( sourceDir.resolve('test-archive-malicious.zip'), ); @@ -249,7 +260,7 @@ describe('ZipArchiveResponse', () => { }); it('should throw on entries that attempt to write outside destination dir', async () => { - const stream = fs.createReadStream( + const stream = createReadStream( sourceDir.resolve('test-archive-malicious.zip'), );