Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notifications stopped #9701

Closed
millette opened this issue Mar 25, 2020 · 13 comments 路 Fixed by #10400
Closed

Notifications stopped #9701

millette opened this issue Mar 25, 2020 · 13 comments 路 Fixed by #10400

Comments

@millette
Copy link

millette commented Mar 25, 2020

Could be related to #9567

馃挜 Regression Report

On Debian GNU/Linux, jest --notify stopped working, same with a config:

{
  "notify": true
}

Last working version

25.1.0

Worked up to version:

I'm new to jest, I only used 25.1.0 before.

Stopped working in version:

25.2.0

To Reproduce

I also installed node-notifier manually (v5 and v6) but it didn't change anything.

Steps to reproduce the behavior:

jest --notify

Expected behavior

I should see a desktop notification.

Link to repl or repo (highly encouraged)

Issues without a reproduction link are likely to stall.

Run npx envinfo --preset jest

  System:
    OS: Linux 5.4 Debian GNU/Linux 10 (buster) 10 (buster)
    CPU: (4) x64 Intel(R) Celeron(R) CPU  J1900  @ 1.99GHz
  Binaries:
    Node: 12.16.1 - ~/n/bin/node
    Yarn: 1.22.4 - /usr/bin/yarn
    npm: 6.14.3 - ~/n/bin/npm

@millette
Copy link
Author

When I compare my local (node_modules) @jest/reporters/package.json with the 25.2.0 release I can still see node-notifier in the dependencies locally, but it's an optionalDependencies only in the release (accoring to github).

@mpareja
Copy link

mpareja commented Apr 8, 2020

I'm using Ubuntu and have tracked this issue down to this change: 00e9e09#diff-020e0df875c9a11b515012594dd609b4R120.

The timeout option is being set to false which is not a valid option for notify-send.

@millette
Copy link
Author

millette commented Apr 8, 2020

@mpareja Thanks for having a look. I only started using jest so I'm really not in a position to investigate. Just now, I changed (in 25.1.0) timeout: 10 to timeout: 0 (instead of false) and I received a notification. Wondering if changing the false in later versions to 0 would work.

@mpareja
Copy link

mpareja commented Apr 9, 2020

I agree with your initial assessment that this is a regression. I'll leave it up to the React team to decide what to do about it. I just wanted to make their job easier by pointing them in the direction of a root-cause.

@Smolations
Copy link

I can confirm the same thing for Mac, though it might have been related to a software update. In any case, they aren't working for me anymore. FWIW, I also tried using node-notifier locally in an isolated script and still couldn't get them working, so might be an issue there (related or not). 馃榿

@ildella
Copy link

ildella commented Aug 9, 2020

According to latest node-notifier docs https://www.npmjs.com/package/node-notifier

    wait: false, // Wait for User Action against Notification or times out. Same as timeout = 5 seconds
 
    // New in latest version. See `example/macInput.js` for usage
    timeout: 5, // Takes precedence over wait if both are defined.

Note: The wait option is shorthand for timeout: 5. This just sets a timeout for 5 seconds. It does not make the notification sticky!

@ildella ildella mentioned this issue Aug 9, 2020
@SimenB
Copy link
Member

SimenB commented Aug 10, 2020

As of Version 6.0 there is a default timeout set of 10 to ensure that the application closes properly. In order to remove the timeout and have an instantly closing notification (does not support actions), set timeout to false. If you are using action it is recommended to set timeout to a high value to ensure the user has time to respond.

So false is valid according to the docs (and should give the behaviour we want). This sounds more like a bug in node-notifier, /cc @mikaelbr

@mikaelbr
Copy link
Contributor

mikaelbr commented Aug 10, 2020

I'll look into it! 馃憤

@mikaelbr
Copy link
Contributor

Could anyone with a repro test this branch/PR and verify the fix? mikaelbr/node-notifier#341 I'm afraid I don't have easy access to the environment right now to reproduce the issue myself.

@SimenB
Copy link
Member

SimenB commented Aug 10, 2020

@millette @mpareja could you test? 馃檹

@mpareja
Copy link

mpareja commented Aug 12, 2020

@SimenB @mikaelbr @millette I've confirmed the changes in mikaelbr/node-notifier#341 resolve the problem, thank you!

@SimenB
Copy link
Member

SimenB commented Aug 20, 2020

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants