-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
describeWithDOM enhancement #126
Merged
lelandrichardson
merged 3 commits into
enzymejs:master
from
nfpiche:describeWithDOM-enhancement
Jan 21, 2016
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,9 @@ | |
"check": "npm run lint && npm run test:all", | ||
"build": "babel src --out-dir build", | ||
"test:watch": "mocha --compilers js:babel/register --recursive src/**/__tests__/*.js --watch", | ||
"test:all": "npm run react:13 && npm test && npm run react:14 && npm test", | ||
"test:describeWithDOMOnly": "mocha --compilers js:babel/register --recursive src/**/__tests__/describeWithDOM/describeWithDOMOnly-spec.js", | ||
"test:describeWithDOMSkip": "mocha --compilers js:babel/register --recursive src/**/__tests__/describeWithDOM/describeWithDOMSkip-spec.js", | ||
"test:all": "npm run react:13 && npm test && npm run test:describeWithDOMOnly && npm run test:describeWithDOMSkip && npm run react:14 && npm test && npm run test:describeWithDOMOnly && npm run test:describeWithDOMSkip", | ||
"react:clean": "rimraf node_modules/react node_modules/react-dom node_modules/react-addons-test-utils", | ||
"react:13": "npm run react:clean && npm i [email protected]", | ||
"react:14": "npm run react:clean && npm i [email protected] [email protected] [email protected]", | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { expect } from 'chai'; | ||
import { describeWithDOM } from '../..'; | ||
|
||
describe('describeWithDOM', () => { | ||
describe('.only()', () => { | ||
describeWithDOM.only('will skip all tests not called with only', () => { | ||
it('will run only this test', () => { | ||
expect(true).to.equal(true); | ||
}); | ||
}); | ||
|
||
describeWithDOM('will not call other tests', () => { | ||
it('will not run this test', () => { | ||
// purposefully failing test that won't be called | ||
expect(true).to.equal(false); | ||
}); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { expect } from 'chai'; | ||
import { describeWithDOM } from '../..'; | ||
|
||
describe('describeWithDOM', () => { | ||
describe('.skip()', () => { | ||
describeWithDOM.skip('will skip tests called with skip', () => { | ||
it('will not run this test', () => { | ||
// purposefully failing test that won't be run | ||
expect(true).to.equal(false); | ||
}); | ||
}); | ||
|
||
describeWithDOM('will still call describeWithDOM tests without .skip', () => { | ||
it('will run this test', () => { | ||
expect(true).to.equal(true); | ||
}); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
let jsdom; | ||
|
||
try { | ||
require('jsdom'); // could throw | ||
jsdom = require('mocha-jsdom'); | ||
} catch (e) { | ||
// jsdom is not supported... | ||
} | ||
|
||
export function describeWithDOM(a, b) { | ||
describe('(uses jsdom)', () => { | ||
if (typeof jsdom === 'function') { | ||
jsdom(); | ||
describe(a, b); | ||
} else { | ||
// if jsdom isn't available, skip every test in this describe context | ||
describe.skip(a, b); | ||
} | ||
}); | ||
} | ||
|
||
function only(a, b) { | ||
describe('(uses jsdom)', () => { | ||
if (typeof jsdom === 'function') { | ||
jsdom(); | ||
describe.only(a, b); | ||
} else { | ||
// if jsdom isn't available, skip every test in this describe context | ||
describe.skip(a, b); | ||
} | ||
}); | ||
} | ||
|
||
function skip(a, b) { | ||
describe('(uses jsdom)', () => { | ||
if (typeof jsdom === 'function') { | ||
jsdom(); | ||
describe.skip(a, b); | ||
} else { | ||
// if jsdom isn't available, skip every test in this describe context | ||
describe.skip(a, b); | ||
} | ||
}); | ||
} | ||
|
||
describeWithDOM.only = only; | ||
describeWithDOM.skip = skip; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in order to eliminate duplication, you could do smth like: (pseudo code) function testSuite(register, title, fn) {
describe('(uses jsdom)', () => {
if (typeof jsdom === 'function') {
jsdom();
register(a, b);
} else {
// if jsdom isn't available, skip every test in this describe context
describe.skip(a, b);
}
});
}
function only(title, fn) {
testSuite(describe.only, title, fn);
}
function skip(title, fn) {
testSuite(describe.skip, title, fn);
}
function describeWithDOM(title, fn) {
testSuite(describe, title, fn);
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm confused. Why exactly were these tests needed to be added separately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't technically need it for .skip, but I felt like having it be consistent would be the way to go. We do need a separate run for the .only tests though, otherwise it will skip the rest of the test suite. Happy to change things up a bit to be less verbose if you have an idea though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose that's fine. I'm not 100% sure we even need to add tests for describeWithDOM, but since they're already added it's better than nothing.