Skip to content

Commit

Permalink
Clean up our callback style and enforce no exclusive tests and remove…
Browse files Browse the repository at this point in the history
… an exclusive test (#1900)

We use arrow functions in most places; this enforces it.

Passing arrow functions to Mocha is discouraged: https://mochajs.org/#arrow-functions

This was a mix of autofixes and hand adjustments.
  • Loading branch information
paulmelnikow authored Aug 13, 2018
1 parent 0a7c833 commit 66d444a
Show file tree
Hide file tree
Showing 35 changed files with 185 additions and 192 deletions.
3 changes: 3 additions & 0 deletions .eslintrc-preferred.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ rules:
no-var: "error"
prefer-const: "error"
strict: "error"
arrow-body-style: ["error", "as-needed"]

# Chai friendly.
no-unused-expressions: "off"
chai-friendly/no-unused-expressions: "error"

mocha/prefer-arrow-callback: "error"
6 changes: 6 additions & 0 deletions .eslintrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ env:
es6: true
mocha: true

plugins:
- mocha

extends:
# Enable a set of unopinionated, style-agnostic rules which cover many
# likely errors.
Expand All @@ -28,3 +31,6 @@ rules:
eol-last: "error"
object-curly-spacing: ["error", "always"]
comma-dangle: ["error", "always-multiline"]

mocha/no-exclusive-tests: "error"
mocha/no-mocha-arrows: "error"
15 changes: 5 additions & 10 deletions frontend/components/search-results.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,11 @@ export default class SearchResults extends React.Component {
}

renderCategoryHeadings() {
return this.preparedExamples.map(function(category, i) {
return (
<Link
to={'/examples/' + category.category.id}
key={category.category.id}
>
<h3 id={category.category.id}>{category.category.name}</h3>
</Link>
)
})
return this.preparedExamples.map((category, i) => (
<Link to={'/examples/' + category.category.id} key={category.category.id}>
<h3 id={category.category.id}>{category.category.name}</h3>
</Link>
))
}

render() {
Expand Down
2 changes: 1 addition & 1 deletion lib/analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function defaultAnalytics() {
function load() {
const defaultAnalyticsObject = defaultAnalytics()
if (useRedis) {
redis.get(analyticsPath, function(err, value) {
redis.get(analyticsPath, (err, value) => {
if (err == null && value != null) {
// if/try/return trick:
// if error, then the rest of the function is run.
Expand Down
69 changes: 34 additions & 35 deletions lib/github-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ if (serverSecrets && serverSecrets.gh_token) {
function setRoutes(server) {
const baseUrl = process.env.BASE_URL || 'https://img.shields.io'

server.route(/^\/github-auth$/, function(data, match, end, ask) {
server.route(/^\/github-auth$/, (data, match, end, ask) => {
if (!(serverSecrets && serverSecrets.gh_client_id)) {
return end('This server is missing GitHub client secrets.')
}
Expand All @@ -41,7 +41,7 @@ function setRoutes(server) {
end('')
})

server.route(/^\/github-auth\/done$/, function(data, match, end, ask) {
server.route(/^\/github-auth\/done$/, (data, match, end, ask) => {
if (
!(
serverSecrets &&
Expand All @@ -68,7 +68,7 @@ function setRoutes(server) {
}),
method: 'POST',
}
request(options, function(err, res, body) {
request(options, (err, res, body) => {
if (err != null) {
return end('The connection to GitHub failed.')
}
Expand Down Expand Up @@ -96,16 +96,16 @@ function setRoutes(server) {
'<p><a href="/">Back to the website</a></p>'
)

sendTokenToAllServers(token).catch(function(e) {
sendTokenToAllServers(token).catch(e => {
console.error('GitHub user token transmission failed:', e)
})
})
})

server.route(/^\/github-auth\/add-token$/, function(data, match, end, ask) {
server.route(/^\/github-auth\/add-token$/, (data, match, end, ask) => {
if (!secretIsValid(data.shieldsSecret)) {
// An unknown entity tries to connect. Let the connection linger for 10s.
return setTimeout(function() {
return setTimeout(() => {
end('Invalid secret.')
}, 10000)
}
Expand All @@ -117,33 +117,34 @@ function setRoutes(server) {
function sendTokenToAllServers(token) {
const ips = serverSecrets.shieldsIps
return Promise.all(
ips.map(function(ip) {
return new Promise(function(resolve, reject) {
const options = {
url: 'https://' + ip + '/github-auth/add-token',
method: 'POST',
form: {
shieldsSecret: serverSecrets.shieldsSecret,
token: token,
},
// We target servers by IP, and we use HTTPS. Assuming that
// 1. Internet routers aren't hacked, and
// 2. We don't unknowingly lose our IP to someone else,
// we're not leaking people's and our information.
// (If we did, it would have no impact, as we only ask for a token,
// no GitHub scope. The malicious entity would only be able to use
// our rate limit pool.)
// FIXME: use letsencrypt.
strictSSL: false,
}
request(options, function(err, res, body) {
if (err != null) {
return reject(err)
ips.map(
ip =>
new Promise((resolve, reject) => {
const options = {
url: 'https://' + ip + '/github-auth/add-token',
method: 'POST',
form: {
shieldsSecret: serverSecrets.shieldsSecret,
token: token,
},
// We target servers by IP, and we use HTTPS. Assuming that
// 1. Internet routers aren't hacked, and
// 2. We don't unknowingly lose our IP to someone else,
// we're not leaking people's and our information.
// (If we did, it would have no impact, as we only ask for a token,
// no GitHub scope. The malicious entity would only be able to use
// our rate limit pool.)
// FIXME: use letsencrypt.
strictSSL: false,
}
resolve()
request(options, (err, res, body) => {
if (err != null) {
return reject(err)
}
resolve()
})
})
})
})
)
)
}

Expand Down Expand Up @@ -179,9 +180,7 @@ function isTokenUsable(token, now) {
// with a reasonable chance that the request will succeed.
function usableTokens() {
const now = utcEpochSeconds()
return githubUserTokens.filter(function(token) {
return isTokenUsable(token, now)
})
return githubUserTokens.filter(token => isTokenUsable(token, now))
}

// Retrieve a user token if there is one for which we believe there are requests
Expand Down Expand Up @@ -308,7 +307,7 @@ function githubRequest(request, url, query, cb) {
url += '?' + qs
}

request(url, { headers: headers }, function(err, res, buffer) {
request(url, { headers: headers }, (err, res, buffer) => {
if (globalToken !== null && githubToken !== null && err === null) {
if (res.statusCode === 401) {
// Unauthorized.
Expand Down
4 changes: 2 additions & 2 deletions lib/github-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const { age } = require('./color-formatters')
function mapGithubCommitsSince({ camp, cache }, githubApiProvider) {
camp.route(
/^\/github\/commits-since\/([^/]+)\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
cache(function(data, match, sendBadge, request) {
cache((data, match, sendBadge, request) => {
const user = match[1] // eg, SubtitleEdit
const repo = match[2] // eg, subtitleedit
const version = match[3] // eg, 3.4.7 or latest
Expand Down Expand Up @@ -73,7 +73,7 @@ function mapGithubCommitsSince({ camp, cache }, githubApiProvider) {
function mapGithubReleaseDate({ camp, cache }, githubApiProvider) {
camp.route(
/^\/github\/(release-date|release-date-pre)\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
cache(function(data, match, sendBadge, request) {
cache((data, match, sendBadge, request) => {
const releaseType = match[1] // eg, release-date-pre / release-date
const user = match[2] // eg, microsoft
const repo = match[3] // eg, vscode
Expand Down
2 changes: 1 addition & 1 deletion lib/licenses.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const { test, given } = require('sazerac')
const { licenseToColor } = require('./licenses')

describe('license helpers', () => {
describe('license helpers', function() {
test(licenseToColor, () => {
given('MIT').expect('green')
given('MPL-2.0').expect('orange')
Expand Down
2 changes: 1 addition & 1 deletion lib/load-logos.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function loadLogos() {
const logos = {}
const logoDir = path.join(__dirname, '..', 'logo')
const logoFiles = fs.readdirSync(logoDir)
logoFiles.forEach(function(filename) {
logoFiles.forEach(filename => {
if (filename[0] === '.') {
return
}
Expand Down
2 changes: 1 addition & 1 deletion lib/load-simple-icons.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const simpleIcons = require('simple-icons')
const { svg2base64 } = require('./logo-helper')

function loadSimpleIcons() {
Object.keys(simpleIcons).forEach(function(key) {
Object.keys(simpleIcons).forEach(key => {
const k = key.toLowerCase().replace(/ /g, '-')
if (k !== key) {
simpleIcons[k] = simpleIcons[key]
Expand Down
2 changes: 1 addition & 1 deletion lib/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const throttledConsoleError = throttle(console.error, 10000, {
module.exports.error = function error(...msg) {
const d = date()
listeners.forEach(f => f(d, ...msg))
Raven.captureException(msg, function(sendErr) {
Raven.captureException(msg, sendErr => {
if (sendErr) {
throttledConsoleError(
'Failed to send captured exception to Sentry',
Expand Down
18 changes: 8 additions & 10 deletions lib/log.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,25 @@ function requireUncached(module) {
return require(module)
}

describe('log', () => {
describe('error', () => {
before(() => {
describe('log', function() {
describe('error', function() {
before(function() {
this.clock = sinon.useFakeTimers()
sinon
.stub(Raven, 'captureException')
.callsFake(function fakeFn(e, callback) {
callback(new Error(`Cannot send message "${e}" to Sentry.`))
})
sinon.stub(Raven, 'captureException').callsFake((e, callback) => {
callback(new Error(`Cannot send message "${e}" to Sentry.`))
})
// we have to create a spy before requiring 'error' function
sinon.spy(console, 'error')
this.error = requireUncached('./log').error
})

after(() => {
after(function() {
this.clock.restore()
console.error.restore()
Raven.captureException.restore()
})

it('should throttle errors from Raven client', () => {
it('should throttle errors from Raven client', function() {
this.error('test error 1')
this.error('test error 2')
this.error('test error 3')
Expand Down
2 changes: 1 addition & 1 deletion lib/luarocks-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ exports.compareVersionLists = compareVersionLists
function parseVersion(versionString) {
versionString = versionString.toLowerCase().replace('-', '.')
const versionList = []
versionString.split('.').forEach(function(versionPart) {
versionString.split('.').forEach(versionPart => {
const parsedPart = /(\d*)([a-z]*)(\d*)/.exec(versionPart)
if (parsedPart[1]) {
versionList.push(parseInt(parsedPart[1]))
Expand Down
8 changes: 4 additions & 4 deletions lib/make-badge.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const badgeKeyWidthCache = new LruCache(1000)
const templates = {}
const templateFiles = fs.readdirSync(path.join(__dirname, '..', 'templates'))
dot.templateSettings.strip = false // Do not strip whitespace.
templateFiles.forEach(async function(filename) {
templateFiles.forEach(async filename => {
if (filename[0] === '.') {
return
}
Expand All @@ -29,7 +29,7 @@ templateFiles.forEach(async function(filename) {
// Substitute dot code.
const mapping = new Map()
let mappingIndex = 1
const untemplatedSvg = templateData.replace(/{{.*?}}/g, function(match) {
const untemplatedSvg = templateData.replace(/{{.*?}}/g, match => {
// Weird substitution that currently works for all templates.
const mapKey = '99999990' + mappingIndex + '.1'
mappingIndex++
Expand All @@ -52,9 +52,9 @@ templateFiles.forEach(async function(filename) {
// Substitute dot code back.
let svg = data
const unmappedKeys = []
mapping.forEach(function(value, key) {
mapping.forEach((value, key) => {
let keySubstituted = false
svg = svg.replace(RegExp(key, 'g'), function() {
svg = svg.replace(RegExp(key, 'g'), () => {
keySubstituted = true
return value
})
Expand Down
Loading

0 comments on commit 66d444a

Please sign in to comment.