Streams tests guidance

[alexjeffburke]

Hi Streams folks,

I'd like to get involved and agree that the most pressing thing is probably as many tests as can be mustered so we can start to move forward able to rely on the test suite for correctness of changes and catch regressions etc.

My current plan is work on a branch in a fork of node that simply slings in tests. In the absence of another approach think it best just to start somewhere and reviews can tell me if individual cases are relevant or not.

That being said, having looked at what's there I'd like to agree how to structure these additions. Many of the current tests seem to test lots of things in one file, for example for _writev it does a few different encodings, then cork(), uncork() and it makes me a little nervous. The following seem like important decisions:

• what is the scope of each test?
  Should they each do one thing or test all cases of a given feature?
• How explicit should tests be? There are any number of assertions one could make - I think the temptation is to assert everything given the separated nature of files but can we assume our coverage is gained over a bunch of files?
• What syntax should these files use? I've been leaning toward using fat arrows in the node repo under the assumption we want to cover the semantics of the current implementation.

Thanks, Alex J Burke.

[mcollina]

Thanks @alexjeffburke! TL;DR get started, and then we review!

A lot of streams tests were written long ago, so they accumulated some cruft. I agree with you that most of the tests are complex and verify a full feature (with a lot of assertions). Also, a lot of things are extremely complex to reason about in the current tests.

IMHO we should have more tests with a narrow scope, any other opinion?

What syntax should these files use? I've been leaning toward using fat arrows in the node repo under the assumption we want to cover the semantics of the current implementation.

Those were never updated to fat arrows, const and the like. They need updating.

You will probably wait some time until a consensus is reached on this one, so start working on it and send a PR.

[calvinmetcalf]

My current plan is work on a branch in a fork of node that simply slings in tests. In the absence of another approach think it best just to start somewhere and reviews can tell me if individual cases are relevant or not.

I'd add a single new test first and we can give you some detailed feed back on that one and I'd suggest sticking to single new tests per pull while you're getting the hang of it so that if 3 out of 4 are good but one needs work we can merge 3 without waiting for the 4th to be fixed which may take a while.

[alexjeffburke]

Great - I've added a first couple at alexjeffburke/streams-tests. Look forward to the feedback :)

[mcollina]

@alexjeffburke send a PR, we will review them there!

[alexjeffburke]

nodejs/node#6493