Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test setup #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Test setup #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
05d53d7
Config setup
Oct 21, 2016
7107a30
Test setup wip
Nov 16, 2016
30ac9fa
Passing test
Nov 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,14 @@
},
"settings": {
"import/resolver": "meteor",
},
"globals": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of other ways we could do this (@pauldowman do you have an opinion here?):

{
    "env": {
        "mocha": true
    }
}

The above is maybe a little cleaner/semantic. One side-effect of setting these is that it affects non-test code. E.g., lodash has before and after methods, and lodash methods may be imported by name directly. app also seems like it could turn up. If we want to avoid that, we could ask testers to set this at the top of each test file:

/* eslint-env mocha */

That might be my preferred method, but I'm interested what anyone else thinks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rdickert
Fell free to change as you see fit. I don't have an opinion about this and you're probably better to decide what's the best method.
😄

"describe": "true",
"it": "true",
"expect": "true",
"app": "true",
"request":"true",
"before": "true",
"after": "true"
}
}
12 changes: 9 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
},
"license": "MIT",
"options": {
"mocha": "--require scripts/mocha_runner src/**/__tests__/**/*.js"
"mocha": "--require scripts/mocha_runner src/**/__tests__/**/*.js --opts src/__tests__/mocha.opts"
},
"scripts": {
"prepublish": ". ./scripts/prepublish.sh",
Expand All @@ -20,23 +20,29 @@
"start": "babel src -d dist --watch"
},
"devDependencies": {
"apollo-client": "^0.5.4",
"babel-cli": "6.x.x",
"babel-core": "6.x.x",
"babel-core": "^6.17.0",
"babel-eslint": "4.x.x",
"babel-plugin-transform-runtime": "6.x.x",
"babel-polyfill": "6.x.x",
"babel-preset-es2015": "6.x.x",
"babel-preset-stage-2": "6.x.x",
"chai": "3.x.x",
"chai-spies": "^0.7.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this package? It seems redundant with testdouble

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, using testdouble for that.

"eslint": "^2.9.0",
"eslint-config-airbnb": "^9.0.1",
"eslint-import-resolver-meteor": "^0.2.4",
"eslint-plugin-babel": "2.x.x",
"eslint-plugin-import": "^1.10.3",
"eslint-plugin-jsx-a11y": "^1.2.0",
"eslint-plugin-react": "^5.2.2",
"graphql-tag": "^0.1.16",
"mocha": "2.x.x",
"nodemon": "1.7.x"
"nodemon": "1.7.x",
"react": "^15.4.0",
"react-apollo": "^0.6.0",
"testdouble": "^1.8.0"
},
"dependencies": {
"babel-runtime": "6.x.x",
Expand Down
35 changes: 27 additions & 8 deletions src/__tests__/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
import {expect} from 'chai';
import {sum} from '../';
const {describe, it} = global;

describe('sum', () => {
it('should add two numbers correctly', async () => {
const result = await sum(10, 20);
expect(result).to.be.equal(30);
import chai from 'chai';
import td from 'testdouble';
import gql from 'graphql-tag';
import react from 'react';

const gqlDouble = gql`
query getTodos {
todos {
goal
}
}`;

const reactApolloMock = td.object(['graphql']);
let graphql;

describe('graphql', () => {

before(() => {
td.replace("react-apollo", reactApolloMock);
graphql = require('../index').graphql;
});


it('passes arguments correctly to originalGraphql', () => {
graphql(gqlDouble);

td.verify(reactApolloMock.graphql(gqlDouble, {name: 'getTodos'}));
});
});
6 changes: 6 additions & 0 deletions src/__tests__/mocha.opts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
--reporter spec
--compilers js:babel-core/register
--colors
--inline-diffs
--no-timeouts
--recursive
2 changes: 2 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const graphql = (document, config = {}) => {
throw new Error(`graphql(react-apollo-helpers): Expected 1 graphQL operation. ${document.definitions.length} operations detected.`);
}

const { name } = config;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug fix. It probably belongs in a separate pr, but since it's needed here and is a one-liner, let's maybe not worry about it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


// gather operation info from the graphQL document
const definitions = document.definitions[0];
const operationType = document.definitions[0].operation;
Expand Down
Loading