Skip to content

Adopt ensureNoDisposablesAreLeakedInTestSuite in unit tests #190503

New issue
New issue
Closed
Closed
Adopt ensureNoDisposablesAreLeakedInTestSuite in unit tests#190503
Assignees
andreamah
Labels
debtCode quality issuesengineeringVS Code - Build / issue tracking / etc.
Milestone
November 2023
@connor4312

Description

@connor4312
connor4312
opened on Aug 15, 2023

Background

Changes in the order tests ran in caused spurious failures in a test that used ensureNoDisposablesAreLeakedInTestSuite. This was almost certainly a result of leaky tests further up the chain, but it's very hard to figure out which test it is. We should seek to adopt ensureNoDisposablesAreLeakedInTestSuite in the unit tests we own.

Adoption

ensureNoDisposablesAreLeakedInTestSuite is a utility that ensures that disposables created in a test are disposed of. This is very useful to find leaks in code you own.

If you have a setup and teardown blocks, make sure the function is called before the former and after the latter, like so:

suite('myCoolTests', () => {
  teardown(() => { /* ... */ });

  ensureNoDisposablesAreLeakedInTestSuite();

  setup(() => { /* ... */ });

  // ...tests
});
  • Else, simply call the function at the beginning of the suite:
suite('myCoolTests', () => {
  ensureNoDisposablesAreLeakedInTestSuite();

  // ...tests
});

The method also returns a store: DisposableStore you can use for convenience to dispose instances within your tests, like so:

suite('myCoolTests', () => {
  const store = ensureNoDisposablesAreLeakedInTestSuite();

  test('MyEditorWorks', () => {
    const editor = store.add(new CodeEditor());

    // ...assertions
  });
});

It will then emit an error in the "after each" step if there are any dangling disposables. The error shown will have a stacktrace to where the leaked disposable was created.

Sometimes, it will not be the specific disposable that wasn't disposed of, but the thing that created it. You will want to walk up the displayed stack to find where the leak is.

⚠️ As referenced, leaks in earlier tests can your tests to fail when the full suite is run in CI. If this is the case, comment out your ensureNoDisposablesAreLeakedInTestSuite. We can do a more efficient pass later as we uncomment the leak detector.

Ask

We have a lot of tests! Please spend some time in the next debt week adding this function call to your tests, prioritizing ones that do more complex setups such as those involving code editors. It's not expected that we'll get every test in the first pass, and we can start calling out individual tests in future items.

Metadata

Metadata

Assignees

Labels

debtCode quality issuesengineeringVS Code - Build / issue tracking / etc.

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions