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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

easy no leak test cases #201017

Merged
merged 4 commits into from
Dec 15, 2023
Merged

easy no leak test cases #201017

merged 4 commits into from
Dec 15, 2023

Conversation

amunger
Copy link
Contributor

@amunger amunger commented Dec 15, 2023

@VSCodeTriageBot VSCodeTriageBot added this to the December / January 2024 milestone Dec 15, 2023
Tyriar
Tyriar previously approved these changes Dec 15, 2023
@amunger amunger enabled auto-merge (squash) December 15, 2023 21:13
@@ -36,6 +36,10 @@ suite('NotebookExecutionService', () => {
let kernelService: INotebookKernelService;
let disposables: DisposableStore;

teardown(() => {
Copy link
Contributor

@rzhao271 rzhao271 Dec 15, 2023

Choose a reason for hiding this comment

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

Instead of disposables as a separate variable, ensureNoDisposablesAreLeakedInTestSuite returns a disposable store to use. Using that function would also render this teardown section unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensureNoDisposablesAreLeakedInTestSuite isn't being called in this test yet. There's still a leak that @rebornix says he'll look into

@@ -34,6 +34,10 @@ suite('NotebookExecutionStateService', () => {
let disposables: DisposableStore;
let testNotebookModel: NotebookTextModel | undefined;

teardown(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of disposables as a separate variable, ensureNoDisposablesAreLeakedInTestSuite returns a disposable store to use. Using that function would also render this teardown section unnecessary.

@amunger amunger merged commit 8371e4a into main Dec 15, 2023
6 checks passed
@amunger amunger deleted the aamunger/noDisposableLeaks branch December 15, 2023 21:53
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants