Add package name to NotifyReporter notification #5898
Conversation
| @@ -22,6 +23,8 @@ const isDarwin = process.platform === 'darwin'; | |||
|
|
|||
| const icon = path.resolve(__dirname, '../assets/jest_logo.png'); | |||
|
|
|||
| const readFile = util.promisify(fs.readFile); | |||
There was a problem hiding this comment.
Not available in node 6, please use pify
There was a problem hiding this comment.
Disregard this, shouldn't need fs here at all?
| 'package.json', | ||
| ); | ||
| const {name: packageName = this._globalConfig.rootDir} = JSON.parse( | ||
| await readFile(packagePath), |
There was a problem hiding this comment.
This file should be available in hastefs already, should not be necessary to hit the file system
There was a problem hiding this comment.
I'll check it out.
|
This sounds great! Do you have a screenshot of the new notification? |
|
@SimenB I've rewritten the code a bit using hastefs. I don't think this change has anything to do with @rickhanlonii I've updated the PR description with screenshots. |
|
What do you think about leading with the percentage:
|
|
And yeah, the node 9 failure is unrelated 👌 |
|
Sorry about the silence, forgot about this one! If you rebase on master, the test failure should go away. This needs a test as well :) |
|
@GeorgeSapkin bump, this is great, would love to get this in for Jest 23 |
Codecov Report
@@ Coverage Diff @@
## master #5898 +/- ##
==========================================
+ Coverage 63.55% 63.58% +0.02%
==========================================
Files 235 235
Lines 9030 9037 +7
Branches 3 4 +1
==========================================
+ Hits 5739 5746 +7
Misses 3290 3290
Partials 1 1
Continue to review full report at Codecov.
|
|
@SimenB I've rebased and existing checks are passing. What kind of additional tests do I need to write? |
|
There are no tests added in this PR, some unit tests asserting that the newly added code does its job would be awesome. |
|
@GeorgeSapkin do you have any time to take this PR over the finish line? |
|
I'll try to add the missing tests this weekend. |
|
@GeorgeSapkin mind rebasing this? After that we should be good to merge, sorry for this taking so long! |
|
|
|
It's failing allover the repo, I don't know what's the issue :(. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This change adds package name to notification in NotifyReporter when using
--notifyflag ornotifyconfiguration option. This clarifies which package is being tested which is especially useful when watching tests in mono-repos and with multiple IDEs opened.First I'm trying to get hasteFS from the first context to get the module name (must be a better way). Then I'm falling back to
rootDirand otherwise to original behavior without an additional title.Before:
After: