hbenl / vscode-jasmine-test-adapter

Jasmine Test Adapter for the VS Code Test Explorer
MIT License
20 stars 20 forks source link

config requires trying to load relative to hbenl.vscode-jasmine-test-adapter-1.5.0 instead of workspace #34

Open samuelneff opened 5 years ago

samuelneff commented 5 years ago

I have a requires set in my Jasmine config file. This works fine when running Jasmine from the command line but fails when run within VSCode Jasmine Test Explorer.

Config:

{
    "spec_dir": "src",
    "spec_files": [
      "**/*.spec.ts"
    ],
    "requires": [
        "src/test-setup.ts"
    ]

Error:

[2019-08-05 15:33:52.465] [INFO] Worker: Caught error { Error: Cannot find module 'src/test-setup.ts'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:690:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at /Users/me/.vscode/extensions/hbenl.vscode-jasmine-test-adapter-1.5.0/node_modules/jasmine/lib/jasmine.js:101:5
    at Array.forEach (<anonymous>)
    at Jasmine.loadRequires (/Users/me/.vscode/extensions/hbenl.vscode-jasmine-test-adapter-1.5.0/node_modules/jasmine/lib/jasmine.js:100:17)
    at Jasmine.execute (/Users/me/.vscode/extensions/hbenl.vscode-jasmine-test-adapter-1.5.0/node_modules/jasmine/lib/jasmine.js:238:8)
    at Object.<anonymous> (/Users/me/.vscode/extensions/hbenl.vscode-jasmine-test-adapter-1.5.0/out/worker/loadTests.js:21:13)
    at Module._compile (internal/modules/cjs/loader.js:776:30) code: 'MODULE_NOT_FOUND' }

I've tried specifying ./src/test-setup.ts, tried using a plain .js file.

It does work if I hack in a relative path from vscode-jasmin-test-adapter directory to my project, but that would not necessarily be consistent across all team members. jasmine/../../../../../projects/my-project/src/test-setup.ts yuck ;)

Thanks!

Sam

hbenl commented 5 years ago

I am getting the same error, but I also get it when I run Jasmine from the command line. Which is not surprising, because Jasmine will simply require('src/test-setup.js'), which means it will try to find the src package in node_modules in the Jasmine install location and load the test-setup.js file in that package (which is obviously not what you want). Apparently require is patched to work differently in your project setup, otherwise this wouldn't work. Why do you not use the helpers property in jasmine.json? That property is for loading files from the project's directory.

hbenl commented 5 years ago

I just realized that using the requires property will only work with this adapter to load globally installed packages. I added a configuration option jasmineExplorer.jasminePath in version 1.6.0, if you set this option to "node_modules/jasmine", the requires property will also work for packages installed locally in your project's node_modules folder. Perhaps this will also solve this issue (even though you're using requires to load a file instead of a package).

apla commented 5 years ago

I added a configuration option jasmineExplorer.jasminePath in version 1.6.0, if you set this option to "node_modules/jasmine", the requires property will also work for packages installed locally in your project's node_modules folder

I have almost the same issue, and changing jasmineExplorer.jasminePath for every project is quite strange. Please change behaviour to automatic discovery of installed jasmine like this: run npm prefix, which gives you project's cwd, stat ${cwd}/node_modules/.bin/jasmine to ensure local jasmine installed and then set jasmineExplorer.jasminePath = "node_modules/jasmine" for vscode project? In this case every [node jasmine] project will run as intended.

Also, this extension can load package.json and if any scripts contains jasmine --config=test/.jasmine.json, then automatically set jasmineExplorer.config option.

I can make pull request for automatic discovery of options.

hbenl commented 5 years ago

@apla Makes sense. I'm just not 100% sure yet how to do that auto-detection:

What do you think about this auto-detection strategy?

apla commented 5 years ago

Looks good to me.

Some notes:

I'd like to also detect a globally installed jasmine package

Just checked, I have 3 years old jasmine installed globally (2.5.2), just because installed it someday and don't care anymore. Probably, this is should be optional step, which ends in jasmine selection dialog for user.

require.resolve('jasmine') should work for projects, where jasmine is already installed as dependency, but log should contain message about jasmine path. Or funny stuff will happen:

npm install not executed + 3 years old jasmine = high probability of strange errors

get all npm scripts and split them if they're containing &&

Actually, I have same idea when I'm writing my previous message, but scripts can be launched in subshells like $(cmd), launched on error command x && command y || command z, or even in parallel cmd1 & ; cmd2 & ; cmd3. Probably regex like /jasmine\s[^|&;]*--config=(.*?)(?:\s|$)/ will work better than parsing all shell madness.

hbenl commented 5 years ago

npm install not executed + 3 years old jasmine = high probability of strange errors

Good point! So maybe the autodetection should just look for node_modules/jasmine after all? That should cover most setups and if someone likes to work with a globally installed jasmine then he can set jasmineExplorer.jasminePath in his user settings (so he won't have to set it in each project individually).

Probably regex like /jasmine\s[^|&;]*--config=(.*?)(?:\s|$)/ will work better than parsing all shell madness.

Yeah, trying to handle all possible shell madness is futile, we just need to find something that is simple and works in as many projects as possible. But since the config autodetection will never be 100% reliable I think it should be optional (e.g. set "jasmineExplorer.config": "auto" in your workspace or user settings to enable it), whereas the jasmine autodetection could be the default.

apla commented 5 years ago

Good point! So maybe the autodetection should just look for node_modules/jasmine after all?

For old scripting languages like perl, python, ruby with globally installed deps it's important to look into globally installed modules. nodejs completely changed that with npm/nvm and having projects self-contained. So, for nodejs, project folder is a source of truth. Jasmine npm package gives user a choice to install for project as dev dependency or globally and user should select on it's own. For cases where jasmine installed globally and maintained manually, you have jasmineExplorer.jasminePath.

But since the config autodetection will never be 100% reliable

Nothing is reliable 100% ;-) If things works for 3% of users and won't work for 0.01%, that's good.

My problem here is many different projects (about 17) with different configs. I already have most things covered with jasmine configurations, but it is painful to configure each project differently in the editor even if it's already configured with jasmine configuration.