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.

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

Replace Tap with Mocha as the test framework #201

Merged
merged 2 commits into from
May 10, 2019

Conversation

demurgos
Copy link
Member

@demurgos demurgos commented May 9, 2019

The tests should remain equivalent.

Closes #196

test/stream.js Outdated Show resolved Hide resolved
Copy link
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.

@demurgos this looks great! I hope the conversion was easy 😄 I just had a few comments/suggestions.

test/error.js Outdated Show resolved Hide resolved
pipe(
[gulp.src(filename), task()],
(err) => {
try {
Copy link
Member

Choose a reason for hiding this comment

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

Thrown errors here don't bubble up to the test?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is related to not using miss.concat in this test: I was just checking for the existence of an error and did not care about the content.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have to keep this style to pass the error to done.

test/error.js Outdated Show resolved Hide resolved
test/extends.js Show resolved Hide resolved
test/filters.js Outdated Show resolved Hide resolved
test/stream.js Outdated Show resolved Hide resolved
test/stream.js Outdated Show resolved Hide resolved
.pipe(task())
.pipe(expectStream(t));
});
return through.obj(assert);
Copy link
Member

Choose a reason for hiding this comment

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

This actually needs to be concat if we plan to keep this abstraction.

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is that this function acts as a stream transformation (pass-through or error). Draining should be handled outside of it. I added concat calls to the pipes using it.

test/test.js Outdated Show resolved Hide resolved
@demurgos demurgos force-pushed the test-framework branch 2 times, most recently from 668d789 to e13decf Compare May 10, 2019 11:42
The tests should remain equivalent.

Closes gulp-community#196
@demurgos
Copy link
Member Author

I fixed the small issues. I'm looking into getting rid of the gulp dependency.

@demurgos
Copy link
Member Author

@phated I update the tests to no longer depend on gulp.

@demurgos demurgos merged commit c7acde3 into gulp-community:master May 10, 2019
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.

Update test framework
2 participants