-
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
Configure unit tests #3590
Configure unit tests #3590
Conversation
src/repositories.ts
Outdated
|
||
const slash = 47; //CharCode.Slash; | ||
const slash = CharCode.Slash; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest leaving this as 47
-- bundlers don't handle the const enum
properly and inject the value at the call site, instead they end up with this whole module/object lookup indirection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. is it the problem of project structure? I mean could we store one file for each enum with default export instead? I know it can be inconvenient now, it's just a clarification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced
src/system/path.ts
Outdated
|
||
export { basename, dirname, extname, join as joinPaths } from 'path'; | ||
|
||
const slash = 47; //slash; | ||
const slash = CharCode.Slash; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced
src/system/trie.ts
Outdated
|
||
const slash = 47; //CharCode.Slash; | ||
const slash = CharCode.Slash; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this split out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it broke the tests running
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I can't remember how I found it first time, but I checked it now, running yarn test
without the splitting leads to the error
Mini-developer:vscode-gitlens developer$ yarn test
yarn run v1.22.22
$ yarn run build:tests
$ tsc -p tsconfig.test.json && tsc-alias -p tsconfig.test.json
$ vscode-test
✔ Validated version: 1.93.1
✔ Found existing install in /Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1
[main 2024-09-19T06:25:25.870Z] update#setState disabled
[main 2024-09-19T06:25:25.873Z] update#ctor - updates are disabled by the environment
2024-09-19 13:25:26.624 Code Helper (Renderer)[25737:9616656] CoreText note: Client requested name ".NewYork-Regular", it will get TimesNewRomanPSMT rather than the intended font. All system UI font access should be through proper APIs such as CTFontCreateUIFontForLanguage() or +[NSFont systemFontOfSize:].
2024-09-19 13:25:26.624 Code Helper (Renderer)[25737:9616656] CoreText note: Set a breakpoint on CTFontLogSystemFontNameRequest to debug.
Started local extension host with pid 25762.
Loading development extension at /Users/developer/vscode-gitlens
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/developer/vscode-gitlens/node_modules/fast-string-truncated-width/dist/index.js from /Users/developer/vscode-gitlens/out/system/string.js not supported.
Instead change the require of index.js in /Users/developer/vscode-gitlens/out/system/string.js to a dynamic import() which is available in all CommonJS modules.
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/system/string.js:39:55)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/git/models/reference.js:28:18)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/env/node/git/git.js:43:21)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/env/node/providers.js:33:15)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/container.js:34:21)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/git/gitUri.js:17:21)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/repositories.js:8:18)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at Object.<anonymous> (/Users/developer/vscode-gitlens/out/system/__tests__/trie.test.js:34:24)
at c._load (node:electron/js2c/node_init:2:13801)
at y._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:184:6031)
at i._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:181:32144)
at n._load (/Users/developer/vscode-gitlens/.vscode-test/vscode-darwin-1.93.1/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:146:34326)
at /Users/developer/vscode-gitlens/node_modules/mocha/lib/mocha.js:416:36
at Array.forEach (<anonymous>)
at Mocha.loadFiles (/Users/developer/vscode-gitlens/node_modules/mocha/lib/mocha.js:413:14)
[main 2024-09-19T06:25:28.047Z] Extension host with pid 25762 exited with code: 0, signal: unknown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, remembered. mocha can't import from 'vscode', so if you need to test any module that imports from vscode, you should think about it's mock. I tried different ways to mock in mocha, but it didn't work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used fork
https://github.com/nzaytsev/fast-string-truncated-width
@gk-nzaytsev/[email protected]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this split out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment above
webpack.config.js
Outdated
@@ -347,6 +347,7 @@ function getWebviewsConfig(mode, env) { | |||
cache: true, | |||
cacheLocation: path.join(__dirname, '.eslintcache', 'webviews/'), | |||
cacheStrategy: 'content', | |||
// uses config file src/webviews/apps/.eslintrc.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this comment for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary, I'll remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
.github/workflows/unit-tests.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make these a "check" on the PR, so it can't be merged until they pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of course, I wanted to do this. but I need admin permission to GH for this. I tested on my test repos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f876184
to
0005bcf
Compare
e9cb6b1
to
3ba8253
Compare
- Stored in __tests__ subdirectories - Adds sample tests for iterable and color - Adds restriction for using .only tests suites
3ba8253
to
3c8cee6
Compare
"module": "CommonJS", | ||
"moduleResolution": "Node", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happened to see this PR in the release notes, and skimmed through it as I was curious 🙃.
Is there a reason to use the "Node" moduleResolution? https://www.typescriptlang.org/tsconfig/#moduleResolution (It's deprecated in favor of the equivalent Node10, which is itself discouraged)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Checklist
Fixes $XXX -
orCloses #XXX -
prefix to auto-close the issue that your PR addresses