Skip to content

Switch test frameworks to mocha, expect and istanbul#99

Merged
phated merged 6 commits intogulpjs:masterfrom
sttk:use_mocha_and_expect
Jan 6, 2017
Merged

Switch test frameworks to mocha, expect and istanbul#99
phated merged 6 commits intogulpjs:masterfrom
sttk:use_mocha_and_expect

Conversation

@sttk
Copy link
Copy Markdown
Contributor

@sttk sttk commented Dec 25, 2016

I modified test codes for mocha + expect.
And I made enable to measure their coverage with istanbul.

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 25, 2016

Coverage Status

Changes Unknown when pulling 445395c on sttk:use_mocha_and_expect into ** on gulpjs:master**.

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 25, 2016

Coverage Status

Changes Unknown when pulling 87c3b74 on sttk:use_mocha_and_expect into ** on gulpjs:master**.

@sttk
Copy link
Copy Markdown
Contributor Author

sttk commented Dec 25, 2016

Sorry. I forgot to modify appveyor.yml for Windows.

Copy link
Copy Markdown
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Some open questions and some tweaks

Comment thread package.json Outdated
"prepublish": "marked-man --name gulp docs/CLI.md > gulp.1",
"pretest": "npm run lint",
"test": "lab test/*.js -cv -I Reflect",
"test": "mocha --reporter spec",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought spec was the default reporter. You can use the same set of scripts as https://github.com/gulpjs/vinyl/blob/master/package.json#L22-L26

Comment thread test/completion.js Outdated

function cb(err, stdout) {
expect(stdout).to.contain('gulp --completion=' + type);
expect(stdout).toMatch('gulp --completion=' + type);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why toMatch instead of toEqual?

Comment thread test/completion.js Outdated

function cb(err, stdout) {
expect(stdout).to.contain('rules for \'unknown\' not found');
expect(stdout).toMatch('rules for \'unknown\' not found');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why toMatch instead of toEqual?

Comment thread test/flags-continue.js Outdated

function cb(err, stdout, stderr) {
expect(err).to.be.not.null();
expect(err).toNotBe(null);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why toNotBe instead of toNotEqual?

Comment thread test/flags-continue.js Outdated

function cb(err, stdout, stderr) {
expect(err).to.be.not.null();
expect(err).toNotBe(null);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why toNotBe instead of toNotEqual?

Comment thread test/flags-verify.js Outdated

function cb(err, stdout) {
expect(err).to.be.not.null();
expect(err).toNotBe(null);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why toNotBe instead of toNotEqual?

Comment thread test/flags-verify.js Outdated

function cb(err, stdout) {
expect(err).to.be.not.null();
expect(err).toNotBe(null);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why toNotBe instead of toNotEqual?

Comment thread test/logging.js Outdated
// jscs:enable

var cmd;
if (isIstanbul) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what is this doing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wrote the code referencing this part of istanbul's document. But the code became unnecessary by nyc.

@phated
Copy link
Copy Markdown
Member

phated commented Dec 26, 2016

@sttk an aside: do you think we should be using https://www.npmjs.com/package/nyc for istanbul coverage because we spawn child processes?

@sttk
Copy link
Copy Markdown
Contributor Author

sttk commented Jan 5, 2017

I modified what you pointed out, and made use of nyc for coverage.

@phated
Copy link
Copy Markdown
Member

phated commented Jan 6, 2017

@sttk great work once again! Thank you so much!!!

@phated phated merged commit dd7ff9f into gulpjs:master Jan 6, 2017
@sttk
Copy link
Copy Markdown
Contributor Author

sttk commented Jan 7, 2017

@phated Thank you for reviewing and merging this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants