Repository: badges/shields
Pull request #2886 (2019-01-28 17:48:36)

 Move suggest code and rewrite tests
The suggest code was an exception to our usual organization pattern. There was a service test, but it's not a service. The code would sometimes regress because it wasn't being tested all the time.

This makes them no longer run as service tests, which is good because they run as part of every build. Some of them are smaller-bracket tests which is good too, because it will make them easier to test, especially as this code grows.

I'd have liked to keep using frisby for the ones that make requests to the server, though I ran into some issues with sequencing of setup that I think will require upstream changes.

shields-ci: 
<!--
  0 failure: 
  1 warning:  :books: Remember ...
  1 messages
  
  DangerID: danger-id-default;
-->


<table>
  <thead>
    <tr>
      <th width="50"></th>
      <th width="100%" data-danger-table="true">Warnings</th>
    </tr>
  </thead>
  <tbody><tr>
      <td>:warning:</td>
      <td>

  :books: Remember to ensure any changes to `serverSecrets` in `services/suggest.integration.js` are reflected in the [server secrets documentation](https://github.com/badges/shields/blob/master/doc/server-secrets.md)
  </td>
    </tr>
  </tbody>
</table>


<table>
  <thead>
    <tr>
      <th width="50"></th>
      <th width="100%" data-danger-table="true">Messages</th>
    </tr>
  </thead>
  <tbody><tr>
      <td>:book:</td>
      <td>:sparkles: Thanks for your contribution to Shields, @paulmelnikow!</td>
    </tr>
  </tbody>
</table>


<p align="right">
  Generated by :no_entry_sign: <a href="https://danger.systems/js">dangerJS</a> against fc9be31e429fee2ad64f08570b19078fe853863d
</p>


Patch:
diff --git a/core/server/server.js b/core/server/server.js
index 72de5b3373..15608d38f9 100644
--- a/core/server/server.js
+++ b/core/server/server.js
@@ -8,7 +8,7 @@ const Camp = require('camp')
 const makeBadge = require('../../gh-badges/lib/make-badge')
 const GithubConstellation = require('../../services/github/github-constellation')
 const { makeBadgeData } = require('../../lib/badge-data')
-const suggest = require('../../lib/suggest')
+const suggest = require('../../services/suggest')
 const { loadServiceClasses } = require('../base-service/loader')
 const { makeSend } = require('../base-service/legacy-result-sender')
 const {
diff --git a/services/suggest.integration.js b/services/suggest.integration.js
new file mode 100644
index 0000000000..e615e185c5
--- /dev/null
+++ b/services/suggest.integration.js
@@ -0,0 +1,145 @@
+'use strict'
+
+const { expect } = require('chai')
+const got = require('got')
+const Camp = require('camp')
+const portfinder = require('portfinder')
+const { setRoutes } = require('./suggest')
+const GithubApiProvider = require('./github/github-api-provider')
+const serverSecrets = require('../lib/server-secrets')
+
+describe('GitHub badge suggestions', function() {
+  const githubApiBaseUrl = process.env.GITHUB_URL || 'https://api.github.com'
+
+  let token, apiProvider
+  before(function() {
+    token = serverSecrets.gh_token
+    if (!token) {
+      throw Error('The integration tests require a gh_token to be set')
+    }
+    apiProvider = new GithubApiProvider({
+      baseUrl: githubApiBaseUrl,
+      globalToken: token,
+      withPooling: false,
+    })
+  })
+
+  let port, baseUrl
+  before(async function() {
+    port = await portfinder.getPortPromise()
+    baseUrl = `http://127.0.0.1:${port}`
+  })
+
+  let camp
+  before(async function() {
+    camp = Camp.start({ port, hostname: '::' })
+    await new Promise(resolve => camp.on('listening', () => resolve()))
+  })
+  after(async function() {
+    if (camp) {
+      await new Promise(resolve => camp.close(resolve))
+      camp = undefined
+    }
+  })
+
+  const origin = 'https://example.test'
+  before(function() {
+    setRoutes([origin], apiProvider, camp)
+  })
+
+  context('with an existing project', function() {
+    it('returns the expected suggestions', async function() {
+      const { statusCode, body } = await got(
+        `${baseUrl}/$suggest/v1?url=${encodeURIComponent(
+          'https://github.com/atom/atom'
+        )}`,
+        {
+          json: true,
+        }
+      )
+      expect(statusCode).to.equal(200)
+      expect(body).to.deep.equal({
+        suggestions: [
+          {
+            title: 'GitHub issues',
+            link: 'https://github.com/atom/atom/issues',
+            path: '/github/issues/atom/atom',
+          },
+          {
+            title: 'GitHub forks',
+            link: 'https://github.com/atom/atom/network',
+            path: '/github/forks/atom/atom',
+          },
+          {
+            title: 'GitHub stars',
+            link: 'https://github.com/atom/atom/stargazers',
+            path: '/github/stars/atom/atom',
+          },
+          {
+            title: 'GitHub license',
+            path: '/github/license/atom/atom',
+            link: 'https://github.com/atom/atom/blob/master/LICENSE.md',
+          },
+          {
+            title: 'Twitter',
+            link:
+              'https://twitter.com/intent/tweet?text=Wow:&url=https%3A%2F%2Fgithub.com%2Fatom%2Fatom',
+            path: '/twitter/url/https/github.com/atom/atom',
+            queryParams: {
+              style: 'social',
+            },
+          },
+        ],
+      })
+    })
+  })
+
+  context('with a non-existent project', function() {
+    it('returns the expected suggestions', async function() {
+      this.timeout(5000)
+
+      const { statusCode, body } = await got(
+        `${baseUrl}/$suggest/v1?url=${encodeURIComponent(
+          'https://github.com/badges/not-a-real-project'
+        )}`,
+        {
+          json: true,
+        }
+      )
+      expect(statusCode).to.equal(200)
+      expect(body).to.deep.equal({
+        suggestions: [
+          {
+            title: 'GitHub issues',
+            link: 'https://github.com/badges/not-a-real-project/issues',
+            path: '/github/issues/badges/not-a-real-project',
+          },
+          {
+            title: 'GitHub forks',
+            link: 'https://github.com/badges/not-a-real-project/network',
+            path: '/github/forks/badges/not-a-real-project',
+          },
+          {
+            title: 'GitHub stars',
+            link: 'https://github.com/badges/not-a-real-project/stargazers',
+            path: '/github/stars/badges/not-a-real-project',
+          },
+          {
+            title: 'GitHub license',
+            path: '/github/license/badges/not-a-real-project',
+            link: 'https://github.com/badges/not-a-real-project',
+          },
+          {
+            title: 'Twitter',
+            link:
+              'https://twitter.com/intent/tweet?text=Wow:&url=https%3A%2F%2Fgithub.com%2Fbadges%2Fnot-a-real-project',
+            path: '/twitter/url/https/github.com/badges/not-a-real-project',
+            queryParams: {
+              style: 'social',
+            },
+          },
+        ],
+      })
+    })
+  })
+})
diff --git a/lib/suggest.js b/services/suggest.js
similarity index 96%
rename from lib/suggest.js
rename to services/suggest.js
index 28fb70b3e1..0d6161fbbd 100644
--- a/lib/suggest.js
+++ b/services/suggest.js
@@ -2,8 +2,7 @@
 //
 // eg. /$suggest/v1?url=https://github.com/badges/shields
 //
-// Tests for this endpoint are in services/suggest/suggest.spec.js. The
-// endpoint is called from frontend/components/suggestion-and-search.js.
+// This endpoint is called from frontend/components/suggestion-and-search.js.
 
 'use strict'
 
@@ -157,5 +156,7 @@ function setRoutes(allowedOrigin, githubApiProvider, server) {
 }
 
 module.exports = {
+  findSuggestions,
+  githubLicense,
   setRoutes,
 }
diff --git a/services/suggest.spec.js b/services/suggest.spec.js
new file mode 100644
index 0000000000..1c7c9db9fb
--- /dev/null
+++ b/services/suggest.spec.js
@@ -0,0 +1,150 @@
+'use strict'
+
+const Camp = require('camp')
+const { expect } = require('chai')
+const got = require('got')
+const nock = require('nock')
+const portfinder = require('portfinder')
+const { setRoutes, githubLicense } = require('./suggest')
+const GithubApiProvider = require('./github/github-api-provider')
+
+describe('Badge suggestions', function() {
+  const githubApiBaseUrl = 'https://api.github.test'
+  const apiProvider = new GithubApiProvider({
+    baseUrl: githubApiBaseUrl,
+    globalToken: 'fake-token',
+    withPooling: false,
+  })
+
+  describe('GitHub license', function() {
+    context('When html_url included in response', function() {
+      it('Should link to it', async function() {
+        const scope = nock(githubApiBaseUrl)
+          .get('/repos/atom/atom/license')
+          .reply(200, {
+            html_url: 'https://github.com/atom/atom/blob/master/LICENSE.md',
+            license: {
+              key: 'mit',
+              name: 'MIT License',
+              spdx_id: 'MIT',
+              url: 'https://api.github.com/licenses/mit',
+              node_id: 'MDc6TGljZW5zZTEz',
+            },
+          })
+
+        expect(await githubLicense(apiProvider, 'atom', 'atom')).to.deep.equal({
+          title: 'GitHub license',
+          path: '/github/license/atom/atom',
+          link: 'https://github.com/atom/atom/blob/master/LICENSE.md',
+        })
+
+        scope.done()
+      })
+    })
+
+    context('When html_url not included in response', function() {
+      it('Should link to the repo', async function() {
+        const scope = nock(githubApiBaseUrl)
+          .get('/repos/atom/atom/license')
+          .reply(200, {
+            license: { key: 'mit' },
+          })
+
+        expect(await githubLicense(apiProvider, 'atom', 'atom')).to.deep.equal({
+          title: 'GitHub license',
+          path: '/github/license/atom/atom',
+          link: 'https://github.com/atom/atom',
+        })
+
+        scope.done()
+      })
+    })
+  })
+
+  describe('Scoutcamp integration', function() {
+    let port, baseUrl
+    before(async function() {
+      port = await portfinder.getPortPromise()
+      baseUrl = `http://127.0.0.1:${port}`
+    })
+
+    let camp
+    before(async function() {
+      camp = Camp.start({ port, hostname: '::' })
+      await new Promise(resolve => camp.on('listening', () => resolve()))
+    })
+    after(async function() {
+      if (camp) {
+        await new Promise(resolve => camp.close(resolve))
+        camp = undefined
+      }
+    })
+
+    const origin = 'https://example.test'
+    before(function() {
+      setRoutes([origin], apiProvider, camp)
+    })
+
+    context('without an origin header', function() {
+      it('returns the expected suggestions', async function() {
+        const scope = nock(githubApiBaseUrl)
+          .get('/repos/atom/atom/license')
+          .reply(200, {
+            html_url: 'https://github.com/atom/atom/blob/master/LICENSE.md',
+            license: {
+              key: 'mit',
+              name: 'MIT License',
+              spdx_id: 'MIT',
+              url: 'https://api.github.com/licenses/mit',
+              node_id: 'MDc6TGljZW5zZTEz',
+            },
+          })
+
+        const { statusCode, body } = await got(
+          `${baseUrl}/$suggest/v1?url=${encodeURIComponent(
+            'https://github.com/atom/atom'
+          )}`,
+          {
+            json: true,
+          }
+        )
+        expect(statusCode).to.equal(200)
+        expect(body).to.deep.equal({
+          suggestions: [
+            {
+              title: 'GitHub issues',
+              link: 'https://github.com/atom/atom/issues',
+              path: '/github/issues/atom/atom',
+            },
+            {
+              title: 'GitHub forks',
+              link: 'https://github.com/atom/atom/network',
+              path: '/github/forks/atom/atom',
+            },
+            {
+              title: 'GitHub stars',
+              link: 'https://github.com/atom/atom/stargazers',
+              path: '/github/stars/atom/atom',
+            },
+            {
+              title: 'GitHub license',
+              path: '/github/license/atom/atom',
+              link: 'https://github.com/atom/atom/blob/master/LICENSE.md',
+            },
+            {
+              title: 'Twitter',
+              link:
+                'https://twitter.com/intent/tweet?text=Wow:&url=https%3A%2F%2Fgithub.com%2Fatom%2Fatom',
+              path: '/twitter/url/https/github.com/atom/atom',
+              queryParams: {
+                style: 'social',
+              },
+            },
+          ],
+        })
+
+        scope.done()
+      })
+    })
+  })
+})


Test patch:
diff --git a/services/suggest/suggest.tester.js b/services/suggest/suggest.tester.js
deleted file mode 100644
index 2a4036b5c4..0000000000
--- a/services/suggest/suggest.tester.js
+++ /dev/null
@@ -1,89 +0,0 @@
-'use strict'
-
-// These tests are for the badge-suggestion endpoint in lib/suggest.js. This
-// endpoint is called from frontend/components/suggestion-and-search.js.
-
-const { ServiceTester } = require('..')
-const { invalidJSON } = require('../response-fixtures')
-
-const t = (module.exports = new ServiceTester({
-  id: 'suggest',
-  title: 'suggest',
-  pathPrefix: '/$suggest',
-}))
-
-t.create('issues, forks, stars and twitter')
-  .get(`/v1?url=${encodeURIComponent('https://github.com/atom/atom')}`)
-  .expectJSON('suggestions.?', {
-    title: 'GitHub issues',
-    link: 'https://github.com/atom/atom/issues',
-    path: '/github/issues/atom/atom',
-  })
-  .expectJSON('suggestions.?', {
-    title: 'GitHub forks',
-    link: 'https://github.com/atom/atom/network',
-    path: '/github/forks/atom/atom',
-  })
-  .expectJSON('suggestions.?', {
-    title: 'GitHub stars',
-    link: 'https://github.com/atom/atom/stargazers',
-    path: '/github/stars/atom/atom',
-  })
-  .expectJSON('suggestions.?', {
-    title: 'Twitter',
-    link:
-      'https://twitter.com/intent/tweet?text=Wow:&url=https%3A%2F%2Fgithub.com%2Fatom%2Fatom',
-    path: '/twitter/url/https/github.com/atom/atom',
-    queryParams: {
-      style: 'social',
-    },
-  })
-
-t.create('license')
-  .get(`/v1?url=${encodeURIComponent('https://github.com/atom/atom')}`)
-  .expectJSON('suggestions.?', {
-    title: 'GitHub license',
-    link: 'https://github.com/atom/atom/blob/master/LICENSE.md',
-    path: '/github/license/atom/atom',
-  })
-
-t.create('license for non-existing project')
-  .get(`/v1?url=${encodeURIComponent('https://github.com/atom/atom')}`)
-  .intercept(nock =>
-    nock('https://api.github.com')
-      .get(/\/repos\/atom\/atom\/license/)
-      .reply(404)
-  )
-  .expectJSON('suggestions.?', {
-    title: 'GitHub license',
-    link: 'https://github.com/atom/atom',
-    path: '/github/license/atom/atom',
-  })
-
-t.create('license when json response is invalid')
-  .get(`/v1?url=${encodeURIComponent('https://github.com/atom/atom')}`)
-  .intercept(nock =>
-    nock('https://api.github.com')
-      .get(/\/repos\/atom\/atom\/license/)
-      .reply(invalidJSON)
-  )
-  .expectJSON('suggestions.?', {
-    title: 'GitHub license',
-    link: 'https://github.com/atom/atom',
-    path: '/github/license/atom/atom',
-  })
-
-t.create('license when html_url not found in GitHub api response')
-  .get(`/v1?url=${encodeURIComponent('https://github.com/atom/atom')}`)
-  .intercept(nock =>
-    nock('https://api.github.com')
-      .get(/\/repos\/atom\/atom\/license/)
-      .reply(200, {
-        license: 'MIT',
-      })
-  )
-  .expectJSON('suggestions.?', {
-    title: 'GitHub license',
-    link: 'https://github.com/atom/atom',
-    path: '/github/license/atom/atom',
-  })