Support symlinked test files - fixes #1143 #1145
Conversation
|
Hmm it seems that it fails the test for Windows. I'm not sure how to handle the symlink in it. Edit: it passes all of the test in Windows with the exception of my new test to check whether the symlink is working or not. |
|
After some research, I think what I could do is I can add check to the test not to be run (or to be automatically passed) if the environment is detected to be Windows. But I'm not sure if it's the right approach considering Windows has no symlink support. |
There was a problem hiding this comment.
After some research, I think what I could do is I can add check to the test not to be run (or to be automatically passed) if the environment is detected to be Windows. But I'm not sure if it's the right approach considering Windows has no symlink support.
It does, actually! http://stackoverflow.com/questions/11662868/what-happens-when-i-clone-a-repository-with-symlinks-on-windows has details. I've pushed a commit to your branch which enables symlink support in our AppVeyor builds.
PR looks good, would just like the test to be more explicit.
|
|
||
| var api = apiCreator(); | ||
|
|
||
| return api.run([path.join(__dirname, 'fixture/symlink')]) |
There was a problem hiding this comment.
What you're passing here are patterns that the API then resolves to actual test files. In this case the pattern is a symlink, and the API needs to resolve tests within the directory it's pointing at.
Could you add another test where you provide a symlink to a test file? The API should use that path as-is, whereas if you provide a (symlinked) directory other code gets in between which may decide to resolve the path for us.
|
Thanks for adding that support for Windows! On the other hand, there is a new problem, when I added new direct symlink to a test file in the test, it fails to recognise that file exist. It throws The following commit shows the problem. It stops before even calling precompilation and |
|
So I copied the same tests to the ava master branch. Surprisingly it failed all of the tests including the one that points to directory. In a sense this fix fixes problem #1143 #1137 as the user is trying to do a symlink to a directory containing test files. But then it also uncovers that direct symlink is somehow can't be resolved. (Maybe it has something to do with how files are searched in ava-files) |
You need to make sure it has the Let's keep the test for symlinked directories, but also add one for symlinked files. |
|
Hi I just added the last commit with the requested change. (My bad for not researching that .js part) Is there anything else I should do to get the PR accepted? Thanks. |
sindresorhus
changed the title
|
Looks good to me too. Nice work @rnkdev :) |
|
@rnkdev Thank you so much for your work 👍 |
Fixes #1143
Following from the bug thread, I modified api.js hashmap preparation so that it uses resolved path as the key. I also added one test to ensure that ava can now be ran with linked test file.