Refactor tests - Githubissues

bithavoc / express-winston

express.js middleware for winstonjs

https://www.npmjs.com/package/express-winston

MIT License

798 stars 186 forks source link

Refactor tests #60

Closed floatingLomas closed 8 years ago

floatingLomas commented 9 years ago

They're a bit of a mess - it's actually my fault for splitting 'v0.1.0' and 'v0.2.0' apis into sections way back - so I'll clean it up when I can - just wanted to make note of it here. Now that there's coverage checking, this can finally be done without missing anything.

The structure of the tests - the describes and its - should better follow the structure of the module with a describe for each exports-ed item;
All the functionality should be explicitly tested for, as some is currently just 'coincidentally' tested;
Most of the setup code can probably be refactored and better reused;

gingermusketeer commented 9 years ago

this would be awesome. I looked at the tests for the first time today. Found them quite hard to follow

rosston commented 8 years ago

@floatingLomas I'm interested in refactoring the tests and think I could probably get it done well—except for the separation between the v0.1.x and v0.2.x API tests. I've looked over them and don't understand why they're separated like that. If both APIs/behaviors are actively supported in the current version, then it seems like the tests should be able to all be lumped together. Does that make sense? I'm sure I'm probably missing some detail here.

bithavoc commented 8 years ago

also @rosston please add yourself as contributor in the Readme

floatingLomas commented 8 years ago

Cool. They're separated like that because I wanted to make sure the new code still passed all the previous tests when I bumped versions - and because I was really new at all this back then. It was done way back in 5e2f0f6beffdbf3196233370aca7bf840e415906.

So it's an ancient artifact, one that you are free to kill in your pursuit of a less hideous test suite - at least as far as I'm concerned.

rosston commented 8 years ago

Wow, that's a long time ago! And cool, that makes things easier. I think your original 3 points at the top of this thread are a rough outline of what I plan to do.

For cleaning up the setup code, I plan to move most of the existing helper functions towards promises, as that should make it easier to chain assertions off of the async code without needing to always use a before block. Which should in turn make it easier to consolidate/eliminate some of the describe blocks.