From 91d6dd6643a2d921f13eb291878ea5ef670e5847 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Mon, 15 Apr 2019 23:47:25 -0400 Subject: [PATCH] Rewrite [codeclimate] coverage (#3316) Attacking this in two pieces for ease of review. The legacy implementation for coverage is still there, though I disabled it via the route. That whole file will be removed in the next PR. Ref #2863 --- core/base-service/redirector.js | 12 ++- core/base-service/redirector.spec.js | 9 ++ services/codeclimate/codeclimate-common.js | 48 ++++++++++ ...codeclimate-coverage-redirector.service.js | 27 ++++++ .../codeclimate-coverage-redirector.tester.js | 30 ++++++ .../codeclimate-coverage.service.js | 94 +++++++++++++++++++ .../codeclimate-coverage.tester.js | 32 ++----- services/codeclimate/codeclimate.service.js | 43 +-------- 8 files changed, 224 insertions(+), 71 deletions(-) create mode 100644 services/codeclimate/codeclimate-common.js create mode 100644 services/codeclimate/codeclimate-coverage-redirector.service.js create mode 100644 services/codeclimate/codeclimate-coverage-redirector.tester.js create mode 100644 services/codeclimate/codeclimate-coverage.service.js diff --git a/core/base-service/redirector.js b/core/base-service/redirector.js index e8876f6995..e738ce8352 100644 --- a/core/base-service/redirector.js +++ b/core/base-service/redirector.js @@ -14,6 +14,7 @@ const { isValidRoute, prepareRoute, namedParamsForMatch } = require('./route') const trace = require('./trace') const attrSchema = Joi.object({ + name: Joi.string().min(3), category: isValidCategory, route: isValidRoute, transformPath: Joi.func() @@ -30,6 +31,7 @@ const attrSchema = Joi.object({ module.exports = function redirector(attrs) { const { + name, category, route, transformPath, @@ -51,9 +53,13 @@ module.exports = function redirector(attrs) { } static get name() { - return `${camelcase(route.base.replace(/\//g, '_'), { - pascalCase: true, - })}Redirect` + if (name) { + return name + } else { + return `${camelcase(route.base.replace(/\//g, '_'), { + pascalCase: true, + })}Redirect` + } } static register({ camp, requestCounter }) { diff --git a/core/base-service/redirector.spec.js b/core/base-service/redirector.spec.js index 525c0734cd..3bc19e9604 100644 --- a/core/base-service/redirector.spec.js +++ b/core/base-service/redirector.spec.js @@ -24,6 +24,15 @@ describe('Redirector', function() { expect(redirector(attrs).name).to.equal('VeryOldServiceRedirect') }) + it('overrides the name', function() { + expect( + redirector({ + ...attrs, + name: 'ShinyRedirect', + }).name + ).to.equal('ShinyRedirect') + }) + it('sets specified route', function() { expect(redirector(attrs).route).to.deep.equal(route) }) diff --git a/services/codeclimate/codeclimate-common.js b/services/codeclimate/codeclimate-common.js new file mode 100644 index 0000000000..86b43ba34c --- /dev/null +++ b/services/codeclimate/codeclimate-common.js @@ -0,0 +1,48 @@ +'use strict' + +const Joi = require('joi') +const { NotFound } = require('..') + +const keywords = ['codeclimate'] + +const repoSchema = Joi.object({ + data: Joi.array() + .max(1) + .items( + Joi.object({ + id: Joi.string().required(), + relationships: Joi.object({ + latest_default_branch_snapshot: Joi.object({ + data: Joi.object({ + id: Joi.string().required(), + }).allow(null), + }).required(), + latest_default_branch_test_report: Joi.object({ + data: Joi.object({ + id: Joi.string().required(), + }).allow(null), + }).required(), + }).required(), + }) + ) + .required(), +}).required() + +async function fetchRepo(serviceInstance, { user, repo }) { + const { + data: [repoInfo], + } = await serviceInstance._requestJson({ + schema: repoSchema, + url: 'https://api.codeclimate.com/v1/repos', + options: { qs: { github_slug: `${user}/${repo}` } }, + }) + if (repoInfo === undefined) { + throw new NotFound({ prettyMessage: 'repo not found' }) + } + return repoInfo +} + +module.exports = { + keywords, + fetchRepo, +} diff --git a/services/codeclimate/codeclimate-coverage-redirector.service.js b/services/codeclimate/codeclimate-coverage-redirector.service.js new file mode 100644 index 0000000000..34a7138f85 --- /dev/null +++ b/services/codeclimate/codeclimate-coverage-redirector.service.js @@ -0,0 +1,27 @@ +'use strict' + +const { redirector } = require('..') + +module.exports = [ + redirector({ + name: 'CodeclimateCoverageShortcutRedirect', + category: 'coverage', + route: { + base: 'codeclimate', + pattern: ':which(c|c-letter)/:user/:repo', + }, + transformPath: ({ which, user, repo }) => + `/codeclimate/${which.replace('c', 'coverage')}/${user}/${repo}`, + dateAdded: new Date('2019-04-15'), + }), + redirector({ + name: 'CodeclimateTopLevelCoverageRedirect', + category: 'coverage', + route: { + base: 'codeclimate', + pattern: ':user/:repo', + }, + transformPath: ({ user, repo }) => `/codeclimate/coverage/${user}/${repo}`, + dateAdded: new Date('2019-04-15'), + }), +] diff --git a/services/codeclimate/codeclimate-coverage-redirector.tester.js b/services/codeclimate/codeclimate-coverage-redirector.tester.js new file mode 100644 index 0000000000..c521a43730 --- /dev/null +++ b/services/codeclimate/codeclimate-coverage-redirector.tester.js @@ -0,0 +1,30 @@ +'use strict' + +const { ServiceTester } = require('../tester') + +const t = (module.exports = new ServiceTester({ + id: 'CodeclimateCoverageRedirector', + title: 'Code Climate Coverage Redirector', + pathPrefix: '/codeclimate', +})) + +t.create('Top-level coverage shortcut') + .get('/jekyll/jekyll.svg', { + followRedirect: false, + }) + .expectStatus(301) + .expectHeader('Location', '/codeclimate/coverage/jekyll/jekyll.svg') + +t.create('Coverage shortcut') + .get('/c/jekyll/jekyll.svg', { + followRedirect: false, + }) + .expectStatus(301) + .expectHeader('Location', '/codeclimate/coverage/jekyll/jekyll.svg') + +t.create('Coverage letter shortcut') + .get('/c-letter/jekyll/jekyll.svg', { + followRedirect: false, + }) + .expectStatus(301) + .expectHeader('Location', '/codeclimate/coverage-letter/jekyll/jekyll.svg') diff --git a/services/codeclimate/codeclimate-coverage.service.js b/services/codeclimate/codeclimate-coverage.service.js new file mode 100644 index 0000000000..8f2f5f1d7e --- /dev/null +++ b/services/codeclimate/codeclimate-coverage.service.js @@ -0,0 +1,94 @@ +'use strict' + +const Joi = require('joi') +const { BaseJsonService, NotFound } = require('..') +const { coveragePercentage, letterScore } = require('../color-formatters') +const { keywords, fetchRepo } = require('./codeclimate-common') + +const schema = Joi.object({ + data: Joi.object({ + attributes: Joi.object({ + covered_percent: Joi.number().required(), + rating: Joi.object({ + letter: Joi.equal('A', 'B', 'C', 'D', 'E', 'F').required(), + }).required(), + }).required(), + }).allow(null), +}).required() + +module.exports = class CodeclimateCoverage extends BaseJsonService { + static get route() { + return { + base: 'codeclimate', + pattern: ':which(coverage|coverage-letter)/:user/:repo', + } + } + + static get category() { + return 'coverage' + } + + static get examples() { + return [ + { + title: 'Code Climate coverage', + namedParams: { which: 'coverage', user: 'jekyll', repo: 'jekyll' }, + staticPreview: this.render({ + which: 'coverage', + percentage: 95.123, + letter: 'A', + }), + keywords, + }, + ] + } + + async fetch({ user, repo }) { + const { + id: repoId, + relationships: { + latest_default_branch_test_report: { data: testReportInfo }, + }, + } = await fetchRepo(this, { user, repo }) + if (testReportInfo === null) { + throw new NotFound({ prettyMessage: 'test report not found' }) + } + const { data } = await this._requestJson({ + schema, + url: `https://api.codeclimate.com/v1/repos/${repoId}/test_reports/${ + testReportInfo.id + }`, + errorMessages: { 404: 'test report not found' }, + }) + return data + } + + static render({ wantLetter, percentage, letter }) { + if (wantLetter) { + return { + message: letter, + color: letterScore(letter), + } + } else { + return { + message: `${percentage.toFixed(0)}%`, + color: coveragePercentage(percentage), + } + } + } + + async handle({ which, user, repo }) { + const { + attributes: { + rating: { letter }, + covered_percent: percentage, + }, + } = await this.fetch({ user, repo }) + + return this.constructor.render({ + wantLetter: which === 'coverage-letter', + letter, + percentage, + }) + } +} diff --git a/services/codeclimate/codeclimate-coverage.tester.js b/services/codeclimate/codeclimate-coverage.tester.js index 8379e15f1b..ebda92499a 100644 --- a/services/codeclimate/codeclimate-coverage.tester.js +++ b/services/codeclimate/codeclimate-coverage.tester.js @@ -1,53 +1,33 @@ 'use strict' const Joi = require('joi') -const { ServiceTester } = require('../tester') const { isIntegerPercentage } = require('../test-validators') - -const t = (module.exports = new ServiceTester({ - id: 'CodeClimateCoverage', - title: 'Code Climate', - pathPrefix: '/codeclimate', -})) +const t = (module.exports = require('../tester').createServiceTester()) t.create('test coverage percentage') - .get('/c/jekyll/jekyll.json') - .expectBadge({ - label: 'coverage', - message: isIntegerPercentage, - }) - -t.create('test coverage percentage alternative coverage URL') .get('/coverage/jekyll/jekyll.json') .expectBadge({ label: 'coverage', message: isIntegerPercentage, }) -t.create('test coverage percentage alternative top-level URL') - .get('/jekyll/jekyll.json') - .expectBadge({ - label: 'coverage', - message: isIntegerPercentage, - }) - t.create('test coverage letter') - .get('/c-letter/jekyll/jekyll.json') + .get('/coverage-letter/jekyll/jekyll.json') .expectBadge({ label: 'coverage', message: Joi.equal('A', 'B', 'C', 'D', 'E', 'F'), }) t.create('test coverage percentage for non-existent repo') - .get('/c/unknown/unknown.json') + .get('/coverage/unknown/unknown.json') .expectBadge({ label: 'coverage', - message: 'not found', + message: 'repo not found', }) t.create('test coverage percentage for repo without test reports') - .get('/c/angular/angular.js.json') + .get('/coverage/angular/angular.js.json') .expectBadge({ label: 'coverage', - message: 'unknown', + message: 'test report not found', }) diff --git a/services/codeclimate/codeclimate.service.js b/services/codeclimate/codeclimate.service.js index 28184d85e9..18d58f50ad 100644 --- a/services/codeclimate/codeclimate.service.js +++ b/services/codeclimate/codeclimate.service.js @@ -8,46 +8,6 @@ const { colorScale, } = require('../color-formatters') -class CodeclimateCoverage extends LegacyService { - static get route() { - return { - base: 'codeclimate', - pattern: ':which(coverage|coverage-letter)/:userRepo*', - } - } - - static get category() { - return 'coverage' - } - - static get examples() { - return [ - { - title: 'Code Climate coverage', - pattern: 'coverage/:userRepo', - namedParams: { userRepo: 'jekyll/jekyll' }, - staticPreview: { - label: 'coverage', - message: '95%', - color: 'green', - }, - }, - { - title: 'Code Climate coverage (letter)', - pattern: 'coverage-letter/:userRepo', - namedParams: { userRepo: 'jekyll/jekyll' }, - staticPreview: { - label: 'coverage', - message: 'A', - color: 'brightgreen', - }, - }, - ] - } - - static registerLegacyRouteHandler() {} -} - // This legacy service should be rewritten to use e.g. BaseJsonService. // // Tips for rewriting: @@ -106,7 +66,7 @@ class Codeclimate extends LegacyService { static registerLegacyRouteHandler({ camp, cache }) { camp.route( - /^\/codeclimate(\/(c|coverage|maintainability|issues|tech-debt)(-letter|-percentage)?)?\/(.+)\.(svg|png|gif|jpg|json)$/, + /^\/codeclimate(\/(maintainability|issues|tech-debt)(-letter|-percentage)?)\/(.+)\.(svg|png|gif|jpg|json)$/, cache((data, match, sendBadge, request) => { let type if (match[2] === 'c' || !match[2]) { @@ -232,6 +192,5 @@ class Codeclimate extends LegacyService { } module.exports = { - CodeclimateCoverage, Codeclimate, }