andywer / leakage

πŸ› Memory leak testing for node.
1.58k stars 52 forks source link

Testing code with promises #30

Closed karpikpl closed 6 years ago

karpikpl commented 6 years ago

Hey, I'm trying to test code that uses promises and always getting the MemoryLeakError. Is there anything I'm doing wrong, or is that just how promises work?

here's my code:

const Leakage =  require('leakage');

it('leak test', async() => {
    let i = 0;
    jasmine.DEFAULT_TIMEOUT_INTERVAL = 100000;
    expect.hasAssertions();

     await Leakage.iterate.async(async () => {
        let dummyPromise = new Promise((resolve) => {
           setTimeout(() => resolve('ok ' + ++i), 1);
        });

        try {
            const result = await dummyPromise;
            expect(result).toBeTruthy();
        } catch (e) {
            throw 'this should not happen ' + e;
        }
    });
})

and very minimalistic package.json

{
  "name": "promise-leak",
  "version": "1.0.0",
  "main": "index.js",
  "scripts": {
      "test": "jest --runInBand"
  },
  "license": "MIT",
  "dependencies": {
    "jest": "^23.6.0",
    "leakage": "^0.4.0"
  }
}
andywer commented 6 years ago

Hey @karpikpl!

Your code looks fine so far. Haven't ever tested the lib with Jasmine, though.

Can you please post the exact error output and ideally a heap dump (maybe in a GitHub gist or so)?

karpikpl commented 6 years ago

So I have good news and bad news. Good news is that the test is passing with mocha but failing with jest.

Same test, just different testing framework:

const Leakage =  require('leakage');
var assert = require('assert');

describe('leaks', function() {

    let expected, actual;

    beforeEach(function() {
        expected = 0;
        actual = 0;
    });

    afterEach(function() {
        if (!expected || expected == actual) return;
          var err = new Error('expected ' + expected + ' assertions, got ' + actual);
          this.currentTest.emit('error', err);
    });

    it('leak test', function() {
        this.timeout(100000);
        expected = 181;

         return Leakage.iterate.async(async () => {
            let dummyPromise = new Promise((resolve) => {
               setTimeout(() => resolve('ok'), 1);
            });

            try {
                const result = await dummyPromise;
                assert.equal(result, 'ok');
                actual++;
            } catch (e) {
                throw 'this should not happen ' + e;
            }
        });
    })
});

With mocha it passes and it's also much faster - 5s.

With Jest it fails but it's also much slower

$ jest --runInBand
 FAIL  tests/test.js (26.959s)
  ● leak test

    MemoryLeakError: Heap grew on 6 subsequent garbage collections (180 of 180 iterations) by 362 kB.

      Iterations between GCs: 30

      Final GC details:
      [   30.3 kB] [+ 378x] [-   2x] system / Context
      [   23.4 kB] [+ 378x] [-   1x] Closure
      [   21.9 kB] [+ 103x] [-   4x] Array
      [    3.6 kB] [+  31x] [-   1x] Timeout
      ... (9 more)

      at testConstantHeapSize (node_modules/leakage/lib/testConstantHeapSize.js:24:12)
      at Object.onAllDone (node_modules/leakage/lib/index.js:130:27)
      at Immediate.onHeapDiff (node_modules/leakage/lib/index.js:114:16)

I was unable to pass the --heap-file heap-diff.json param to jest. It worked with mocha, but mocha doesn't have the issue

andywer commented 6 years ago

Thanks for the comprehensive update! πŸ‘πŸ™‚

I suppose we can turn that into one actionable only: Document that you should use tape or mocha; test runners that come with a minimal overhead and donβ€˜t do much magic.