(test): fix glob mode match and remove duplicate loader.js in renderer.html #183507
Conversation
| // During unit testing, it may be empty and needs. | ||
| viewDescriptorService.getDefaultViewContainer(ViewContainerLocation.Sidebar)?.id || '', |
There was a problem hiding this comment.
Why is this needed? Are any tests currently failing because of this?
There was a problem hiding this comment.
Why is this needed? Are any tests currently failing because of this?
Now the existing unit tests in VS Code are not executed into the SidebarPart constructor, so this writing (use ! rather than ? ) does not cause unit test execution to fail. The reason I encountered unit test failure is that I did not use TestSidebarPart , but directly use SidebarPart.
When I need to initialize SidebarPart, it will report an error because I haven't registered any other views at the time of unit testing.
Another reason is that other modules are fetched using optional chain processing, because there is no guarantee that there will be a default view container.
| .describe('build', 'run with build output (out-build)').boolean('build') | ||
| .describe('run', 'only run tests matching <relative_file_path>').string('run') | ||
| .describe('glob', 'only run tests matching <pattern>').string('glob') |
There was a problem hiding this comment.
This should be runGlob to match our other unit test types, and be described as "only run test files matching..."
There was a problem hiding this comment.
This should be
runGlobto match our other unit test types, and be described as "only run test files matching..."
emmmm, but in
Line 24 in 89db3e4
vscode/test/unit/browser/index.js
Line 84 in 89db3e4
--glob. Has it been abandoned?
There was a problem hiding this comment.
It looks like in the electron test we allow both glob and runGlob, so I guess that's fine. I would add a similar .alias('glob', 'runGlob') just for consistency's sake
Head branch was pushed to by a user without write access
40181e3 to
34f628c
Compare
|
has been merged the latest code from main, Can you help me look at the code again when you are free? @roblourens @connor4312 |
|
These files were reworked recently, I reapplied the necessary change in #199979 |
Close #183506