-
Notifications
You must be signed in to change notification settings - Fork 294
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
test-provider: adjust workspace location handling #972
base: master
Are you sure you want to change the base?
Conversation
The VSCode reported workspace path is unresolved (microsoft/vscode#18837). When the repository is hosted in a path substituted path (e.g. `subst X: C:`), the test path will not be properly translated as the drive is different. This may potentially also impact paths which are hosted within a NT junction. Explicitly resolve the workspace path to match the fact that `jest` will report paths which have been resolved to the real path.
@connectdotz thanks for approving the CI run. I'm not sure how to make progress here. The failure on Ubuntu is particularly odd. Would you happen to have any suggestions? |
@compnerd, thanks for the PR. Regarding the test, you would need to mock I have a few questions and am not so sure if we should make this change across the board just yet:
|
My pleasure :)
I think that resolving the questions is something we should do before addressing the tests and linter. At the very least the commit message needs to be amended. This is not for a symlinked project, but for the case where the project itself is checked out to the moral equivalent of what on Linux would amount to a bind mount. On Windows you can create a drive from an existing path via the The reason that I am locally doing this resolution explicitly rather than at the stored path (which would be computationally better as it avoids unnecessary re-computation), is that while VSCode has opted to not perform the path resolution, the Jest command line tool will list the test locations with the resolved path. The result is that the path prefix will not match and we will fail to compute a project relative path.
The big thing here is a difference in behavior between jest and VSCode. The jest tool reports the location of the tests using the resolved path while VSCode reports the path using the unresolved path.
We will now resolve the path for the project, however, given that jest will report the resolved path, in either case this should be an improvement as we will now properly identify the repository relative path for the tests.
So far the one thing that I immediately noticed trying to use the extension was that the tests were reported being nested under each path arc where the source tree was located in the resolved path rather than where VSCode was pointed to as the root of the project. It was frustrating enough that I didn't spend much time exploring the rest of the extension as the path is rather long and so having ~10 directories to iterate through to see the result was off-putting (which also unnecessary right shifts the entry). |
Sorry for the delayed response. This issue is interesting and I would like to play with it further to see if there is any other implication. If you have time to explore it further that would be great: to ensure everything else works, and this is the only place that needs to be changed. If not, that's fine, too; I will probably get back to this after the next release. Thanks for your patience. |
From my testing for the functionality, it seemed to correct the issues that I was having with it. |
@connectdotz Hi, just following up on this, have you had a chance to play with this any further? |
@compnerd, yes, I have been thinking about this. Do you mind trying the extension with different I think we could do this with a new setting, let's say |
@connectdotz playing around with the settings, it seems that everything works as expected with the changes. |
The VSCode reported workspace path is unresolved
(microsoft/vscode#18837). When the repository is hosted in a path substituted path (e.g.
subst X: C:
), the test path will not be properly translated as the drive is different. This may potentially also impact paths which are hosted within a NT junction. Explicitly resolve the workspace path to match the fact thatjest
will report paths which have been resolved to the real path.