AriaFallah / mobx-store

A data store with declarative querying, observable state, and easy undo/redo.
MIT License
282 stars 9 forks source link

Consider putting tests next to testees for better code cohesion? #21

Closed dmitriz closed 8 years ago

dmitriz commented 8 years ago

Just a general remark/ suggestion, a lot has been written about the advantages...

Unless you have some reason not too, of course.

AriaFallah commented 8 years ago

@dmitriz what do you mean by this? Could you be a bit more specific?

dmitriz commented 8 years ago

Sorry for being cryptic ;)

There are many long definitions of code cohesion but the idea is simple:

Put things that belong together close to each other.

A test file for store.js is closely related to the file, so it is best to put it right next to it. This is what most modern libraries are doing, see e.g.:

https://github.com/callemall/material-ui/tree/master/src/Avatar https://github.com/angular/material/tree/master/src/components/card

BTW, in their style guidelines, Google recommends to add _test.js rather than .test.js or .spec.js, which I find more readable and easy to distinguish from the rest or your code:

store.css
store.js
store_test.js
store.submodule.css
store.submodule.js
store.submodule_test.js
AriaFallah commented 8 years ago

Hmmm I don't think I'm going to do that for a few reasons.

Put things that belong together close to each other.

I believe I'm not violating this premise. This actually follows the conventions shown in the repos you linked. You should consider this one large component like those. The only difference is that instead of putting all the tests into one giant file I opted to split them up in the test directory. Otherwise I could just put everything is store.spec.js if I wished.

dmitriz commented 8 years ago

It is actually easy to set up your test runner to filter for something like *_test.js, but I agree it is not too much of a problem with a single folder package.

But for larger packages can be real pain to navigate in parallel their convoluted trees of source and test files.

AriaFallah commented 8 years ago

Good point on the filtering, but until I end up making something like this:

https://github.com/babel/babel/tree/master/packages

I think my tests are fine where they are.