Forgot to push tests #1

Open
timoxley opened this Issue Mar 30, 2012 · 12 comments

2 participants

@timoxley

You must have forgotten to upload your tests.

@DamonOehlman

Um, yes. I forgot...

In actual fact I was just starting to write them then, but I wanted to base them on a simple CSV parser that implements good streams support. Surprisingly I couldn't find any. I'll keep looking and get those tests up :)

@timoxley

As in, streaming csv output?

@DamonOehlman

Yep - something you can pipe from. Although I'm not sure if I'm breaking some rules around Node.js streams by allowing objects to be transmitted in data events rather than just strings or buffers. Still it makes sense to me a stream is a pattern in node and objects are entitled to be streamed just as much as raw data.

Anyway, I've forked a copy of node-csv-parser and looking at making it support pipe (by inheriting from Stream rather than EventEmitter). Just installing expresso now to make sure I haven't broken it.

@timoxley

This fellow supports a pseudo stream, in that it emits data events for each csv line, but doesn't inherit from Stream unfortunately, so no pipe(). Might be an easy, valuable fix though.

@timoxley

haha. Looks like we're on the same page.

@DamonOehlman

Yep :) Can't get expresso to install though :/

@timoxley

Refactor to mocha? It installed fine for me, perhaps push to a fork and I'll run the tests for you :P

@DamonOehlman

Probably have a borked install, will try cleaning up and installing again. In the meantime if you could run the tests against https://github.com/DamonOehlman/node-csv-parser that would be awesome :)

@timoxley

I will… when you push some changes.

@DamonOehlman

Ah, yes I see. Not sure what I did there :/

Anyway they are pushed now...

@timoxley

Well, you didn't break anything, but you also didn't test that pipe() was actually working…

@DamonOehlman

Fair point, I managed to get expresso installed finally, so I'll add a test to check that pipe actually works.

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