Add notifications for test results #1170
Conversation
ghost
added
the
| @@ -16,7 +16,8 @@ | |||
| "minimatch": "^3.0.0", | |||
| "mkdirp": "^0.5.1", | |||
| "progress": "^1.1.8", | |||
| "rimraf": "^2.5.2" | |||
| "rimraf": "^2.5.2", | |||
| "growl": "^1.9.2" | |||
There was a problem hiding this comment.
that's a top level package.json though. You might want to put it into packages/jest-cli/package.json
|
woah this is cool! cc @gillesruppert – we need to bring this to www too :) Does growl somehow work across ssh sessions? |
| /** | ||
| * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. | ||
| * | ||
| * @flow |
There was a problem hiding this comment.
this should be after the copyright notice :)
The reporters were moved from runtime to config, but the Growler class in this file did not move which broke the build.
| } | ||
| }; | ||
|
|
||
| class Growler { |
There was a problem hiding this comment.
Can we add a test for this class? I wanna make sure we won't be breaking this feature :)
We can just jest.mock('grown') and check if it has been called with the proper args.
The reporters were moved from runtime to config, but the Growler class in this file did not move which broke the build.
| @@ -76,6 +76,7 @@ Jest uses Jasmine 2 by default. An introduction to Jasmine 2 can be found | |||
| - [`testRunner` [string]](#testrunner-string) | |||
| - [`unmockedModulePathPatterns` [array<string>]](#unmockedmodulepathpatterns-array-string) | |||
| - [`verbose` [boolean]](#verbose-boolean) | |||
| - [`growl` [boolean]](#growl-boolean) | |||
There was a problem hiding this comment.
can we put this and the description below into its correct alphabetical location?
|
Does growl support images at 2x for retina displays? If so, can you add a file that is 2x as big? Most Mac users nowadays use a retina display, so it should be the default :) I'm also sorry for our silly abc style guide. I wonder if there is a lint rule for this. |
|
I will update the code with the abc rule. |
|
Does growl resize the image to a set size? If yes, we could just ship the 2x image and let growl take care of it? On Wed, Jun 15, 2016 at 4:01 PM -0700, "Marcel Erz" <[email protected]mailto:[email protected]> wrote: I will update the code with the abc rule. You are receiving this because you commented. |
|
A couple comments from the outside (since @cpojer posted on Twitter):
|
| /** | ||
| * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. | ||
| * | ||
| * @flow |
There was a problem hiding this comment.
Typically we put this below the license. Ignore me if you just copy-pasted from another file - we can do a bulk change later if that's the case.
|
@zpao Sounds good. I will move it to Any suggestions for names? Maybe |
|
"Notifier" as a class name is good :) |
|
Will also change the option from |
|
This is awesome! Now I need to get that into www :) |
| let message; | ||
|
|
||
| if (result.success) { | ||
| info = NOTIFICATIONS['success']; |
There was a problem hiding this comment.
please do NOTIFICATIONS.success.
|
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. |
No description provided.