From 4d758e415fc6ec99efc14253709ed5cab45a126f Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 9 Jul 2019 13:08:11 -0400 Subject: [PATCH 1/9] test: Improved unit tests for pagination checks --- src/infrastructure/RequestHelper.ts | 1 + test/unit/infrastructure/RequestHelper.ts | 178 +++++++++------------- 2 files changed, 70 insertions(+), 109 deletions(-) diff --git a/src/infrastructure/RequestHelper.ts b/src/infrastructure/RequestHelper.ts index bc640f89..a9b7f7bc 100644 --- a/src/infrastructure/RequestHelper.ts +++ b/src/infrastructure/RequestHelper.ts @@ -21,6 +21,7 @@ export async function get( query: query || {}, sudo, }); + const { headers } = response; let { body } = response; let pagination = { diff --git a/test/unit/infrastructure/RequestHelper.ts b/test/unit/infrastructure/RequestHelper.ts index 50522c0c..a3da61fe 100644 --- a/test/unit/infrastructure/RequestHelper.ts +++ b/test/unit/infrastructure/RequestHelper.ts @@ -11,112 +11,59 @@ const mockedGetBasic = () => ({ }, }); -const mockedGetExtended = (url, { query }) => { - const pages = [ - { - body: [ - { - prop1: 1, - prop2: 'test property1', - }, - { - prop1: 2, - prop2: 'test property2', - }, - ], - headers: { - link: `; rel="next", - ; rel="first", - ; rel="last"`, - 'x-next-page': 2, - 'x-page': 1, - 'x-per-page': 2, - 'x-prev-page': '', - 'x-total': 8, - 'x-total-pages': 4, - }, - }, - { - body: [ - { - prop1: 3, - prop2: 'test property3', - }, - { - prop1: 4, - prop2: 'test property4', - }, - ], - headers: { - link: `; rel="prev", - ; rel="next", - ; rel="first", - ; rel="last"`, - 'x-next-page': 3, - 'x-page': 2, - 'x-per-page': 2, - 'x-prev-page': 1, - 'x-total': 8, - 'x-total-pages': 4, - }, - }, - { - body: [ - { - prop1: 5, - prop2: 'test property5', - }, - { - prop1: 6, - prop2: 'test property6', - }, - ], - headers: { - link: `; rel="prev", - ; rel="next", - ; rel="first", - ; rel="last"`, - 'x-next-page': 4, - 'x-page': 3, - 'x-per-page': 2, - 'x-prev-page': 2, - 'x-total': 8, - 'x-total-pages': 4, - }, - }, - { - body: [ - { - prop1: 7, - prop2: 'test property7', - }, - { - prop1: 8, - prop2: 'test property8', - }, - ], - headers: { - link: `; rel="prev", - ; rel="first", - ; rel="last"`, - 'x-next-page': '', - 'x-page': 4, - 'x-per-page': 2, - 'x-prev-page': 3, - 'x-total': 8, - 'x-total-pages': 4, - }, - }, - ]; - +const mockedGetExtended = (url, { query }, limit=30) => { + const pages: object[] = []; const q = url.match(/page=([0-9]+)/); let page = 1; if (q != null) page = q[1]; else if (query.page) page = query.page; + // Only load pages needed for the test + // TODO: modify this to only generate the required response, without the loop + for (let i = 1, a = 1, b = 2; i <= limit && i <= page; i++, a+=2, b+=2) { + let next = ''; + let prev = ''; + let nextPage; + let prevPage; + + if ((i+1) <= limit ) { + next = `; rel="next",`; + nextPage = i+1 + } + + if ((i-1) >= 1) { + prev = `; rel="prev",`; + prevPage = i-1 + } + + pages.push({ + body: [ + { + prop1: a, + prop2: `test property ${a}`, + }, + { + prop1: b, + prop2: `test property ${b}`, + }, + ], + headers: { + link: next + prev + ` + ; rel="first", + ; rel="last"`, + 'x-next-page': nextPage, + 'x-page': i, + 'x-per-page': 2, + 'x-prev-page': prevPage, + 'x-total': limit * 2, + 'x-total-pages': limit, + }, + }); + } + return pages[page - 1]; -}; +} const service = new BaseService({ host: 'https://testing.com', @@ -151,10 +98,23 @@ describe('RequestHelper.get()', () => { response.forEach((l, index) => { expect(l.prop1).toBe(1 + index); - expect(l.prop2).toBe(`test property${1 + index}`); + expect(l.prop2).toBe(`test property ${1 + index}`); }); - expect(response).toHaveLength(8); + expect(response).toHaveLength(60); + }); + + it('Should handle large paginated (50 pages) results when links are present', async () => { + KyRequester.get = jest.fn(({}, url, options) => mockedGetExtended(url, options, 70)); + + const response = await RequestHelper.get(service, 'test'); + + response.forEach((l, index) => { + expect(l.prop1).toBe(1 + index); + expect(l.prop2).toBe(`test property ${1 + index}`); + }); + + expect(response).toHaveLength(140); }); it('Should be paginated but limited by the maxPages option', async () => { @@ -166,7 +126,7 @@ describe('RequestHelper.get()', () => { response.forEach((l, index) => { expect(l.prop1).toBe(1 + index); - expect(l.prop2).toBe(`test property${1 + index}`); + expect(l.prop2).toBe(`test property ${1 + index}`); }); }); @@ -179,7 +139,7 @@ describe('RequestHelper.get()', () => { response.forEach((l, index) => { expect(l.prop1).toBe(3 + index); - expect(l.prop2).toBe(`test property${3 + index}`); + expect(l.prop2).toBe(`test property ${3 + index}`); }); }); @@ -192,16 +152,16 @@ describe('RequestHelper.get()', () => { response.data.forEach((l, index) => { expect(l.prop1).toBe(3 + index); - expect(l.prop2).toBe(`test property${3 + index}`); + expect(l.prop2).toBe(`test property ${3 + index}`); }); expect(response.pagination).toMatchObject({ - total: 8, + total: 60, previous: 1, current: 2, next: 3, perPage: 2, - totalPages: 4, + totalPages: 30, }); }); @@ -217,16 +177,16 @@ describe('RequestHelper.get()', () => { response.data.forEach((l, index) => { expect(l.prop1).toBe(1 + index); - expect(l.prop2).toBe(`test property${1 + index}`); + expect(l.prop2).toBe(`test property ${1 + index}`); }); expect(response.pagination).toMatchObject({ - total: 8, + total: 60, previous: 2, current: 3, next: 4, perPage: 2, - totalPages: 4, + totalPages: 30, }); }); }); From c9a681d402e98684a12219cf60d8c0d7c961725e Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 9 Jul 2019 14:23:42 -0400 Subject: [PATCH 2/9] test: Adding basic Labels API test --- test/integration/services/Issues.ts | 6 ++-- test/integration/services/Labels.ts | 56 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 test/integration/services/Labels.ts diff --git a/test/integration/services/Issues.ts b/test/integration/services/Issues.ts index 4a4f5045..9033937d 100644 --- a/test/integration/services/Issues.ts +++ b/test/integration/services/Issues.ts @@ -49,20 +49,20 @@ describe('Issues.all', () => { const issues = await service.all(); expect(issues).toBeInstanceOf(Array); - expect(issues.length).toEqual(2); + expect(issues).toHaveLength(2); }); it('should return a list filtered to a specfic page', async () => { const issues1 = await service.all({projectId: project.id, perPage: 1, page: 1 }); expect(issues1).toBeInstanceOf(Array); - expect(issues1.length).toEqual(1); + expect(issues1).toHaveLength(1); expect(issues1[0].title).toBe('Issue Integration test2'); const issues2 = await service.all({ projectId: project.id, perPage: 1, page: 2 }); expect(issues2).toBeInstanceOf(Array); - expect(issues2.length).toEqual(1); + expect(issues2).toHaveLength(1); expect(issues2[0].title).toBe('Issue Integration test1'); }); }); diff --git a/test/integration/services/Labels.ts b/test/integration/services/Labels.ts new file mode 100644 index 00000000..515497fa --- /dev/null +++ b/test/integration/services/Labels.ts @@ -0,0 +1,56 @@ +import { Labels, Projects } from '../../../dist'; + +const config = { + host: process.env.GITLAB_URL, + token: process.env.PERSONAL_ACCESS_TOKEN, +}; +let project; +let service: Labels; + +beforeAll(async () => { + // Crease project service + const projectService = new Projects(config); + + // Create issue service + service = new Labels(config); + + // Create a template project + project = await projectService.create({ name: 'Labels Integration test' }); +}); + +describe('Labels.create', () => { + it('should create a valid label on a project', async () => { + const label = await service.create(project.id, { name: 'Test Label' }); + + expect(label).toBeInstanceOf(Object); + expect(label.name).toBe('Test Label'); + }); +}); + +describe('Labels.show', () => { + it('should get a valid label on a project', async () => { + const labelCreated = await service.create(project.id, { name: 'Test Label2' }); + const labelShown = await service.show(project.id, labelCreated.id); + + expect(labelCreated).toMatchObject(labelShown); + }); +}); + +describe('Labels.remove', () => { + it('should remove/delete a valid label on a project', async () => { + const label = await service.create(project.id, { name: 'Test Label3' }); + + await service.remove(project.id, label.id); + + expect(service.show(project.id, label.id)).rejects.toThrow(); + }); +}); + +// describe('Labels.all', () => { +// it('should return a list of labels on a project', async () => { +// const labels = await service.all(project.id); + +// expect(labels).toBeInstanceOf(Array); +// expect(issues).toHaveLength(2); +// }); +// }); From 199e32d34e5cd1b5f8994342c552832e3d769eda Mon Sep 17 00:00:00 2001 From: Justin Date: Thu, 11 Jul 2019 17:17:00 -0400 Subject: [PATCH 3/9] refactor: Adding required labelName and colour option to the create method BREAKING CHANGE: Labels require a colour and a name to be created. Now the create method takes a second and third argument: 'labelName' and 'color' --- src/infrastructure/KyRequester.ts | 3 +++ src/services/Labels.ts | 4 ++-- test/integration/services/Labels.ts | 8 ++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/infrastructure/KyRequester.ts b/src/infrastructure/KyRequester.ts index d49c9e42..84dbb09b 100644 --- a/src/infrastructure/KyRequester.ts +++ b/src/infrastructure/KyRequester.ts @@ -70,6 +70,9 @@ methods.forEach(m => { e.description = output.error || output.message; } + console.log(requestOptions) + console.log(endpoint) + throw e; } diff --git a/src/services/Labels.ts b/src/services/Labels.ts index 269396de..7324c9d8 100644 --- a/src/services/Labels.ts +++ b/src/services/Labels.ts @@ -14,10 +14,10 @@ class Labels extends BaseService { return RequestHelper.get(this, `projects/${pId}/labels`, options); } - create(projectId: ProjectId, options?: BaseRequestOptions) { + create(projectId: ProjectId, labelName: string, color: string, options?: BaseRequestOptions) { const pId = encodeURIComponent(projectId); - return RequestHelper.post(this, `projects/${pId}/labels`, options); + return RequestHelper.post(this, `projects/${pId}/labels`, { name: labelName, color, ...options }); } edit(projectId: ProjectId, labelName: string, options?: BaseRequestOptions) { diff --git a/test/integration/services/Labels.ts b/test/integration/services/Labels.ts index 515497fa..212f72d3 100644 --- a/test/integration/services/Labels.ts +++ b/test/integration/services/Labels.ts @@ -20,7 +20,7 @@ beforeAll(async () => { describe('Labels.create', () => { it('should create a valid label on a project', async () => { - const label = await service.create(project.id, { name: 'Test Label' }); + const label = await service.create(project.id, 'Test Label1', '#FFAABB'); expect(label).toBeInstanceOf(Object); expect(label.name).toBe('Test Label'); @@ -29,7 +29,7 @@ describe('Labels.create', () => { describe('Labels.show', () => { it('should get a valid label on a project', async () => { - const labelCreated = await service.create(project.id, { name: 'Test Label2' }); + const labelCreated = await service.create(project.id, 'Test Label2', '#FFAABB'); const labelShown = await service.show(project.id, labelCreated.id); expect(labelCreated).toMatchObject(labelShown); @@ -38,7 +38,7 @@ describe('Labels.show', () => { describe('Labels.remove', () => { it('should remove/delete a valid label on a project', async () => { - const label = await service.create(project.id, { name: 'Test Label3' }); + const label = await service.create(project.id, 'Test Label3', '#FFAABB'); await service.remove(project.id, label.id); @@ -53,4 +53,4 @@ describe('Labels.remove', () => { // expect(labels).toBeInstanceOf(Array); // expect(issues).toHaveLength(2); // }); -// }); +// }); \ No newline at end of file From ae5cb1730a25b20120188bcbdc2db6eb3b324200 Mon Sep 17 00:00:00 2001 From: Justin Date: Thu, 11 Jul 2019 17:39:26 -0400 Subject: [PATCH 4/9] test fixes --- test/integration/services/Labels.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/test/integration/services/Labels.ts b/test/integration/services/Labels.ts index 212f72d3..56e96c81 100644 --- a/test/integration/services/Labels.ts +++ b/test/integration/services/Labels.ts @@ -23,16 +23,7 @@ describe('Labels.create', () => { const label = await service.create(project.id, 'Test Label1', '#FFAABB'); expect(label).toBeInstanceOf(Object); - expect(label.name).toBe('Test Label'); - }); -}); - -describe('Labels.show', () => { - it('should get a valid label on a project', async () => { - const labelCreated = await service.create(project.id, 'Test Label2', '#FFAABB'); - const labelShown = await service.show(project.id, labelCreated.id); - - expect(labelCreated).toMatchObject(labelShown); + expect(label.name).toBe('Test Label1'); }); }); @@ -40,9 +31,7 @@ describe('Labels.remove', () => { it('should remove/delete a valid label on a project', async () => { const label = await service.create(project.id, 'Test Label3', '#FFAABB'); - await service.remove(project.id, label.id); - - expect(service.show(project.id, label.id)).rejects.toThrow(); + expect(service.remove(project.id, label.name)).resolves.toBeTruthy(); }); }); From 19acfc69288f5cf3c6c361a7191e3cc8212f8a7b Mon Sep 17 00:00:00 2001 From: Justin Date: Fri, 12 Jul 2019 10:21:55 -0400 Subject: [PATCH 5/9] Adding all test --- test/integration/services/Labels.ts | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/test/integration/services/Labels.ts b/test/integration/services/Labels.ts index 56e96c81..43e8cdd7 100644 --- a/test/integration/services/Labels.ts +++ b/test/integration/services/Labels.ts @@ -31,15 +31,26 @@ describe('Labels.remove', () => { it('should remove/delete a valid label on a project', async () => { const label = await service.create(project.id, 'Test Label3', '#FFAABB'); - expect(service.remove(project.id, label.name)).resolves.toBeTruthy(); + expect(service.remove(project.id, label.name)).resolves.toEqual(""); }); }); -// describe('Labels.all', () => { -// it('should return a list of labels on a project', async () => { -// const labels = await service.all(project.id); +describe('Labels.all', () => { + beforeAll(async () => { + const labels = []; -// expect(labels).toBeInstanceOf(Array); -// expect(issues).toHaveLength(2); -// }); -// }); \ No newline at end of file + for (let i = 0; i < 50; i++) { + lables.push(service.create(project.id, `All Labels ${i}`, '#FFAABB')); + } + + return Promise.all(labels); + }); + + it('should return a list of labels on a project', async () => { + const labels = await service.all(project.id, { perPage: 3 }); + const filtered = labels.filter(l => l.name.includes('All Labels')); + + expect(labels).toBeInstanceOf(Array); + expect(filtered).toHaveLength(50); + }); +}); \ No newline at end of file From 6fea712a694439092a133ea08b26ff827c5bbf02 Mon Sep 17 00:00:00 2001 From: Justin Date: Fri, 12 Jul 2019 11:05:48 -0400 Subject: [PATCH 6/9] Typo and typing --- test/integration/services/Labels.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/services/Labels.ts b/test/integration/services/Labels.ts index 43e8cdd7..1ab69db8 100644 --- a/test/integration/services/Labels.ts +++ b/test/integration/services/Labels.ts @@ -37,10 +37,10 @@ describe('Labels.remove', () => { describe('Labels.all', () => { beforeAll(async () => { - const labels = []; + const labels: object[] = []; for (let i = 0; i < 50; i++) { - lables.push(service.create(project.id, `All Labels ${i}`, '#FFAABB')); + labels.push(service.create(project.id, `All Labels ${i}`, '#FFAABB')); } return Promise.all(labels); From e68bdabd8a8528281afc8bb339e0a023850aa7e8 Mon Sep 17 00:00:00 2001 From: Justin Date: Fri, 12 Jul 2019 12:12:00 -0400 Subject: [PATCH 7/9] Add pagination object check --- test/integration/services/Labels.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/integration/services/Labels.ts b/test/integration/services/Labels.ts index 1ab69db8..0463b415 100644 --- a/test/integration/services/Labels.ts +++ b/test/integration/services/Labels.ts @@ -53,4 +53,26 @@ describe('Labels.all', () => { expect(labels).toBeInstanceOf(Array); expect(filtered).toHaveLength(50); }); + + it('should return a list of labels on a project restricted to page 5', async () => { + const labels = await service.all(project.id, { perPage: 5, page: 5}); + + expect(labels).toBeInstanceOf(Array); + expect(labels).toHaveLength(5); + }); + + it('should return a list of labels on a project restricted to page 5 and show the pagination object', async () => { + const { data, pagination } = await service.all(project.id, { perPage: 5, page: 5, showPagination: true}); + + expect(data).toBeInstanceOf(Array); + expect(data).toHaveLength(5); + expect(pagination).toMatchObject({ + total: 51, // TODO: change this to not depend on previous data + previous: 4, + current: 5, + next: 6, + perPage: 5, + totalPages: 11, + }); + }); }); \ No newline at end of file From 84f619ff33df8d7d18b3384ec2f870f8119ab318 Mon Sep 17 00:00:00 2001 From: Justin Date: Fri, 12 Jul 2019 12:31:13 -0400 Subject: [PATCH 8/9] Numbers should be ints not strings --- src/infrastructure/RequestHelper.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/infrastructure/RequestHelper.ts b/src/infrastructure/RequestHelper.ts index a9b7f7bc..5e7c9e04 100644 --- a/src/infrastructure/RequestHelper.ts +++ b/src/infrastructure/RequestHelper.ts @@ -25,12 +25,12 @@ export async function get( const { headers } = response; let { body } = response; let pagination = { - total: headers['x-total'], + total: parseInt(headers['x-total'], 10), next: parseInt(headers['x-next-page'], 10) || null, current: parseInt(headers['x-page'], 10) || 1, - previous: headers['x-prev-page'] || null, - perPage: headers['x-per-page'], - totalPages: headers['x-total-pages'], + previous: parseInt(headers['x-prev-page'], 10) || null, + perPage: parseInt(headers['x-per-page'], 10), + totalPages: parseInt(headers['x-total-pages'], 10), }; const underLimit = maxPages ? pagination.current < maxPages : true; From d46efc55a49d2821ffff22da0943c20db9ba979a Mon Sep 17 00:00:00 2001 From: Justin Date: Mon, 15 Jul 2019 08:47:02 -0400 Subject: [PATCH 9/9] Remove console logs --- src/infrastructure/KyRequester.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/infrastructure/KyRequester.ts b/src/infrastructure/KyRequester.ts index 84dbb09b..93ab50c5 100644 --- a/src/infrastructure/KyRequester.ts +++ b/src/infrastructure/KyRequester.ts @@ -69,9 +69,6 @@ methods.forEach(m => { e.description = output.error || output.message; } - - console.log(requestOptions) - console.log(endpoint) throw e; }