Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for pnpm round 2 #1046

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

robmcguinness
Copy link

Credit to original PR #1033. This includes unit tests.

  • Followed instructions for code --extensionDevelopmentPath=your-local-vscode-jest and seemed to work fine 馃馃徏
  • yarn test passed
  • yarn vscode:prepublish seemed to work

Closes #974

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5497229823

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 97.755%

Totals Coverage Status
Change from base Build 5492310856: 0.001%
Covered Lines: 3544
Relevant Lines: 3564

馃挍 - Coveralls

Copy link
Collaborator

@connectdotz connectdotz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robmcguinness Thanks for the PR. It looks good.

Before we go ahead with this, just want to double-check that you have tested it with a pnpm project without the debug config in launch.json, and you were able to use the plugin to generate a default config to debug the test. That means you should see this message in the test terminal when you debug the test:
No debug config named "vscode-jest-tests.v2" or "vscode-jest-tests" found in launch.json, will use a default config.

@robmcguinness
Copy link
Author

@connectdotz seems like there is more work todo. I tried using plugin to generate default config for debugging. Here are the results with pnpm:

./node_modules/.bin/jest --runInBand --watchAll=false --testNamePattern "" --runTestsByPath "" 
Debugger attached.
Waiting for the debugger to disconnect...
[workspace root]/node_modules/.bin/jest:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^

SyntaxError: missing ) after argument list
    at internalCompileFunction (node:internal/vm:73:18)
    at wrapSafe (node:internal/modules/cjs/loader:1176:20)
    at Module._compile (node:internal/modules/cjs/loader:1218:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47

Reference article that had similar problem https://dev.to/elpddev/debug-jest-spec-in-vscode-error-missing-after-argument-list-1p3b

I'll try and dig further.

@connectdotz
Copy link
Collaborator

Thanks for digging, a few thoughts:

  1. the --testNamePattern and --runTestsByPath are all blank, which doesn't seem right. The extension should fill the variables with the actual test and testFile names accordingly if the test config is properly named: vscode-jest-tests.v2, for example... you can take a look at the code here, which does the filling/replacing the variables above.
  2. If you save the default config to launch.json and replace the command with the one suggested by the link you posted above... did it work?

@seanpoulter
Copy link
Member

seanpoulter commented Mar 2, 2024

@robmcguinness Thanks for the PR. It looks good.

Before we go ahead with this, just want to double-check that you have tested it with a pnpm project without the debug config in launch.json, and you were able to use the plugin to generate a default config to debug the test. That means you should see this message in the test terminal when you debug the test: No debug config named "vscode-jest-tests.v2" or "vscode-jest-tests" found in launch.json, will use a default config.

Hi @connectdotz, it's been a while. Let me know if you'd want to write a test for this. I've recently used the WebdriverIO VS Code Testing Service to test another extension and could set that up pretty quickly (in a separate Issue and PR).

@connectdotz
Copy link
Collaborator

@seanpoulter, nice to see you here! 馃槂

I dropped the ball on this issue, it would be great if you could test this against a pnpm workspace! Thanks!

@robmcguinness
Copy link
Author

Sorry I have not been able to circle back on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pnpm run test -- is not handled as a package script when specified in jestCommandLine
4 participants