pact-foundation / pact-js

JS version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
https://pact.io
Other
1.59k stars 343 forks source link

Jest detecting memory leak on require of @pact-foundation/pact #1033

Open Maxim-Filimonov opened 1 year ago

Maxim-Filimonov commented 1 year ago

I have been investigating memory leak in our project and found that some of it comes from @pact-foundation/pact at least according to jest.

Software versions

Please provide at least OS and version of pact-js

Issue Checklist

Please confirm the following:

Reproduction repo

Expected behaviour

Jest shouldn’t be detecting leaks when requiring pact-foundation.

Actual behaviour

Jest detect memory leaks when not even using pact - only requiring it

Steps to reproduce

  1. Go to reproduction repo
  2. Run npm install
  3. Run npm test
  4. See jest reporting leaks:

    FAIL  ./leak.test.js
    ● Test suite failed to run
    
    EXPERIMENTAL FEATURE!
    Your test suite is leaking memory. Please ensure all references are cleaned.
    
    There is a number of things that can leak memory:
      - Async operations that have not finished (e.g. fs.readFile).
      - Timers not properly mocked (e.g. setInterval, setTimeout).
      - Keeping references to the global scope.
    
      at onResult (node_modules/@jest/core/build/TestScheduler.js:150:18)
          at Array.map (<anonymous>)
Maxim-Filimonov commented 1 year ago

I added some extra tests to the reproduction repo. It looks like it has something to do with pino-pretty transport configuration. Looks like it might need separate issue in pact-core. Any thoughts?

TimothyJones commented 1 year ago

First off, thank you SO MUCH for the repo!

Secondly, I'm not hugely surprised that pino is a problem. Probably we should just replace pino with something else. Pact doesn't need a fully functional logger, just something that prints out timestamps, version numbers, and strings.

mefellows commented 1 year ago

Thanks Maxim - this is awesome!

Secondly, I'm not hugely surprised that pino is a problem. Probably we should just replace pino with something else. Pact doesn't need a fully functional logger, just something that prints out timestamps, version numbers, and strings.

Exactly. I think this is the best approach to resolve this, albeit if it's a problem with Pino we ought to raise an upstream issue there also. I'll pick that job up when we get to this.

github-actions[bot] commented 11 months ago

👋 Hi! The 'smartbear-supported' label has just been added to this issue, which will create an internal tracking ticket in PactFlow's Jira (PACT-1255). We will use this to prioritise and assign a team member to this task. All activity will be public on this ticket. For now, sit tight and we'll update this ticket once we have more information on the next steps.

See our documentation for more information.