Repository: badges/shields
Pull request #2861 (2019-01-24 05:29:06)

Refactor service loader; test on [static dynamic endpoint]
This moves the loader code into `core/base-service`, leaving behind in `services/index.js` only the convenience imports.

Ref #2832


Patch:
diff --git a/core/base-service/loader.js b/core/base-service/loader.js
new file mode 100644
index 0000000000..1363c39d36
--- /dev/null
+++ b/core/base-service/loader.js
@@ -0,0 +1,83 @@
+'use strict'
+
+const path = require('path')
+const glob = require('glob')
+const { categories } = require('../../services/categories')
+const BaseService = require('./base')
+const { assertValidServiceDefinitionExport } = require('./service-definitions')
+
+const serviceDir = path.join(__dirname, '..', '..', 'services')
+
+class InvalidService extends Error {
+  constructor(message) {
+    super(message)
+    this.name = 'InvalidService'
+  }
+}
+
+function loadServiceClasses(servicePaths) {
+  if (!servicePaths) {
+    servicePaths = glob.sync(path.join(serviceDir, '**', '*.service.js'))
+  }
+
+  const serviceClasses = []
+  servicePaths.forEach(path => {
+    const module = require(path)
+    if (
+      !module ||
+      (module.constructor === Array && module.length === 0) ||
+      (module.constructor === Object && Object.keys(module).length === 0)
+    ) {
+      throw new InvalidService(
+        `Expected ${path} to export a service or a collection of services`
+      )
+    } else if (module.prototype instanceof BaseService) {
+      serviceClasses.push(module)
+    } else if (module.constructor === Array || module.constructor === Object) {
+      for (const key in module) {
+        const serviceClass = module[key]
+        if (serviceClass.prototype instanceof BaseService) {
+          serviceClasses.push(serviceClass)
+        } else {
+          throw new InvalidService(
+            `Expected ${path} to export a service or a collection of services; one of them was ${serviceClass}`
+          )
+        }
+      }
+    } else {
+      throw new InvalidService(
+        `Expected ${path} to export a service or a collection of services; got ${module}`
+      )
+    }
+  })
+
+  serviceClasses.forEach(ServiceClass => ServiceClass.validateDefinition())
+
+  return serviceClasses
+}
+
+function collectDefinitions() {
+  const services = loadServiceClasses()
+    // flatMap.
+    .map(ServiceClass => ServiceClass.getDefinition())
+    .reduce((accum, these) => accum.concat(these), [])
+
+  const result = { schemaVersion: '0', categories, services }
+
+  assertValidServiceDefinitionExport(result)
+
+  return result
+}
+
+function loadTesters() {
+  return glob
+    .sync(path.join(serviceDir, '**', '*.tester.js'))
+    .map(path => require(path))
+}
+
+module.exports = {
+  InvalidService,
+  loadServiceClasses,
+  collectDefinitions,
+  loadTesters,
+}
diff --git a/core/base-service/loader.spec.js b/core/base-service/loader.spec.js
new file mode 100644
index 0000000000..7d9b051c51
--- /dev/null
+++ b/core/base-service/loader.spec.js
@@ -0,0 +1,57 @@
+'use strict'
+
+const { expect } = require('chai')
+const { loadServiceClasses, InvalidService } = require('./loader')
+
+describe('loadServiceClasses function', function() {
+  it('throws if module exports empty', function() {
+    expect(() =>
+      loadServiceClasses(['../../test-fixtures/empty-undefined.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../../test-fixtures/empty-array.fixture.js'])
+    ).to.throw()
+    expect(() =>
+      loadServiceClasses(['../../test-fixtures/empty-object.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../../test-fixtures/empty-no-export.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses([
+        '../../test-fixtures/valid-array.fixture.js',
+        '../../test-fixtures/valid-class.fixture.js',
+        '../../test-fixtures/empty-array.fixture.js',
+      ])
+    ).to.throw(InvalidService)
+  })
+
+  it('throws if module exports invalid', function() {
+    expect(() =>
+      loadServiceClasses(['../../test-fixtures/invalid-no-base.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../../test-fixtures/invalid-wrong-base.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../../test-fixtures/invalid-mixed.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses([
+        '../../test-fixtures/valid-array.fixture.js',
+        '../../test-fixtures/valid-class.fixture.js',
+        '../../test-fixtures/invalid-no-base.fixture.js',
+      ])
+    ).to.throw(InvalidService)
+  })
+
+  it('registers services if module exports valid service classes', function() {
+    expect(
+      loadServiceClasses([
+        '../../test-fixtures/valid-array.fixture.js',
+        '../../test-fixtures/valid-object.fixture.js',
+        '../../test-fixtures/valid-class.fixture.js',
+      ])
+    ).to.have.length(5)
+  })
+})
diff --git a/core/server/server.js b/core/server/server.js
index f4fe2869fe..72de5b3373 100644
--- a/core/server/server.js
+++ b/core/server/server.js
@@ -7,9 +7,9 @@ const Joi = require('joi')
 const Camp = require('camp')
 const makeBadge = require('../../gh-badges/lib/make-badge')
 const GithubConstellation = require('../../services/github/github-constellation')
-const { loadServiceClasses } = require('../../services')
 const { makeBadgeData } = require('../../lib/badge-data')
 const suggest = require('../../lib/suggest')
+const { loadServiceClasses } = require('../base-service/loader')
 const { makeSend } = require('../base-service/legacy-result-sender')
 const {
   handleRequest,
diff --git a/scripts/export-service-definitions-cli.js b/scripts/export-service-definitions-cli.js
index 7d33f91225..5994515ca0 100644
--- a/scripts/export-service-definitions-cli.js
+++ b/scripts/export-service-definitions-cli.js
@@ -2,7 +2,7 @@
 
 const yaml = require('js-yaml')
 
-const { collectDefinitions } = require('../services')
+const { collectDefinitions } = require('../core/base-service/loader')
 
 const definitions = collectDefinitions()
 
diff --git a/scripts/refactoring-cli.js b/scripts/refactoring-cli.js
index 4e8b09270c..8981e4a2b2 100644
--- a/scripts/refactoring-cli.js
+++ b/scripts/refactoring-cli.js
@@ -4,7 +4,7 @@ const chalk = require('chalk')
 
 const { namedColors } = require('../gh-badges/lib/color')
 const { floorCount } = require('../lib/color-formatters')
-const { loadServiceClasses } = require('../services')
+const { loadServiceClasses } = require('../core/base-service/loader')
 
 const serviceClasses = loadServiceClasses()
 const legacyServices = serviceClasses
diff --git a/services/check-services.spec.js b/services/check-services.spec.js
new file mode 100644
index 0000000000..082f8a6cd7
--- /dev/null
+++ b/services/check-services.spec.js
@@ -0,0 +1,9 @@
+'use strict'
+
+const { collectDefinitions } = require('../core/base-service/loader')
+
+it('Can collect the service definitions', function() {
+  // When this fails, it will throw AssertionErrors. Wrapping this in an
+  // `expect().not.to.throw()` makes the error output unreadable.
+  collectDefinitions()
+})
diff --git a/services/index.js b/services/index.js
index 7e9bd6ba55..5d1a862733 100644
--- a/services/index.js
+++ b/services/index.js
@@ -1,87 +1,11 @@
 'use strict'
 
-const glob = require('glob')
 const base = require('../core/base-service')
 const createServiceTester = require('../core/service-test-runner/create-service-tester')
 const ServiceTester = require('../core/service-test-runner/service-tester')
-const {
-  assertValidServiceDefinitionExport,
-} = require('../core/base-service/service-definitions')
-const { categories } = require('./categories')
-
-const { BaseService } = base
-
-class InvalidService extends Error {
-  constructor(message) {
-    super(message)
-    this.name = 'InvalidService'
-  }
-}
-
-function loadServiceClasses(servicePaths) {
-  if (!servicePaths) {
-    servicePaths = glob.sync(`${__dirname}/**/*.service.js`)
-  }
-
-  const serviceClasses = []
-  servicePaths.forEach(path => {
-    const module = require(path)
-    if (
-      !module ||
-      (module.constructor === Array && module.length === 0) ||
-      (module.constructor === Object && Object.keys(module).length === 0)
-    ) {
-      throw new InvalidService(
-        `Expected ${path} to export a service or a collection of services`
-      )
-    } else if (module.prototype instanceof BaseService) {
-      serviceClasses.push(module)
-    } else if (module.constructor === Array || module.constructor === Object) {
-      for (const key in module) {
-        const serviceClass = module[key]
-        if (serviceClass.prototype instanceof BaseService) {
-          serviceClasses.push(serviceClass)
-        } else {
-          throw new InvalidService(
-            `Expected ${path} to export a service or a collection of services; one of them was ${serviceClass}`
-          )
-        }
-      }
-    } else {
-      throw new InvalidService(
-        `Expected ${path} to export a service or a collection of services; got ${module}`
-      )
-    }
-  })
-
-  serviceClasses.forEach(ServiceClass => ServiceClass.validateDefinition())
-
-  return serviceClasses
-}
-
-function collectDefinitions() {
-  const services = loadServiceClasses()
-    // flatMap.
-    .map(ServiceClass => ServiceClass.getDefinition())
-    .reduce((accum, these) => accum.concat(these), [])
-
-  const result = { schemaVersion: '0', categories, services }
-
-  assertValidServiceDefinitionExport(result)
-
-  return result
-}
-
-function loadTesters() {
-  return glob.sync(`${__dirname}/**/*.tester.js`).map(path => require(path))
-}
 
 module.exports = {
   ...base,
   createServiceTester,
   ServiceTester,
-  InvalidService,
-  loadServiceClasses,
-  loadTesters,
-  collectDefinitions,
 }
diff --git a/services/index.spec.js b/services/index.spec.js
deleted file mode 100644
index 556601cde3..0000000000
--- a/services/index.spec.js
+++ /dev/null
@@ -1,67 +0,0 @@
-'use strict'
-
-const { expect } = require('chai')
-const {
-  loadServiceClasses,
-  InvalidService,
-  collectDefinitions,
-} = require('./index')
-
-describe('loadServiceClasses function', function() {
-  it('throws if module exports empty', function() {
-    expect(() =>
-      loadServiceClasses(['../test-fixtures/empty-undefined.fixture.js'])
-    ).to.throw(InvalidService)
-    expect(() =>
-      loadServiceClasses(['../test-fixtures/empty-array.fixture.js'])
-    ).to.throw()
-    expect(() =>
-      loadServiceClasses(['../test-fixtures/empty-object.fixture.js'])
-    ).to.throw(InvalidService)
-    expect(() =>
-      loadServiceClasses(['../test-fixtures/empty-no-export.fixture.js'])
-    ).to.throw(InvalidService)
-    expect(() =>
-      loadServiceClasses([
-        '../test-fixtures/valid-array.fixture.js',
-        '../test-fixtures/valid-class.fixture.js',
-        '../test-fixtures/empty-array.fixture.js',
-      ])
-    ).to.throw(InvalidService)
-  })
-
-  it('throws if module exports invalid', function() {
-    expect(() =>
-      loadServiceClasses(['../test-fixtures/invalid-no-base.fixture.js'])
-    ).to.throw(InvalidService)
-    expect(() =>
-      loadServiceClasses(['../test-fixtures/invalid-wrong-base.fixture.js'])
-    ).to.throw(InvalidService)
-    expect(() =>
-      loadServiceClasses(['../test-fixtures/invalid-mixed.fixture.js'])
-    ).to.throw(InvalidService)
-    expect(() =>
-      loadServiceClasses([
-        '../test-fixtures/valid-array.fixture.js',
-        '../test-fixtures/valid-class.fixture.js',
-        '../test-fixtures/invalid-no-base.fixture.js',
-      ])
-    ).to.throw(InvalidService)
-  })
-
-  it('registers services if module exports valid service classes', function() {
-    expect(
-      loadServiceClasses([
-        '../test-fixtures/valid-array.fixture.js',
-        '../test-fixtures/valid-object.fixture.js',
-        '../test-fixtures/valid-class.fixture.js',
-      ])
-    ).to.have.length(5)
-  })
-
-  it('check the service definitions', function() {
-    // When this fails, it will throw AssertionErrors. Wrapping this in an
-    // `expect().not.to.throw()` makes the error output unreadable.
-    collectDefinitions()
-  })
-})


Test patch:
diff --git a/core/service-test-runner/runner.js b/core/service-test-runner/runner.js
index ba7daa82b8..8ade7a3175 100644
--- a/core/service-test-runner/runner.js
+++ b/core/service-test-runner/runner.js
@@ -1,6 +1,6 @@
 'use strict'
 
-const { loadTesters } = require('../../services')
+const { loadTesters } = require('../base-service/loader')
 
 /**
  * Load a collection of ServiceTester objects and register them with Mocha.