Repository: okonet/lint-staged
Pull request #309 (2017-10-09 10:15:08)

Add full coverage for runAll.js
I am trying to solve this [issue](https://github.com/okonet/lint-staged/issues/259) but I have little time. So I will solve the issue in chunks.

I will be glad if you will point out any mistakes

codecov[bot]: # [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=h1) Report
> Merging [#309](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=desc) into [master](https://codecov.io/gh/okonet/lint-staged/commit/50416a94d6e4280b37e8a0d8cdf867910f042af1?src=pr&el=desc) will **increase** coverage by `3.48%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/okonet/lint-staged/pull/309/graphs/tree.svg?token=LSoEgsjk4O&src=pr&height=150&width=650)](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #309      +/-   ##
==========================================
+ Coverage   91.86%   95.34%   +3.48%     
==========================================
  Files          10       10              
  Lines         172      172              
  Branches       26       26              
==========================================
+ Hits          158      164       +6     
+ Misses         13        8       -5     
+ Partials        1        0       -1
```


| [Impacted Files](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/runAll.js](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=tree#diff-c3JjL3J1bkFsbC5qcw==) | `100% <0%> (+22.22%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=footer). Last update [50416a9...2993a2a](https://codecov.io/gh/okonet/lint-staged/pull/309?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Andrew1024: As for the test with falling snapshot - I could not find how the time is set. I mean I tried to mock `Date.now` but It doesn't seem to work
okonet: I think we might need a special List renderer that doesn't output time to console. Check out https://github.com/SamVerschueren/listr-verbose-renderer. Or probably providing a format that renders nothing could help but I don't know if this is possible: https://date-fns.org/docs/format
okonet: Since verbose render is so simple, I'd create a listr-test-render with a copy of it and remove the timestamp rendering here: https://github.com/SamVerschueren/listr-verbose-renderer/blob/master/lib/utils.js#L8
okonet: Just realized the actual code is here: https://github.com/SamVerschueren/listr-verbose-renderer/blob/master/index.js

Probably create a PR to the verbose renderer that would allow rendering without the timestamp would be the simplest way? Thoughts?
Andrew1024: > create a PR to the verbose renderer that would allow rendering without the timestamp

I will try to do that.

And do we actually need a timestamp in lint-staged at all?
okonet: > And do we actually need a timestamp in lint-staged at all?

Don't know. I assume this can be practical when used with non-TTY mode but ideally lint-staged should not be run in non-TTY.
Andrew1024: About timestamp:
personally, I can't see the usage of timestamp in a console output during development
okonet: Okay I agree on the logging format without dates. Could you please fix the console mock cleanup and this is good to merge.
sudo-suhas: @okonet, do you mind if I review this tomorrow? 
sudo-suhas: @cameronhunter As it turns out, in this case at least, there aren't any `console.log` statements in this file. So we should be looking to capture the logs from global `console` itself which `listr` logs to.
okonet: @sudo-suhas I'd love to!
sudo-suhas: Thanks @okonet 😃. I have added the review.
Andrew1024: > So we should be looking to capture the logs from global `console` itself which `listr` logs to.

What do we do now then? I suppose that we mock the console and listr uses this mocked version of theconsole
sudo-suhas: @Andrew1024 The code we have now is already doing that. The snapshots have the logs from listr.

Patch:
diff --git a/package.json b/package.json
index 3c7131e..4052111 100644
--- a/package.json
+++ b/package.json
@@ -34,7 +34,7 @@
     "execa": "^0.8.0",
     "is-glob": "^4.0.0",
     "jest-validate": "^21.1.0",
-    "listr": "^0.12.0",
+    "listr": "^0.13.0",
     "lodash": "^4.17.4",
     "log-symbols": "^2.0.0",
     "minimatch": "^3.0.0",
diff --git a/src/runAll.js b/src/runAll.js
index 04038a6..76a46fb 100644
--- a/src/runAll.js
+++ b/src/runAll.js
@@ -38,6 +38,7 @@ module.exports = function runAll(scripts, config) {
           new Listr(runScript(task.commands, task.fileList, scripts, config), {
             // In sub-tasks we don't want to run concurrently
             // and we want to abort on errors
+            dateFormat: false,
             concurrent: false,
             exitOnError: true
           }),
@@ -51,6 +52,7 @@ module.exports = function runAll(scripts, config) {
 
       if (tasks.length) {
         return new Listr(tasks, {
+          dateFormat: false,
           concurrent,
           renderer,
           exitOnError: !concurrent // Wait for all errors when running concurrently


Test patch:
diff --git a/test/__snapshots__/runAll.spec.js.snap b/test/__snapshots__/runAll.spec.js.snap
index 61ac8c6..8270309 100644
--- a/test/__snapshots__/runAll.spec.js.snap
+++ b/test/__snapshots__/runAll.spec.js.snap
@@ -1,5 +1,20 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`runAll should not skip tasks if there are files 1`] = `
+"
+LOG Running tasks for *.js [started]
+LOG echo \\"sample\\" [started]
+LOG echo \\"sample\\" [completed]
+LOG Running tasks for *.js [completed]"
+`;
+
+exports[`runAll should resolve the promise with no files 1`] = `
+"
+LOG Running tasks for *.js [started]
+LOG Running tasks for *.js [skipped]
+LOG → No staged files match *.js"
+`;
+
 exports[`runAll should throw when invalid config is provided 1`] = `"Invalid config provided to runAll! Use getConfig instead."`;
 
 exports[`runAll should throw when invalid config is provided 2`] = `"Invalid config provided to runAll! Use getConfig instead."`;
diff --git a/test/runAll.spec.js b/test/runAll.spec.js
index 255ed04..20fda93 100644
--- a/test/runAll.spec.js
+++ b/test/runAll.spec.js
@@ -1,3 +1,4 @@
+import { makeConsoleMock } from 'consolemock'
 import sgfMock from 'staged-git-files'
 import { getConfig } from '../src/getConfig'
 import runAll from '../src/runAll'
@@ -9,10 +10,14 @@ sgfMock.mockImplementation((params, callback) => {
 })
 
 const scripts = { mytask: 'echo "Running task"' }
+const globalConsoleTemp = global.console
 
 describe('runAll', () => {
-  afterEach(() => {
-    sgfMock.mockClear()
+  beforeEach(() => {
+    global.console = makeConsoleMock()
+  })
+  afterAll(() => {
+    global.console = globalConsoleTemp
   })
   it('should throw when invalid config is provided', () => {
     expect(() => runAll(scripts, {})).toThrowErrorMatchingSnapshot()
@@ -35,8 +40,21 @@ describe('runAll', () => {
     return expect(runAll(scripts, getConfig({}))).resolves.toEqual('No tasks to run.')
   })
 
+  it('should resolve the promise with no files', async () => {
+    await runAll(scripts, getConfig({ linters: { '*.js': ['echo "sample"'] } }))
+    expect(console.printHistory()).toMatchSnapshot()
+  })
+
+  it('should not skip tasks if there are files', async () => {
+    sgfMock.mockImplementationOnce((params, callback) => {
+      callback(null, [{ filename: 'sample.js', status: 'sample' }])
+    })
+    await runAll(scripts, getConfig({ linters: { '*.js': ['echo "sample"'] } }))
+    expect(console.printHistory()).toMatchSnapshot()
+  })
+
   it('should reject the promise when staged-git-files errors', () => {
-    sgfMock.mockImplementation((params, callback) => {
+    sgfMock.mockImplementationOnce((params, callback) => {
       callback('test', undefined)
     })
     expect.assertions(1)