Skip to content
This repository was archived by the owner on Sep 10, 2019. It is now read-only.

Commit 96ba0c3

Browse files
jlengstorfecwyne
authored andcommitted
fix: add tests
* test: add logger tests * test: add tests for data source handling * refactor: move tests to a separate directory Jest was testing the fixtures, and moving things was faster than trying to figure out why. :) * fix: improve error handling While writing tests, realized that there were uncaught Promise rejections and that part of the error handling chain was apparently unreachable. This fixes that. * test: add Code Climate coverage reporting * test: enable testing during CI * refactor: make the gateway more testable Express is hard to mock, so by splitting this into an app-specific and server-specific set of files, we can bypass mocking Express altogether. * refactor: update cleanup for easier testing The cleanup process is now its own helper file. Made the cleanup synchronous to simplify the program flow and make testing easier. Removed some unused imports. close #22 * refactor: map + filter -> filter
1 parent f96465d commit 96ba0c3

23 files changed

+668
-392
lines changed

.travis.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,4 @@ branches:
2626
- "/^greenkeeper/.*$/"
2727
env:
2828
global:
29-
- CC_TEST_REPORTER_ID=500f2daa890ec8bac5ffac26012bd3ff6e672ceb28ac53a1f254645d35168012
29+
- CC_TEST_REPORTER_ID=94dc98db9b1d2264af57f5a2891f135593bd89f611050a8c711d8af4f69a2a63

bin/dev.js

+17-24
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ import path from 'path';
33
import yargs from 'yargs';
44
import cleanup from 'node-cleanup';
55
import { spawn } from 'cross-spawn';
6-
import startDefaultGateway from '../lib/gateway';
6+
import startDefaultGateway from '../gateway';
77
import {
88
loadDataSources,
99
transpileDataSources,
1010
cleanUpTempDir,
1111
} from '../lib/data-sources';
12-
import { log, success, warn } from '../lib/logger';
12+
import cleanupOnExit from '../lib/cleanup-on-exit';
13+
import { warn } from '../lib/logger';
1314

1415
const getDirPath = dir => path.resolve(process.cwd(), dir);
1516

@@ -75,11 +76,9 @@ export const builder = yargs =>
7576
['transpile', 'no-transpile'],
7677
'Choose whether to transpile data sources with Babel:',
7778
)
78-
.options({
79-
transpile: {
80-
type: 'boolean',
81-
default: true,
82-
},
79+
.option('transpile', {
80+
type: 'boolean',
81+
default: true,
8382
});
8483

8584
export const handler = async ({
@@ -93,9 +92,16 @@ export const handler = async ({
9392
let dataSourcePaths = [];
9493
let loadedDataSources = [];
9594
if (dataSources.length) {
96-
// Get an array of paths to the local data sources.
97-
dataSourcePaths = await transpileDataSources(transpile, dataSources);
98-
loadedDataSources = loadDataSources(dataSourcePaths);
95+
try {
96+
// Get an array of paths to the local data sources.
97+
dataSourcePaths = await transpileDataSources(transpile, dataSources);
98+
loadedDataSources = loadDataSources(dataSourcePaths);
99+
} catch (error) {
100+
// If something went wrong loading data sources, log it, tidy up, and die.
101+
console.error(error);
102+
await cleanUpTempDir();
103+
process.exit(2); // eslint-disable-line no-process-exit
104+
}
99105
}
100106

101107
startGateway({
@@ -106,17 +112,4 @@ export const handler = async ({
106112
});
107113
};
108114

109-
cleanup((exitCode, signal) => {
110-
// Delete the temporary directory.
111-
cleanUpTempDir().then(shouldPrintShutdownMessage => {
112-
if (shouldPrintShutdownMessage) {
113-
success('Successfully shut down. Thanks for using GrAMPS!');
114-
}
115-
116-
process.kill(process.pid, signal);
117-
});
118-
119-
// Uninstall the handler to prevent an infinite loop.
120-
cleanup.uninstall();
121-
return false;
122-
});
115+
cleanup(cleanupOnExit);

bin/gramps.js

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { EOL } from 'os';
21
import yargs from 'yargs';
32
import dev from './dev';
43

gateway/configure-app.js

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import bodyParser from 'body-parser';
2+
import gramps from '@gramps/gramps';
3+
import { graphqlExpress } from 'apollo-server-express';
4+
import playground from 'graphql-playground-middleware-express';
5+
6+
import { GRAPHQL_ENDPOINT, TESTING_ENDPOINT } from '.';
7+
8+
export default function configureApp(app, config) {
9+
const GraphQLOptions = gramps(config);
10+
11+
app.use(bodyParser.json());
12+
app.use(GRAPHQL_ENDPOINT, graphqlExpress(GraphQLOptions));
13+
app.use(TESTING_ENDPOINT, playground({ endpoint: GRAPHQL_ENDPOINT }));
14+
15+
return app;
16+
}

gateway/index.js

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import Express from 'express';
2+
3+
import configureApp from './configure-app';
4+
import startServer from './start-server';
5+
6+
export const GRAPHQL_ENDPOINT = '/graphql';
7+
export const TESTING_ENDPOINT = '/playground';
8+
export const DEFAULT_PORT = 8080;
9+
10+
export default config => {
11+
const app = configureApp(Express(), config);
12+
startServer(app, config);
13+
};

gateway/start-server.js

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import getPort from 'get-port';
2+
3+
import { success } from '../lib/logger';
4+
import { DEFAULT_PORT, GRAPHQL_ENDPOINT, TESTING_ENDPOINT } from '.';
5+
6+
export default async function startServer(
7+
app,
8+
{ enableMockData, dataSources = [] } = {},
9+
) {
10+
const PORT = await getPort(DEFAULT_PORT);
11+
app.listen(PORT, () => {
12+
const mode = enableMockData ? 'mock' : 'live';
13+
success([
14+
'='.repeat(65),
15+
'',
16+
` A GraphQL gateway has successfully started using ${mode} data.`,
17+
``,
18+
` The following GrAMPS data sources are running locally:`,
19+
...dataSources.map(src => ` - ${src.namespace}`),
20+
``,
21+
` For UI development, point your GraphQL client to:`,
22+
` http://localhost:${PORT}${GRAPHQL_ENDPOINT}`,
23+
``,
24+
` To test your data sources in the GraphQL Playground, visit:`,
25+
` http://localhost:${PORT}${TESTING_ENDPOINT}`,
26+
``,
27+
' To stop this gateway, press `control` + `C`.',
28+
``,
29+
'='.repeat(65),
30+
]);
31+
});
32+
}

lib/cleanup-on-exit.js

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import cleanup from 'node-cleanup';
2+
import { cleanUpTempDir } from './data-sources';
3+
import { success } from './logger';
4+
5+
export default (_, signal) => {
6+
// Uninstall the handler to prevent an infinite loop.
7+
cleanup.uninstall();
8+
9+
// Delete the temporary directory.
10+
const shouldPrintShutdownMessage = cleanUpTempDir();
11+
12+
if (shouldPrintShutdownMessage) {
13+
success('Successfully shut down. Thanks for using GrAMPS!');
14+
}
15+
16+
process.kill(process.pid, signal);
17+
18+
return false;
19+
};

lib/data-sources.js

+35-16
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { error, log, warn } from './logger';
99

1010
const TEMP_DIR = path.resolve(__dirname, '..', '.tmp');
1111

12-
const handleError = (err, msg, callback) => {
12+
export const handleError = (err, msg, callback) => {
1313
if (!err) {
1414
return;
1515
}
@@ -34,11 +34,12 @@ const getDirName = dir =>
3434
export const cleanUpTempDir = () => {
3535
if (fs.existsSync(TEMP_DIR)) {
3636
log(' -> cleaning up temporary files');
37-
return del(TEMP_DIR, { force: true });
37+
del.sync(TEMP_DIR, { force: true });
38+
return true;
3839
}
3940

4041
// If there’s no temp dir, return a resolved promise (basically a no-op).
41-
return Promise.resolve(false);
42+
return false;
4243
};
4344

4445
const makeTempDir = tmpDir =>
@@ -52,14 +53,37 @@ const makeTempDir = tmpDir =>
5253

5354
const makeParentTempDir = () =>
5455
new Promise((resolve, reject) => {
55-
cleanUpTempDir().then(() => {
56-
mkdirp(TEMP_DIR, err => {
57-
handleError(err, `Could not create ${TEMP_DIR}`);
58-
resolve(TEMP_DIR);
59-
});
56+
cleanUpTempDir();
57+
58+
mkdirp(TEMP_DIR, err => {
59+
handleError(err, `Could not create ${TEMP_DIR}`);
60+
resolve(TEMP_DIR);
6061
});
6162
});
6263

64+
const hasRequiredProperties = (dataSource, dirPath) => {
65+
const requiredKeys = ['namespace', 'resolvers', 'typeDefs'];
66+
const missingKeys = requiredKeys.filter(k => !dataSource.hasOwnProperty(k));
67+
const hasAllRequiredProperties = !missingKeys.length;
68+
69+
if (!hasAllRequiredProperties) {
70+
const name = dataSource.namespace || `in ${getDirName(dirPath)}`;
71+
error([
72+
`${'='.repeat(36)} ERROR! ${'='.repeat(36)}`,
73+
``,
74+
` The data source ${name} is missing required properties:`,
75+
...missingKeys.map(str => ` - ${str}`),
76+
` `,
77+
` For details on data sources, see the docs at:`,
78+
` http://gramps.js.org/data-source/data-source-overview/`,
79+
``,
80+
`=`.repeat(80),
81+
]);
82+
}
83+
84+
return hasAllRequiredProperties;
85+
};
86+
6387
const isValidDataSource = dataSourcePath => {
6488
if (!fs.existsSync(dataSourcePath)) {
6589
warn(`Could not load a data source from ${dataSourcePath}`);
@@ -74,18 +98,13 @@ const loadDataSourceFromPath = dataSourcePath => {
7498
const src = require(dataSourcePath);
7599
const dataSource = src.default || src;
76100

77-
if (!dataSource.namespace || !dataSource.context || !dataSource.typeDefs) {
78-
error([
79-
`Error: this data source is missing required properties:`,
80-
dataSourcePath,
81-
]);
82-
101+
if (!hasRequiredProperties(dataSource, dataSourcePath)) {
83102
throw new Error('Invalid data source.');
84103
}
85104

86105
log(` -> successfully loaded ${dataSource.namespace}`);
87106

88-
return src.default || src;
107+
return dataSource;
89108
};
90109

91110
export const loadDataSources = pathArr => {
@@ -152,7 +171,7 @@ const transpileDataSource = parentDir => dataSource =>
152171
.then(copyGraphQLFiles(dataSource))
153172
.then(symlinkNodeModules(dataSource))
154173
.then(resolve)
155-
.catch(err => handleError(err, null, reject));
174+
.catch(reject);
156175
});
157176

158177
export const transpileDataSources = (shouldTranspile, dataSources) =>

lib/gateway.js

-52
This file was deleted.

package.json

+3-5
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"gramps": "bin/index.js",
2323
"prepush": "npm test",
2424
"lint": "eslint {bin,lib}/**/*.js",
25-
"test": "npm run lint --silent",
25+
"test": "npm run lint --silent && npm run test:unit",
2626
"test:unit": "cross-env NODE_ENV=test LOG4JS_LEVEL='OFF' jest --coverage",
2727
"semantic-release": "semantic-release pre && npm publish && semantic-release post"
2828
},
@@ -40,7 +40,6 @@
4040
"get-port": "^3.2.0",
4141
"globby": "^7.1.1",
4242
"graphql-playground-middleware-express": "^1.3.8",
43-
"inquirer": "^4.0.1",
4443
"mkdirp": "^0.5.1",
4544
"node-cleanup": "^2.1.2",
4645
"reify": "^0.13.5",
@@ -65,7 +64,6 @@
6564
"graphql-tools": "^2.14.1",
6665
"husky": "^0.14.3",
6766
"jest": "^22.0.0",
68-
"nodemon": "^1.11.0",
6967
"prettier": "^1.5.3",
7068
"semantic-release": "^8.2.0"
7169
},
@@ -76,8 +74,8 @@
7674
"lcov"
7775
],
7876
"collectCoverageFrom": [
79-
"{src,bin}/**/*.js",
80-
"!bin/gramps.js"
77+
"{bin,lib,gateway}/**/*.js",
78+
"!bin/index.js"
8179
],
8280
"coverageThreshold": {
8381
"global": {

0 commit comments

Comments
 (0)