-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Should process.cwd()
be changed to the directory of the currently running test?
#32
Comments
I think it's a good idea but I'm not sure if it has any drawbacks as I lack experience in this kind of thing. |
It sounds good, but it might make migrating from Mocha a bit of a pain. New projects will benefit from it; ones coming from Mocha won't. |
@Qix- Why? It will make no difference if you've already hard-coded your paths with |
True. Those that haven't should be anyway. |
👍 Also tired of |
Yeah, I like option two too because that's really what is expected. Using the project root would confuse me. |
Me too. 👍 |
I think this is more intuitive for people who haven't used mocha. But would this confuse people who just learned |
That's not correct. If you start the node script from the file location they're the same. The point is that running a test is kinda like doing |
True, very intuitive because this is what I thought when I first used mocha :) |
I have been pretty consistently been annoyed by this "feature". Yes // relies on cwd - but it isn't apparent how use my IDE's search tool to find it.
execa('node foo.js'); Moving a test that relies on Conversely paths relative to the module file are pretty easy to locate. Look for I would have preferred that |
@jamestalmage I agree. In hindsight, it was a bad idea with good intentions. Too bad it didn't show its downsides until much later. I've personally been bitten by this on multiple occasions too and seen some problems with it in the ecosystem. I think we should reconsider. I've never seen anyone other than me use it anyways (I've looked at a gadzillion open source AVA test files). Some known problems this feature caused:
@vdemedes @novemberborn @sotojuan @jedrichards @istarkov @alexbooker @MarkHerhold @KidkArolis |
Relative imports are more cumbersome than relative file paths. I like setting |
Not sure why it would be. Usually there is a |
Ok, we're going to do this. |
We could add a |
You are right, that is the default npm behaviour. Okay then that is a good
|
Another bug caused by our current behavior: dtinth/babel-plugin-__coverage__#39 We should revert this for the next release. |
What's the status of this, changing cwd to the test file location is really annoying. |
It's blocked on having a migrate script: #32 (comment) The change itself is simple. |
@sindresorhus I don't think the codemod should block this issue. I just started using ava and I wasted a lot of time figuring out why hapi was not loading my models correctly in the tests. I initially blamed hapi-shelf (plugin to load bookshelf models), but after tracing the code I discovered this issue and easily fixed my test to work around it. |
Yeah, let's just do the change. It's hard to do a codemod for this correctly anyway, and I don't think it's important enough to force us to continue having surprising behavior for new users. @avajs/core ? |
👍 |
So is anybody working on this? I'm not volunteering, just doing a "ping" 😀 |
Fixed by 476c653. |
Currently the cwd is the directory you execute
ava
in, butava
can be executed from any upper level in the project and will search down for test files, so the cwd is pretty arbitrary and useless. I want to make the cwd actually useful.There are two options:
The reason I prefer the latter is that we run the test files hence IMHO the cwd is where we run it. It's like we would do:
cd tests && node test.js
.Currently you have to do this to be able to load a read a test relative file:
If we changed
process.cwd()
to the currently running test directory we could instead just do:Thoughts? Anything I'm missing?
The text was updated successfully, but these errors were encountered: