Notifier with --watch uses 100% CPU on Mac and notifier processes never terminate

[jackkinsella]

🐛 Bug Report

I noticed that when running the jest unit testing library with --watch --notify (which calls node-notifier), my CPU usage soon went up to 90%, due to multiple instances of node-notifier hanging around. There are a couple of other people with the same issue over on in the jest repo.

Here's an example process that was spinning:

./myapp/node_modules/node-notifier/vendor/mac.noindex/terminal-notifier.app/Contents/MacOS/terminal-notifier -actions Run again,Exit tests -closeLabel "Close" -message "-&M-T️ 1 of 29 tests failed" -title "4% Failed" -ap

Here's is a log of a single iteration of the repeated system calls — it seems to be unable to unlock a semaphore: system-calls.log

I experimented around with the node-notifier library's API and noticed that if I passed the same configuration options to node-notifier as jest does, the program never ends. To see what I mean, run the following in a stand-alone program. You'll notice it displays the notification but the program keeps running indefinitely.

// reproduceIssue.js
// Usage: node reproduceIssue.js

const notifier = require('node-notifier')
notifier.notify(
  {
    actions: [ 'Run again', 'Exit tests' ],
    closeLabel: 'Close',
    message: '⛔️ 1 of 9 tests failed',
    title: '12% Failed'
  }, () => {}
)

However, when I removed the actions attribute, the program ends after the notification is shown. I could also get the program to end, even with actions, by adding an explicit timeout attribute:

const notifier = require('node-notifier')
notifier.notify(
  {
    actions: [ 'Run again', 'Exit tests' ],
    closeLabel: 'Close',
    // new code
    timeout: 5,
    // end new code
    message: '⛔️ 1 of 9 tests failed',
    title: '12% Failed'
  }, () => {}
)

I brought up this issue with the maintainer of node-notifier and both of us think it would be best if the jest team considered adding a timeout here https://github.com/facebook/jest/blob/master/packages/jest-reporters/src/notify_reporter.ts#L108-L122

To Reproduce

MacOS 10.14.3
Node 10.14.1
node-notifier@5.4.1
jest@24.8.0

Make sure one test fails then run:

$ jest --watch --notifier

Next run top -o cpu — you should see a terminal-notifier process that never ends.

Expected behavior

The number of terminal-notifier processes should not increase towards infinity with each notification, and CPU usage should be negligible.

[SimenB]

PR very much welcome adding a timeout 🙂

[SimenB]

Fixed and released

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.