Jest not exiting on failure with notify true

[th3fallen]

Do you want to request a feature or report a bug? Bug

What is the current behavior? Tests do not exit on failure with notify set to true

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

can be reproduced in any test setup, if you dont already have notify true just run a failing test with jest {pathtotest} --notify and observe on failure the process does not exit

What is the expected behavior? test should exit regardless of notify flag

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

Config

"jest": {
    "modulePaths": [
      "app"
    ],
    "coverageThreshold": {
      "global": {
        "statements": 85,
        "branches": 70,
        "functions": 80,
        "lines": 85
      }
    },
    "coverageDirectory": "<rootDir>/reports/coverage",
    "collectCoverageFrom": [
      "app/**/*.js",
      "!app/tests/**",
      "!app/shared/vendor/**",
      "!app/main.js"
    ],
    "notify": true,
    "coverageReporters": [
      "lcov",
      "text-summary"
    ],
    "setupFiles": [
      "<rootDir>/scripts/testSetup.js"
    ],
    "snapshotSerializers": [
      "enzyme-to-json/serializer"
    ],
    "moduleNameMapper": {
      "^.+\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/__mocks__/fileMock.js",
      "^.+\\.(css|scss)$": "<rootDir>/__mocks__/styleMock.js"
    }
  },

Enviro Information

OS: OSX 10.12.4 beta
Node: 6.10.0
NPM: 4.4.2
Yarn: 0.21.3

[thymikee]

I think it's made on purpose, because you may want to run the tests once more. See the messaging:

cc: @mikaelbr

[th3fallen]

@thymikee oh im aware of the messaging in the notification, my discrepancy is that it used to just exit, it would be nice if that could be re-enabled with a flag or what have you.

[thymikee]

If we exit the process, then these messages under "Show" won't make sense. Wonder how can we improve it. Get rid of it completely and only leave "Close"?
Ideas welcome!

[th3fallen]

My initial thought was to have a way to change the notification type i.e.

notifyType: 'simple|actionable' with a default to actionable if notify is true, and with simple would get a notice like before with just the message with no actions and the process to exit.

Not sure if that's the best idea but I'll miss having the notification popup because what I do is trigger a test run and go back to working and when I see the notice I address it based on the content. As using those buttons is much slower than just hitting up and enter in my terminal for me.

[thymikee]

Then I strongly recommend the watch mode 😄. I like the idea of notifyType, but just thinking it can be too much. I wonder if anybody uses "Run again" option from notifications.

[th3fallen]

watch mode obliterates my cpu for some reason 😭 , and the goal with my suggestion was a sane default that would only be changed if a user saw fit.

[cpojer]

@mikaelbr did you have any suggestions for this?

[th3fallen]

on this topic, i think there may be a memory leak with the terminal-notifier on osx if you run a failing test and forget about it and leave it running for ~5m it will eat up a TON of ram, hit 6gb on my machine before i noticed and nixed it.

[mikaelbr]

@th3fallen Yeah, there's a pretty bad memory leak in the terminal-notifier action implementation as of now. I've done some workaround with timeouts to temporary avoid this, but will do a permanent fix once this is better (julienXX/terminal-notifier#173). The workaround should be in the latest version of node-notifier. What version are you using?

Regarding Jest not exiting, it should exit after a timeout period. This might be related to the memory leak issue as mentioned above, and if it doesn't timeout (no activation of the notification), jest should exit. Could you please verify node-notifier version and check again?

I don't know if notification type, really is necessary. It'd definitely be possible, and I'm not entirely against it, but I think simplicity of the API is better. And I think with the timeout working properly (which it should either with the memory leak fixed, or the workaround as mentioned), Jest should stop eventually.

[th3fallen]

npm info node-notifier has the following... version: '5.1.2', misspoke
version is 5.0.2 from jest 19.0.2

[mikaelbr]

Ah. There you have it. The temporary workaround is in the latest version, using that should avoid this issue. May be worth bumping version in jest.

[th3fallen]

yeah i'd agree it should definitely be bumped in jest imo

[clawconduce]

This doesn't work for me, but I did find the culprit and have a 1 liner workaround I can put into a PR after getting the CLA signed.

See mikaelbr/node-notifier#218

If the node-notifier issue is fixed, then no update to jest is required.

[Frondor]

This is still a problem in "^23.6.0"