From fc5984b4863802b01bd84bc7b95d339700bb9642 Mon Sep 17 00:00:00 2001 From: Naz Date: Thu, 25 Nov 2021 21:06:40 +0400 Subject: [PATCH] Slimmed down redirects test suites refs https://github.com/TryGhost/Toolbox/issues/139 - The regression test suite for redirects functionality for way too big. And each restart was causing massive overhead. It's enough to have a single exhaustive test using multiple input files - The tests testing API endpoints should've been e2e tests to start with - The rest is covered in the unit tests for redirects api service --- test/e2e-api/admin/redirects.test.js | 40 ++++ .../api/canary/admin/redirects.test.js | 175 ------------------ .../server/services/redirects/api.test.js | 25 ++- 3 files changed, 55 insertions(+), 185 deletions(-) create mode 100644 test/e2e-api/admin/redirects.test.js diff --git a/test/e2e-api/admin/redirects.test.js b/test/e2e-api/admin/redirects.test.js new file mode 100644 index 0000000000..e9748a4459 --- /dev/null +++ b/test/e2e-api/admin/redirects.test.js @@ -0,0 +1,40 @@ +const should = require('should'); +const supertest = require('supertest'); +const fs = require('fs-extra'); +const path = require('path'); +const localUtils = require('./utils'); +const config = require('../../../core/shared/config'); + +describe('Redirects API', function () { + let request; + + before(async function () { + await localUtils.startGhost(); + request = supertest.agent(config.get('url')); + await localUtils.doAuth(request, 'users:extra', 'posts'); + }); + + it('download', function () { + return request + .get(localUtils.API.getApiQuery('redirects/download/')) + .set('Origin', config.get('url')) + .expect('Content-Type', /application\/json/) + .expect('Content-Disposition', 'Attachment; filename="redirects.json"') + .expect(200); + }); + + it('upload', function () { + // Provide a redirects file in the root directory of the content test folder + fs.writeFileSync(path.join(config.get('paths:contentPath'), 'redirects-init.json'), JSON.stringify([{ + from: 'k', + to: 'l' + }])); + + return request + .post(localUtils.API.getApiQuery('redirects/upload/')) + .set('Origin', config.get('url')) + .attach('redirects', path.join(config.get('paths:contentPath'), 'redirects-init.json')) + .expect('Content-Type', /application\/json/) + .expect(200); + }); +}); diff --git a/test/regression/api/canary/admin/redirects.test.js b/test/regression/api/canary/admin/redirects.test.js index 6d94817ddb..71e324c72d 100644 --- a/test/regression/api/canary/admin/redirects.test.js +++ b/test/regression/api/canary/admin/redirects.test.js @@ -1,7 +1,6 @@ const should = require('should'); const supertest = require('supertest'); const fs = require('fs-extra'); -const Promise = require('bluebird'); const path = require('path'); const os = require('os'); const uuid = require('uuid'); @@ -74,109 +73,6 @@ describe('Redirects API', function () { from: 'k', to: 'l' }])); - }); - }); - - it('override', function () { - return startGhost({ - frontend: true, - forceStart: true - }) - .then(() => { - return request - .get('/my-old-blog-post/') - .expect(301); - }) - .then((response) => { - response.headers.location.should.eql('/revamped-url/'); - }) - .then(() => { - // Provide a second redirects file in the root directory of the content test folder - fs.writeFileSync(path.join(config.get('paths:contentPath'), 'redirects.json'), JSON.stringify([{ - from: 'c', - to: 'd' - }])); - }) - .then(() => { - // Override redirects file - return request - .post(localUtils.API.getApiQuery('redirects/upload/')) - .set('Origin', config.get('url')) - .attach('redirects', path.join(config.get('paths:contentPath'), 'redirects.json')) - .expect('Content-Type', /application\/json/) - .expect(200); - }) - .then((res) => { - res.headers['x-cache-invalidate'].should.eql('/*'); - - return request - .get('/my-old-blog-post/') - .expect(404); - }) - .then(() => { - return request - .get('/c/') - .expect(302); - }) - .then((response) => { - response.headers.location.should.eql('/d'); - - // check backup of redirects files - const dataFiles = fs.readdirSync(config.getContentPath('data')); - dataFiles.join(',').match(/(redirects)/g).length.should.eql(2); - - // Provide another redirects file in the root directory of the content test folder - fs.writeFileSync(path.join(config.get('paths:contentPath'), 'redirects-something.json'), JSON.stringify([{ - from: 'e', - to: 'b' - }])); - }) - .then(() => { - // the backup is in the format HH:mm:ss, we have to wait minimum a second - return new Promise((resolve) => { - setTimeout(resolve, 1100); - }); - }) - .then(() => { - // Override redirects file again and ensure the backup file works twice - return request - .post(localUtils.API.getApiQuery('redirects/upload/')) - .set('Origin', config.get('url')) - .attach('redirects', path.join(config.get('paths:contentPath'), 'redirects-something.json')) - .expect('Content-Type', /application\/json/) - .expect(200); - }) - .then(() => { - const dataFiles = fs.readdirSync(config.getContentPath('data')); - dataFiles.join(',').match(/(redirects)/g).length.should.eql(3); - - const fileContent = fs.readFileSync(path.join(config.get('paths:contentPath'), 'data', 'redirects.json'), 'utf-8'); - fileContent.should.eql(JSON.stringify([{ - from: 'e', - to: 'b' - }])); - }); - }); - }); - }); - - describe('Upload yaml', function () { - describe('Ensure re-registering redirects works', function () { - it('override', function () { - // We want to test if we can override old redirects.json with new redirects.yaml - // That's why we start with .json. - return startGhost({ - frontend: true, - forceStart: true, - redirectsFileExt: '.json' - }) - .then(() => { - return request - .get('/my-old-blog-post/') - .expect(301); - }) - .then((response) => { - response.headers.location.should.eql('/revamped-url/'); }) .then(() => { // Provide a second redirects file in the root directory of the content test folder @@ -206,81 +102,10 @@ describe('Redirects API', function () { .then((response) => { response.headers.location.should.eql('/d'); - // check backup of redirects files - const dataFiles = fs.readdirSync(config.getContentPath('data')); - dataFiles.join(',').match(/(redirects)/g).length.should.eql(2); - const fileContent = fs.readFileSync(path.join(config.get('paths:contentPath'), 'data', 'redirects.yaml'), 'utf-8'); fileContent.should.eql('302:\n c: d'); - - // Provide another redirects file in the root directory of the content test folder - fs.writeFileSync(path.join(config.get('paths:contentPath'), 'redirects-something.json'), JSON.stringify([{ - from: 'e', - to: 'b' - }])); - }) - .then(() => { - // the backup is in the format HH:mm:ss, we have to wait minimum a second - return new Promise((resolve) => { - setTimeout(resolve, 1100); - }); - }) - .then(() => { - // Override redirects file again and ensure the backup file works twice - return request - .post(localUtils.API.getApiQuery('redirects/upload/')) - .set('Origin', config.get('url')) - .attach('redirects', path.join(config.get('paths:contentPath'), 'redirects-something.json')) - .expect('Content-Type', /application\/json/) - .expect(200); - }) - .then(() => { - return request - .get('/e/') - .expect(302); - }) - .then((response) => { - response.headers.location.should.eql('/b'); - - const dataFiles = fs.readdirSync(config.getContentPath('data')); - dataFiles.join(',').match(/(redirects)/g).length.should.eql(3); - - const fileContent = fs.readFileSync(path.join(config.get('paths:contentPath'), 'data', 'redirects.json'), 'utf-8'); - fileContent.should.eql(JSON.stringify([{ - from: 'e', - to: 'b' - }])); }); }); }); }); - - // TODO: For backward compatibility, we only check if download, upload endpoints work here. - // when updating to v4, the old endpoints should be updated to the new ones. - // And the tests below should be removed. - describe('New endpoints work', function () { - it('download', function () { - return request - .get(localUtils.API.getApiQuery('redirects/download/')) - .set('Origin', config.get('url')) - .expect('Content-Type', /application\/json/) - .expect('Content-Disposition', 'Attachment; filename="redirects.json"') - .expect(200); - }); - - it('upload', function () { - // Provide a redirects file in the root directory of the content test folder - fs.writeFileSync(path.join(config.get('paths:contentPath'), 'redirects-init.json'), JSON.stringify([{ - from: 'k', - to: 'l' - }])); - - return request - .post(localUtils.API.getApiQuery('redirects/upload/')) - .set('Origin', config.get('url')) - .attach('redirects', path.join(config.get('paths:contentPath'), 'redirects-init.json')) - .expect('Content-Type', /application\/json/) - .expect(200); - }); - }); }); diff --git a/test/unit/server/services/redirects/api.test.js b/test/unit/server/services/redirects/api.test.js index 84aea8413d..ef0d75a983 100644 --- a/test/unit/server/services/redirects/api.test.js +++ b/test/unit/server/services/redirects/api.test.js @@ -12,11 +12,6 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () { const basePath = path.join(__dirname, '../../../../utils/fixtures/data/'); before(function () { - redirectManager = { - removeAllRedirects: sinon.stub(), - addRedirect: sinon.stub() - }; - customRedirectsAPI = new CustomRedirectsAPI({ basePath, redirectManager, @@ -26,6 +21,11 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () { }); beforeEach(function () { + redirectManager = { + removeAllRedirects: sinon.stub(), + addRedirect: sinon.stub() + }; + sinon.stub(fs, 'pathExists'); sinon.stub(fs, 'writeFile'); sinon.stub(fs, 'readFile'); @@ -41,11 +41,6 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () { describe('init', function () { it('initializes without errors when redirects file is not present', async function () { - redirectManager = { - removeAllRedirects: sinon.stub(), - addRedirect: sinon.stub() - }; - customRedirectsAPI = new CustomRedirectsAPI({ basePath, redirectManager, @@ -183,6 +178,11 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () { // written new routes file fs.writeFile.calledWith(existingRedirectsFilePath, redirectsJSONConfig, 'utf-8').should.be.true(); + + // redirects have been re-registered + redirectManager.removeAllRedirects.calledOnce.should.be.true(); + // one redirect in total + redirectManager.addRedirect.calledOnce.should.be.true(); }); it('creates a backup file from existing redirects.yaml file', async function () { @@ -226,6 +226,11 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () { // overwritten with incoming routes.yaml file fs.copy.calledWith(incomingFilePath, `${basePath}redirects.yaml`).should.be.true(); + + // redirects have been re-registered + redirectManager.removeAllRedirects.calledOnce.should.be.true(); + // two redirects in total + redirectManager.addRedirect.calledTwice.should.be.true(); }); }); });