Dispose instances of test instantiation service #187482
Conversation
Pull request was converted to draft
|
@sandy081 A bunch (all?) of the |
|
Same for |
|
Instead of restoring on dispose, is it possible to hook at the end of each test file and restore? |
|
I did a new change that restore the stubs after every test file - #187496 - I ran some builds with this change enabling unit tests in windows 32 bit and they seem to run. But there is one unit test that is failing consistently in Mac. |
| @@ -129,6 +129,10 @@ export class TestInstantiationService extends InstantiationService { | |||
| override createChild(services: ServiceCollection): TestInstantiationService { | |||
| return new TestInstantiationService(services, false, this); | |||
| } | |||
|
|
|||
| dispose() { | |||
| sinon.restore(); | |||
There was a problem hiding this comment.
I think calling restore on sinon here might not be correct as this will restore all sinon stubs that this service does not own.
After discussing in standup the outcome was that we should properly clean up after each test/test suite and should not use a hook to restore after every test file. |
This will call `sinon.restore()` and prevent a memory leak Part of #187471
alexr00
force-pushed
the
alexr00/disposeTestInstaService
branch
from
4910d9c
to
67d35f8
Compare
| suite('DecorationAddon', async () => { | ||
| suite('DecorationAddon', () => { | ||
| let decorationAddon: DecorationAddon; | ||
| let xterm: any; |
There was a problem hiding this comment.
@meganrogge and @Tyriar I know this any is not ideal, but having the async suite here causes the async code to run while other tests are running and can interfere with the other tests.
There was a problem hiding this comment.
Change looks good, you should be able to import the type like this still though:
| suite('DecorationAddon', async () => { | ||
| suite('DecorationAddon', () => { | ||
| let decorationAddon: DecorationAddon; | ||
| let xterm: any; |
There was a problem hiding this comment.
Change looks good, you should be able to import the type like this still though:
* Dispose instances of test instantiation service This will call `sinon.restore()` and prevent a memory leak Part of microsoft#187471 * Fix interfering terminal tests * Remove `async` from terminal test suite * Fix `any`
This will call
sinon.restore()and prevent a memory leakPart of #187471