From d586b84d8b117ca745e714e3eb3300ae16f2634b Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 22:36:14 +0200 Subject: [PATCH 01/10] MediaE2E: Add app.close() to afterAll This terminates the app after all test have finished. Signed-off-by: Philip Molares --- test/private-api/media.e2e-spec.ts | 3 ++- test/public-api/media.e2e-spec.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/private-api/media.e2e-spec.ts b/test/private-api/media.e2e-spec.ts index 2fc5f7c9a..5920ab3e5 100644 --- a/test/private-api/media.e2e-spec.ts +++ b/test/private-api/media.e2e-spec.ts @@ -133,6 +133,7 @@ describe('Media', () => { afterAll(async () => { // Delete the upload folder - await fs.rmdir(uploadPath); + await fs.rmdir(uploadPath, { recursive: true }); + await app.close(); }); }); diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index 97d35eb86..78a95b56f 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/test/public-api/media.e2e-spec.ts @@ -142,6 +142,7 @@ describe('Media', () => { afterAll(async () => { // Delete the upload folder - await fs.rmdir(uploadPath); + await fs.rmdir(uploadPath, { recursive: true }); + await app.close(); }); }); From 5a3ddc28fc7341e55362a413d1901a1771c4a4e8 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 22:41:38 +0200 Subject: [PATCH 02/10] MediaE2E: Extract 'uploads' deletion The deletion of upload was moved to beforeEach and afterEach block in the 'POST /media' > 'fails' tests. The test if the folder was not created, because there was no file uploaded, now correctly expects the behaviour. Signed-off-by: Philip Molares --- test/private-api/media.e2e-spec.ts | 12 ++++++++---- test/public-api/media.e2e-spec.ts | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/test/private-api/media.e2e-spec.ts b/test/private-api/media.e2e-spec.ts index 5920ab3e5..7270661aa 100644 --- a/test/private-api/media.e2e-spec.ts +++ b/test/private-api/media.e2e-spec.ts @@ -99,13 +99,16 @@ describe('Media', () => { await fs.unlink(join(uploadPath, fileName)); }); describe('fails:', () => { + beforeEach(async () => { + await fs.rmdir(uploadPath, { recursive: true }); + }); it('MIME type not supported', async () => { await request(app.getHttpServer()) .post('/media') .attach('file', 'test/private-api/fixtures/test.zip') .set('HedgeDoc-Note', 'test_upload_media') .expect(400); - expect(await fs.access(uploadPath)).toBeFalsy(); + await expect(fs.access(uploadPath)).rejects.toBeDefined(); }); it('note does not exist', async () => { await request(app.getHttpServer()) @@ -113,10 +116,9 @@ describe('Media', () => { .attach('file', 'test/private-api/fixtures/test.zip') .set('HedgeDoc-Note', 'i_dont_exist') .expect(400); - expect(await fs.access(uploadPath)).toBeFalsy(); + await expect(fs.access(uploadPath)).rejects.toBeDefined(); }); it('mediaBackend error', async () => { - await fs.rmdir(uploadPath); await fs.mkdir(uploadPath, { mode: '444', }); @@ -126,7 +128,9 @@ describe('Media', () => { .set('HedgeDoc-Note', 'test_upload_media') .expect('Content-Type', /json/) .expect(500); - await fs.rmdir(uploadPath); + }); + afterEach(async () => { + await fs.rmdir(uploadPath, { recursive: true }); }); }); }); diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index 78a95b56f..d8c3f3b63 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/test/public-api/media.e2e-spec.ts @@ -95,13 +95,16 @@ describe('Media', () => { await fs.unlink(join(uploadPath, fileName)); }); describe('fails:', () => { + beforeEach(async () => { + await fs.rmdir(uploadPath, { recursive: true }); + }); it('MIME type not supported', async () => { await request(app.getHttpServer()) .post('/media') .attach('file', 'test/public-api/fixtures/test.zip') .set('HedgeDoc-Note', 'test_upload_media') .expect(400); - expect(await fs.access(uploadPath)).toBeFalsy(); + await expect(fs.access(uploadPath)).rejects.toBeDefined(); }); it('note does not exist', async () => { await request(app.getHttpServer()) @@ -109,10 +112,9 @@ describe('Media', () => { .attach('file', 'test/public-api/fixtures/test.zip') .set('HedgeDoc-Note', 'i_dont_exist') .expect(400); - expect(await fs.access(uploadPath)).toBeFalsy(); + await expect(fs.access(uploadPath)).rejects.toBeDefined(); }); it('mediaBackend error', async () => { - await fs.rmdir(uploadPath); await fs.mkdir(uploadPath, { mode: '444', }); @@ -122,7 +124,9 @@ describe('Media', () => { .set('HedgeDoc-Note', 'test_upload_media') .expect('Content-Type', /json/) .expect(500); - await fs.rmdir(uploadPath); + }); + afterEach(async () => { + await fs.rmdir(uploadPath, { recursive: true }); }); }); }); From a2e7616484698c0c9226c26bd8c7d79fa520fe0e Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 22:42:56 +0200 Subject: [PATCH 03/10] E2ETests: Use recursive for 'uploads/' deletion. This ensures the folder is always deleted, even if there are still files (from previous broken tests) in it. Signed-off-by: Philip Molares --- test/private-api/notes.e2e-spec.ts | 2 +- test/public-api/me.e2e-spec.ts | 2 +- test/public-api/notes.e2e-spec.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/private-api/notes.e2e-spec.ts b/test/private-api/notes.e2e-spec.ts index 7e5806755..1e68c0d30 100644 --- a/test/private-api/notes.e2e-spec.ts +++ b/test/private-api/notes.e2e-spec.ts @@ -256,7 +256,7 @@ describe('Notes', () => { // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); } - await fs.rmdir(uploadPath); + await fs.rmdir(uploadPath, { recursive: true }); }); it('fails, when note does not exist', async () => { await request(app.getHttpServer()) diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index 5b13f4b52..76a3161c9 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -270,7 +270,7 @@ describe('Me', () => { // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); } - await fs.rmdir(uploadPath); + await fs.rmdir(uploadPath, { recursive: true }); }); afterAll(async () => { diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index b86d187fd..b7f2e2e2c 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -362,7 +362,7 @@ describe('Notes', () => { // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); } - await fs.rmdir(uploadPath); + await fs.rmdir(uploadPath, { recursive: true }); }); it('fails, when note does not exist', async () => { await request(app.getHttpServer()) From dbbc73be022543c41807a5fe01726e8b45d86f38 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 22:43:13 +0200 Subject: [PATCH 04/10] MediaE2E: Fix typo in comments Signed-off-by: Philip Molares --- test/private-api/media.e2e-spec.ts | 2 +- test/public-api/media.e2e-spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/private-api/media.e2e-spec.ts b/test/private-api/media.e2e-spec.ts index 7270661aa..c6c6e17ce 100644 --- a/test/private-api/media.e2e-spec.ts +++ b/test/private-api/media.e2e-spec.ts @@ -93,7 +93,7 @@ describe('Media', () => { const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const downloadResponse = await request(app.getHttpServer()).get(path); expect(downloadResponse.body).toEqual(testImage); - // Remove /upload/ from path as we just need the filename. + // Remove /uploads/ from path as we just need the filename. const fileName = path.replace('/uploads/', ''); // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index d8c3f3b63..9ae4379d8 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/test/public-api/media.e2e-spec.ts @@ -89,7 +89,7 @@ describe('Media', () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const downloadResponse = await request(app.getHttpServer()).get(path); expect(downloadResponse.body).toEqual(testImage); - // Remove /upload/ from path as we just need the filename. + // Remove /uploads/ from path as we just need the filename. const fileName = path.replace('/uploads/', ''); // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); From 70e731818054114b7c193383253db3e0d9e3ef97 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 22:44:46 +0200 Subject: [PATCH 05/10] NotesE2EPrivate: Fix copy&paste error Since large parts of this test were copied from the public api e2e test, somethings still used the public api e2e test files. Signed-off-by: Philip Molares --- test/private-api/notes.e2e-spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/private-api/notes.e2e-spec.ts b/test/private-api/notes.e2e-spec.ts index 1e68c0d30..0b6b8f6f0 100644 --- a/test/private-api/notes.e2e-spec.ts +++ b/test/private-api/notes.e2e-spec.ts @@ -63,7 +63,7 @@ describe('Notes', () => { GroupsModule, TypeOrmModule.forRoot({ type: 'sqlite', - database: './hedgedoc-e2e-notes.sqlite', + database: './hedgedoc-e2e-private-notes.sqlite', autoLoadEntities: true, synchronize: true, dropSchema: true, @@ -236,7 +236,7 @@ describe('Notes', () => { .expect(200); expect(response.body).toHaveLength(0); - const testImage = await fs.readFile('test/public-api/fixtures/test.png'); + const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const url0 = await mediaService.saveFile(testImage, 'hardcoded', note.id); const url1 = await mediaService.saveFile( testImage, From 19d88e7e8ccb135910ce454ddcf9bc8024499b98 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 23:23:15 +0200 Subject: [PATCH 06/10] PackageJson: Add --runInBand to test:e2e scripts This ensures the e2e tests run in serially and never concurrently. See https://jestjs.io/docs/cli#--runinband Signed-off-by: Philip Molares --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index aad27825a..df8b0ca26 100644 --- a/package.json +++ b/package.json @@ -20,8 +20,8 @@ "test:watch": "jest --watch", "test:cov": "jest --coverage", "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand", - "test:e2e": "jest --config jest-e2e.json", - "test:e2e:cov": "jest --config jest-e2e.json --coverage" + "test:e2e": "jest --config jest-e2e.json --runInBand", + "test:e2e:cov": "jest --config jest-e2e.json --coverage --runInBand" }, "dependencies": { "@azure/storage-blob": "12.5.0", From 2d39160d9ebd75ffb4de72fbb2163f2c137751cb Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 23:26:38 +0200 Subject: [PATCH 07/10] E2EConfig: Remove maxConcurrency This is not necessary as it only effects `it.concurrent` test and we currently don't use those. See https://jestjs.io/docs/configuration#maxconcurrency-number Signed-off-by: Philip Molares --- jest-e2e.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jest-e2e.json b/jest-e2e.json index 20f0c2312..7594b2068 100644 --- a/jest-e2e.json +++ b/jest-e2e.json @@ -13,6 +13,5 @@ "^.+\\.(t|j)s$": "ts-jest" }, "coverageDirectory": "./coverage-e2e", - "testTimeout": 10000, - "maxConcurrency": 1 + "testTimeout": 10000 } From 354db0c1a2f6d2c3889974e75ed078691979872d Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 22:58:15 +0200 Subject: [PATCH 08/10] MediaConfigMock: Change upload path This changes the upload path in all test to 'test_uploads' to ensure no real uploads are lost. Signed-off-by: Philip Molares --- src/config/mock/media.config.mock.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/mock/media.config.mock.ts b/src/config/mock/media.config.mock.ts index 3f729d34c..175253e33 100644 --- a/src/config/mock/media.config.mock.ts +++ b/src/config/mock/media.config.mock.ts @@ -10,7 +10,7 @@ export default registerAs('mediaConfig', () => ({ backend: { use: 'filesystem', filesystem: { - uploadPath: 'uploads', + uploadPath: 'test_uploads', }, }, })); From c821fe6f040b8e6a38d560de03cb953601ee78de Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 22:59:47 +0200 Subject: [PATCH 09/10] Git: Ignore test_uploads folder Ignore the uploads folder for tests. Signed-off-by: Philip Molares --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f8aa626e6..99a0704cb 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,4 @@ dist public/uploads/* !public/uploads/.gitkeep uploads +test_uploads From 4fb76232250a121ec65f29b6f0d28161912d0321 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 31 Mar 2021 23:01:29 +0200 Subject: [PATCH 10/10] FileMediaBackend: Fix generated urls All urls should be of the form `uploads/.` regardless of what the uploadDirectory is, because the backend proxies all locally uploaded files. Signed-off-by: Philip Molares --- src/media/backends/filesystem-backend.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/media/backends/filesystem-backend.ts b/src/media/backends/filesystem-backend.ts index cf9ce7cce..42924d288 100644 --- a/src/media/backends/filesystem-backend.ts +++ b/src/media/backends/filesystem-backend.ts @@ -36,7 +36,7 @@ export class FilesystemBackend implements MediaBackend { await this.ensureDirectory(); try { await fs.writeFile(filePath, buffer, null); - return ['/' + filePath, null]; + return ['/uploads/' + fileName, null]; } catch (e) { this.logger.error((e as Error).message, (e as Error).stack, 'saveFile'); throw new MediaBackendError(`Could not save '${filePath}'`);