-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove bsb dev setup suggestion #49
Conversation
This setup causes problems in most projects because (at least in create-react-app based ones, probably many others): - if you put `__tests__` folder at the root (outside of `src`) it will not be picked up by Jest. - if you put it in the `src` folder then bsb will try to double compile it and cause compiler errors. Not doing this configuration is fine because the test files are never bundled with the other files, even though it's not in dev mode. It also follows Create React Apps best practices of [putting test files next to the implementation ones.](https://facebook.github.io/create-react-app/docs/running-tests#filename-conventions)
|
||
## Usage | ||
|
||
Put tests in a `__tests__` directory and use the suffix `*test.ml`/`*test.re` (Make sure to use valid module names. e.g. `<name>_test.re` is valid while `<name>.test.re` is not). When compiled they will be put in a `__tests__` directory under `lib`, with a `*test.js` suffix, ready to be picked up when you run `jest`. If you're not already familiar with [Jest](https://github.com/facebook/jest), see [the Jest documentation](https://facebook.github.io/jest/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just changed the wording a little here. Mostly adding back-tics around things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an issue with create-react-app
and probably several other over-complicated boilerplate generators making flawed assumptions. But then that's kind of their thing, and ought to be a reason to just not use it IMO.
if you put tests folder at the root (outside of src) it will not be picked up by Jest.
It should work fine if you run jest
from the root of your project, which is where most people work from anyways.
if you put it in the src folder then bsb will try to double compile it and cause compiler errors.
I assume this is because you now have a source for both src
and src/__tests__
? Otherwise I don't see what would cause the problem, other than perhaps some magic from create-react-app
. A workaround might be to have the first source point to a subdirectory like src/app
.
Not doing this configuration is fine because the test files are never bundled with the other files, even though it's not in dev mode.
No, it's not. Dev-only sources and dependencies exist to isolate dev-time code and dependencies from production code and dependencies. This is especially important when developing a library to avoid consumers having to install unnecessary dev-time dependencies and compile code that might pollute the namespace and yield warnings or even errors that are completely unnecessary. But even when building an app you shouldn't have test modules polluting the production namespace, and the possibility of having production code accidentally depend on them.
It also follows Create React Apps best practices of putting test files next to the implementation ones.
One of the arguments don't apply to BuckleScript projects because there are no imports. The other is at best just preference, but is outright bad practice in BuckleScript for the reason explained above.
So the bottom line is: I'm not going to accept this as-is since it encourages bad practice. But if you can find a way to make it work with create-react-app
and friends without significantly degrading the experience for everyone else, I'd be happy to. Otherwise this should be fixed at the source.
Yeah I was doing Yeah I can see your viewpoint. I didn't think about the pure-reason library perspective. In this case it might cause problems. Webpack shakes out the tests from what I can see (probably because it does this with all I don't mind having test separated, however I feel that it's very foreign to JS developers, at least to my company. I think I could settle for perhaps a warning for people trying to add bs-jest to an existing CRA project? I just found that I following the README.md verbatim has got me in trouble multiple times. |
Webpack starts from the entry point you give it and then recursively includes whatever is imported from there. So if you accidentally reference a test module (or more likely a module containing utilities for tests), that'll be included in the bundle too. It's not because tests are treated differently.
There's definitely an argument to be made for familiarity, but it's not exactly on the top of my list. Consider what
Yeah, a footnote or something for CRA-users with actionable information would be great! |
Closed in favor of my second try #50 |
This setup causes problems in most projects because (at least in create-react-app based ones, probably many others):
__tests__
folder at the root (outside ofsrc
) it will not be picked up by Jest.src
folder then bsb will try to double compile it and cause compiler errors.Not doing this configuration is fine because the test files are never bundled with the other files, even though it's not in dev mode. It also follows Create React Apps best practices of putting test files next to the implementation ones.