Reorganize tests folder

[talyssonoc]

I'm willing to contribute more to the lib but wasn't able to run the tests locally, so I reorganized the tests folder to make sure it will run in every case, what do you think?

[alex-seville]

What happened when you tried to run the tests locally?

[jeremyruppel]

And what os/node/mocha version are you running? Are you running npm test or something else? To me this sounds like a problem other than the organization of the test files.

Sent from my iPhone

On May 3, 2016, at 7:12 PM, Alex Seville notifications@github.com wrote:

What happened when you tried to run the tests locally?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub

[talyssonoc]

@alex-seville It wasn't finding tmpdir, since it wasn't running _setup.js before the tests. Mocha doesn't guarantee the order files on the same folder will run, so you have to use nested folders to ensure that a file is running before another.

@jeremyruppel I was running npm test, on Windows 8, Node v6, with local Mocha in 2.3.4 version specified on package.json.

It's a problem caused by the file organization since it was not running _setup.js before the others.

Also, thanks for that lib, really liked the approach on recording/playbacking network interactions!

[alex-seville]

Is it possible that the beforeEach statements in _setup are being run after some of the describe blocks?

[alex-seville]

@jeremyruppel this one - https://github.com/flickr/yakbak/blob/master/test/yakbak.js#L12

If this file is parsed by mocha before setup it's possible that the beforeEach gets called before tmpdir is initialized.

[alex-seville]

Is it as simple as moving that beforeEach into the describe block?

[jeremyruppel]

@alex-seville Yuuuuuup, I bet you're right about that.

@talyssonoc can you check out #8 and see if that fixes your issue?

[talyssonoc]

@jeremyruppel @alex-seville yep, it solve that problem!

But it still doesn't run _setup.js before the rest of the files. Since it's a setup file, shouldn't we ensure that it's run first?

[jeremyruppel]

@talyssonoc another way to fix this might be to refactor _setup.js into some helper modules and have tests that need a server or tmpdir use them explicitly. This way, tests like buffer wouldn't have that (admittedly small) overhead and we can reduce the dependence on the context namespace. I'll try and whip up a patch for that now.

[jeremyruppel]

@talyssonoc what do you think about #9?

[talyssonoc]

@jeremyruppel way better now, IMHO! I guess I can close my PR after #9 gets merged :)

What do you think about we still have the yakbak modes inside contexts inside a describe('yakbak') like I did in my PR? Maybe it would make test output clearer.

[jeremyruppel]

I was thinking about that while I was making that patch, actually. If we're going to add more modes, it might be cleaner to have a separate test file for each mode, like mode.record.js, mode.playback.js, etc. (maybe there's a better name?). Right now I keep getting tripped up on whether a failure in the "record" test is actually in record.js or yakbak.js 😞 . Whaddya think?

[talyssonoc]

Having different files may be a good approach too, but what do you think about modes/record.js instead of mode.record.js? Then test/**/*.js should be added to mocha.opts file. For now, maybe using the context could solve if you prefer not having a mocha.opts file.

[jeremyruppel]

I suppose subdirectories could work, but nowadays I've been trying to follow the flat unit test suite style from projects like express and superagent. Seems simpler to use the mocha default of not looking for test files recursively, but I'm really not married to one way over the other 😄 .

[talyssonoc]

I see! Well, maybe context is an option by now while there isn't more modes yet?