Detect from commit history which service tests to run

.circleci/config.yml

@@ -167,22 +167,12 @@ jobs:
           command: npm install
 
       - run:
-          name: Identify services tagged in the PR title
-          command: |
-            if [[ ! -z $CI_PULL_REQUEST ]]; then
-              npm run test:services:pr:prepare
-            else
-              echo 'This is not a pull request. Skipping.'
-            fi
+          name: Identify services in affected files and tagged in commit messages
+          command: npm run which-services
 
       - run:
-          name: Run tests for tagged services
-          command: |
-            if [[ ! -z $CI_PULL_REQUEST ]]; then
-              npm run test:services:pr:run
-            else
-              echo 'This is not a pull request. Skipping.'
-            fi
+          name: Run tests for identified services
+          command: npm run test:services
 
   services-pr@node-latest:
     docker:
@@ -207,22 +197,12 @@ jobs:
           command: npm install
 
       - run:
-          name: Identify services tagged in the PR title
-          command: |
-            if [[ ! -z $CI_PULL_REQUEST ]]; then
-              npm run test:services:pr:prepare
-            else
-              echo 'This is not a pull request. Skipping.'
-            fi
+          name: Identify services in affected files and tagged in commit messages
+          command: npm run which-services
 
       - run:
-          name: Run tests for tagged services
-          command: |
-            if [[ ! -z $CI_PULL_REQUEST ]]; then
-              npm run test:services:pr:run
-            else
-              echo 'This is not a pull request. Skipping.'
-            fi
+          name: Run tests for identified services
+          command: npm run test:services
 
   services-daily:
     docker:
@@ -236,7 +216,7 @@ jobs:
 
       - run:
           name: Run all service tests
-          command: npm run test:services
+          command: npm run test:services -- --all
 
 workflows:
   version: 2

.prettierignore

@@ -8,3 +8,4 @@ package-lock.json
 /.next
 /build
 /coverage
+**/*.md

dangerfile.js

@@ -8,6 +8,7 @@
 const { danger, fail, message, warn } = require('danger');
 const chainsmoker = require('chainsmoker');
 const { default: noTestShortcuts } = require('danger-plugin-no-test-shortcuts');
+const { identifyServices, identifyTesters } = require('./services');
 
 const fileMatch = chainsmoker({
   created: danger.git.created_files,
@@ -111,26 +112,8 @@ all_files.forEach(function(file) {
   });
 });
 
-function onlyUnique(value, index, self) {
-  return self.indexOf(value) === index;
-}
-
-const affectedServices = all_files
-  .map(function(file) {
-    const match = file.match(/^services\/(.+)\/.+\.service.js$/);
-    return match ? match[1] : undefined;
-  })
-  .filter(Boolean)
-  .filter(onlyUnique);
-
-const testedServices = all_files
-  .map(function(file) {
-    const match = file.match(/^services\/(.+)\/.+\.tester.js$/);
-    return match ? match[1] : undefined;
-  })
-  .filter(Boolean)
-  .filter(onlyUnique);
-
+const affectedServices = identifyServices(all_files);
+const testedServices = identifyTesters(all_files);
 affectedServices.forEach(function(service) {
   if (testedServices.indexOf(service) === -1) {
     warn(

doc/service-tests.md

@@ -162,11 +162,19 @@ harness will call it for you.
 Run the test:
 
 ```
-npm run test:services -- --only=travis
+npm run test:services
 ```
 
-The `--only=` option indicates which service or services you want to test. You
-can provide a comma-separated list of ids.
+The test runner examines the commit history to automatically detect which
+services to test.
+
+If for any reason it's not detecting the correct services, you can specify them
+yourself:
+
+npm run test:services -- --only=travis
+
+```
+You can provide a comma-separated list of case-insensitive ids.
 
 The `--` tells the NPM CLI to pass the remaining arguments through to the test
 runner.
@@ -279,10 +287,9 @@ t.create('connection error')
 Pull requests
 -------------
 
-The affected service ids should be included in brackets in the pull request
-title. That way, Travis will run those service tests. When a pull request
-affects multiple services, they should be separated with spaces. The test
-runner is case-insensitive, so they should be capitalized for readability.
+If for any reason the automatic service detection doesn't correctly identify
+the affected services, you can tag additional services in your commit
+messages.
 
 For example:
 

lib/service-test-runner/affected-services.js

@@ -0,0 +1,68 @@
+// Automatically detect the services changed by comparing the current commit
+// to `master`. This approach of examining commits was taken because detecting
+// a pull request from CircleCI is unreliable. It allows for a nicer dev
+// experience because it will generally automatically detect which files have
+// changed.
+//
+// If for any reason the automatic service detection doesn't correctly
+// identify the affected services, additional services can be tagged in a commit
+// message, e.g.
+//
+// - [Travis] Fix timeout issues
+// - [Travis Sonar] Support user token authentication
+// - [CRAN CPAN CTAN] Add test coverage
+
+'use strict'
+
+const simpleGit = require('simple-git/promise')
+const { identifyServices } = require('../../services')
+const servicesForTitle = require('./services-for-title')
+
+async function taggedServices() {
+  const log = await simpleGit(__dirname).log(['--first-parent', 'master..HEAD'])
+  const commitMessages = log.all.map(l => l.message)
+  return commitMessages.reduce(
+    (accum, message) => accum.concat(servicesForTitle(message)),
+    []
+  )
+}
+
+async function modifiedServices() {
+  const diffSummary = await simpleGit(__dirname).diffSummary([
+    '--no-renames',
+    'master...HEAD',
+  ])
+  const affectedFiles = diffSummary.files.map(f => f.file)
+  return identifyServices(affectedFiles)
+}
+
+async function allAffectedServices() {
+  return (await taggedServices()).concat(await modifiedServices())
+}
+
+async function main() {
+  const services = await allAffectedServices()
+  if (services.length === 0) {
+    console.error('No services found. Nothing to do.')
+  } else {
+    console.error(
+      `Services: (${services.length} found) ${services.join(', ')}\n`
+    )
+    console.log(services.join('\n'))
+  }
+}
+
+if (require.main === module) {
+  ;(async () => {
+    try {
+      await main()
+    } catch (e) {
+      console.error(e)
+      process.exit(1)
+    }
+  })()
+}
+
+module.exports = {
+  allAffectedServices,
+}

lib/service-test-runner/cli.js

@@ -46,9 +46,9 @@
 'use strict'
 
 const minimist = require('minimist')
-const readAllStdinSync = require('read-all-stdin-sync')
-const Runner = require('./runner')
 const serverHelpers = require('../../lib/in-process-server-test-helpers')
+const Runner = require('./runner')
+const { allAffectedServices } = require('./affected-services')
 
 require('../../lib/unhandled-rejection.spec')
 
@@ -61,42 +61,46 @@ after('Shut down the server', async function() {
   await serverHelpers.stop(server)
 })
 
-const runner = new Runner()
-runner.prepare()
-// The server's request cache causes side effects between tests.
-runner.beforeEach = () => {
-  serverHelpers.reset(server)
-}
+async function main() {
+  const runner = new Runner()
+  runner.prepare()
+  // The server's request cache causes side effects between tests.
+  runner.beforeEach = () => {
+    serverHelpers.reset(server)
+  }
 
-const args = minimist(process.argv.slice(3))
-const stdinOption = args.stdin
-const onlyOption = args.only
+  const args = minimist(process.argv.slice(3))
+  const allOption = args.all
+  const onlyOption = args.only
 
-let onlyServices
+  const onlyServices = onlyOption
+    ? onlyOption.split(',')
+    : await allAffectedServices()
 
-if (stdinOption && onlyOption) {
-  console.error('Do not use --only with --stdin')
-} else if (stdinOption) {
-  const allStdin = readAllStdinSync().trim()
-  onlyServices = allStdin ? allStdin.split('\n') : []
-} else if (onlyOption) {
-  onlyServices = onlyOption.split(',')
-}
+  if (allOption) {
+    console.info('Running all service tests.')
+  } else if (onlyServices.length === 0) {
+    console.info('No service tests to run. Exiting.')
+    process.exit(0)
+  } else {
+    console.info(
+      `Running tests for ${onlyServices.length} services: ${onlyServices.join(
+        ', '
+      )}.\n`
+    )
+    runner.only(onlyServices)
+  }
+
+  runner.toss()
 
-if (typeof onlyServices === 'undefined') {
-  console.info('Running all service tests.')
-} else if (onlyServices.length === 0) {
-  console.info('No service tests to run. Exiting.')
-  process.exit(0)
-} else {
-  console.info(
-    `Running tests for ${onlyServices.length} services: ${onlyServices.join(
-      ', '
-    )}.\n`
-  )
-  runner.only(onlyServices)
+  run()
 }
 
-runner.toss()
-// Invoke run() asynchronously, because Mocha will not start otherwise.
-process.nextTick(run)
+;(async () => {
+  try {
+    await main()
+  } catch (e) {
+    console.error(e)
+    process.exit(1)
+  }
+})()