Print watch mode shortcut commands #1555

luthfianto · 2017-10-17T15:51:46Z

novemberborn

Thanks for your PR @rilut!

Could you have a look at the watcher tests? I think you need to add a spy for the write() method: https://github.com/avajs/ava/blob/3f81fc431016ca115177d2d79cf9db36b05f69ad/test/watcher.js#L76:L82

And some assertions as well.

novemberborn · 2017-10-24T15:45:26Z

lib/watcher.js

@@ -123,6 +123,7 @@ class Watcher {
 				.then(runStatus => {
 					runStatus.previousFailCount = this.sumPreviousFailures(currentVector);
 					logger.finish(runStatus);
+					logger.write('[ava] To rerun tests, enter `r`\n[ava] To update snapshots used in the previous test run, enter `u`');


I don't think this needs to be prefixed with [ava]. Currently the key needs to be followed by Enter in order to be recognized. How about:

To rerun all tests, type 'r', followed by Enter
To update snapshots used in the previous tests, type 'u', followed by Enter

Perhaps using chalk.gray.dim as the color.

novemberborn · 2017-11-02T16:27:48Z

Hi @rilut, just wondering if there's more commits forthcoming? Would be good to have some assertions on the logger.write() spy, like is done for the other logger methods. Thanks!

luthfianto · 2017-11-03T03:53:29Z

Oh sure, will do

novemberborn · 2017-11-13T17:43:00Z

@rilut thanks for following up. I've just restarted CI, so hopefully all the tests will pass. Still need to give this a whirl, have been a bit busy sorry.

novemberborn · 2017-11-13T17:50:30Z

Ah no, the CLI integration tests are failing because there's new output. Could you look into that @rilut? 😄

luthfianto · 2017-11-18T09:41:16Z

@novemberborn I'll look into it

luthfianto · 2017-11-18T10:37:58Z

test/cli.js

-			t.is(buffer.replace(/\s/g, ''), '');
+			const rerunMessage = `To rerun all tests, type 'r', followed by Enter\nTo update snapshots used in the previous tests, type 'u', followed by Enter\n`;
+			const rerunMessageWithoutWhitespace = rerunMessage.replace(/\s/g, '');
+			t.is(buffer.replace(/\s/g, ''), rerunMessageWithoutWhitespace);


@novemberborn I'm not sure if what I did (asserting this) is the right way to do this

Most tests use t.match for this. You could do an assertion for each line.

luthfianto · 2017-11-18T10:48:22Z

not sure what happened with the node:6 FreshDeps=false tests

luthfianto · 2017-11-18T10:49:52Z

https://travis-ci.org/avajs/ava/jobs/303922129

test/cli.js ....................................... 138/140 5m
  snapshots work (tests)
  not ok timeout!
    signal: SIGTERM
    handles:
      - type: ChildProcess
        events:
          - close
      - type: Socket
        events:
          - end
          - finish
          - _socketEnd
      - type: Socket
        events:
          - end
          - finish
          - _socketEnd
          - close
          - error
          - data
      - type: Socket
        events:
          - end
          - finish
          - _socketEnd
          - close
          - error
          - data
    expired: TAP
    stack: |
      emit (node_modules/nyc/node_modules/signal-exit/index.js:77:11)
      process.listener (node_modules/nyc/node_modules/signal-exit/index.js:91:7)
      processEmit (node_modules/nyc/node_modules/signal-exit/index.js:155:32)
      process.emit (node_modules/source-map-support/source-map-support.js:439:21)
      processEmit (node_modules/signal-exit/index.js:155:32)
    test: snapshots work (tests)
  
  not ok test count !== plan
    +++ found
    --- wanted
    -1
    +61
    results:
      ok: false
      count: 61
      pass: 60
      fail: 2
      bailout: false
      todo: 0
      skip: 0
      plan:
        start: null
        end: null
        skipAll: false
        skipReason: ''
        comment: ''
      failures:
        - ok: false
          id: 61
          time: 5460.13
          name: snapshots work (tests)
        - tapError: no plan

novemberborn · 2017-11-28T18:13:15Z

@rilut the CI failures seem to be intermittent. I'm running out of time for today but I'll try and have a look at this tomorrow.

novemberborn

OK, CI is passing 🎉

Unfortunately this doesn't work with the mini reporter: the test output itself is overwritten. It's fine with the verbose reporter though. I also think we should tweak some of the colors, see inline comments.

Thank you for your work on this @rilut, sorry for taking a bit longer to get back to you on this PR.

novemberborn · 2017-11-29T16:07:37Z

lib/watcher.js

@@ -18,6 +19,7 @@ function rethrowAsync(err) {

 const MIN_DEBOUNCE_DELAY = 10;
 const INITIAL_DEBOUNCE_DELAY = 100;
+const rerunMessage = chalk.gray.dim(`To rerun all tests, type 'r', followed by Enter\nTo update snapshots used in the previous tests, type 'u', followed by Enter\n`);


Looking at this in my terminal I think this should just be chalk.gray. But we then should also update the timestamp output in the mini & verbose loggers to not be dimmed.

novemberborn · 2017-11-29T16:09:42Z

test/cli.js

-			t.is(buffer.replace(/\s/g, ''), '');
+			const rerunMessage = `To rerun all tests, type 'r', followed by Enter\nTo update snapshots used in the previous tests, type 'u', followed by Enter\n`;
+			const rerunMessageWithoutWhitespace = rerunMessage.replace(/\s/g, '');
+			t.is(buffer.replace(/\s/g, ''), rerunMessageWithoutWhitespace);


Most tests use t.match for this. You could do an assertion for each line.

novemberborn · 2017-11-29T16:11:25Z

lib/watcher.js

@@ -126,6 +128,7 @@ class Watcher {
 				.then(runStatus => {
 					runStatus.previousFailCount = this.sumPreviousFailures(currentVector);
 					logger.finish(runStatus);
+					logger.write(rerunMessage);


Unfortunately this doesn't work with the mini reporter. I think finish() should take an optional second parameter, call it footer perhaps, and both the mini and verbose loggers can then include this in their regular output.

sindresorhus · 2017-12-13T12:41:12Z

It shouldn't reprint the message when I press r+Enter.

luthfianto · 2017-12-16T18:05:31Z

@sindresorhus So after Ava writing the status in MiniLogger.finish, I need to write the rerunMessage. And delete the rerunMessage using ansiEscapes.eraseLines() every time I pressed r+Enter, right?

Which function do you think the rerunMessage should be in?

novemberborn · 2017-12-28T16:40:56Z

@rilut I think what @sindresorhus is seeing is due to my earlier comment:

Unfortunately this doesn't work with the mini reporter: the test output itself is overwritten.

My suggestion for resolving this is https://github.com/avajs/ava/pull/1555/files#r153833345.

novemberborn · 2018-02-11T17:26:15Z

Closing this since it has stalled. If anybody is looking to revive this PR please check in first, I'm considering refactoring the reporter pipeline which impacts this PR.

luthfianto force-pushed the feature/add-watch-mode-commands-info branch from 603f053 to 8de3834 Compare October 17, 2017 16:07

novemberborn requested changes Oct 24, 2017

View reviewed changes

luthfianto added 4 commits November 18, 2017 16:40

feat(watcher): print commands info

7587b3b

Use color on rerunMessage and add sinon spy for logger.write

81aed1c

Use apostrophe instead of double quotes

502ff4f

test(watcher.js): Add assertion on logger.write call count

6de3a6e

luthfianto force-pushed the feature/add-watch-mode-commands-info branch from 771f870 to 00ab140 Compare November 18, 2017 10:18

test(cli.js): Add the watcher rerunMessage into assertion

c90b54a

luthfianto force-pushed the feature/add-watch-mode-commands-info branch from 00ab140 to c90b54a Compare November 18, 2017 10:30

luthfianto commented Nov 18, 2017

View reviewed changes

novemberborn requested changes Nov 29, 2017

View reviewed changes

novemberborn closed this Feb 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Print watch mode shortcut commands #1555

Print watch mode shortcut commands #1555

luthfianto commented Oct 17, 2017

novemberborn left a comment

novemberborn Oct 24, 2017

novemberborn commented Nov 2, 2017

luthfianto commented Nov 3, 2017

novemberborn commented Nov 13, 2017

novemberborn commented Nov 13, 2017

luthfianto commented Nov 18, 2017

luthfianto Nov 18, 2017

novemberborn Nov 29, 2017

luthfianto commented Nov 18, 2017

luthfianto commented Nov 18, 2017 •

edited

Loading

novemberborn commented Nov 28, 2017

novemberborn left a comment

novemberborn Nov 29, 2017

novemberborn Nov 29, 2017

novemberborn Nov 29, 2017

sindresorhus commented Dec 13, 2017

luthfianto commented Dec 16, 2017 •

edited

Loading

novemberborn commented Dec 28, 2017

novemberborn commented Feb 11, 2018

Print watch mode shortcut commands #1555

Print watch mode shortcut commands #1555

Conversation

luthfianto commented Oct 17, 2017

novemberborn left a comment

Choose a reason for hiding this comment

novemberborn Oct 24, 2017

Choose a reason for hiding this comment

novemberborn commented Nov 2, 2017

luthfianto commented Nov 3, 2017

novemberborn commented Nov 13, 2017

novemberborn commented Nov 13, 2017

luthfianto commented Nov 18, 2017

luthfianto Nov 18, 2017

Choose a reason for hiding this comment

novemberborn Nov 29, 2017

Choose a reason for hiding this comment

luthfianto commented Nov 18, 2017

luthfianto commented Nov 18, 2017 • edited Loading

novemberborn commented Nov 28, 2017

novemberborn left a comment

Choose a reason for hiding this comment

novemberborn Nov 29, 2017

Choose a reason for hiding this comment

novemberborn Nov 29, 2017

Choose a reason for hiding this comment

novemberborn Nov 29, 2017

Choose a reason for hiding this comment

sindresorhus commented Dec 13, 2017

luthfianto commented Dec 16, 2017 • edited Loading

novemberborn commented Dec 28, 2017

novemberborn commented Feb 11, 2018

luthfianto commented Nov 18, 2017 •

edited

Loading

luthfianto commented Dec 16, 2017 •

edited

Loading