Native Flow, use Jest #767

tmcw · 2017-05-01T19:01:40Z

Fixes Build system: switch out of Flow comment syntax #729 - switches to native Flow annotations
Fixes Update to prettier 0.23.0 when it's released #709 - upgrades Prettier
Fixes Switch to ava #664 - uses jest

tmcw · 2017-05-02T16:29:52Z

Would much appreciate a code review if y'all can spare the time! This is a big change, but has mainly accomplished:

Using native Flow annotations
Switching from node-tap to Jest and using Jest's snapshot tests, and keeping code coverage in place - though it now only tests directly unit-tested code, rather than counting integration tests in the same bucket
Sets up babel to transpile the library. It's restrictive - not going crazy on JS features. I'd love to keep it strict enough that we could eventually use Rollup to distribute a bundle.
Tests are faster now: 2:48 instead of 3:20

arv

Overall an improvement. I had a few minor questions but nothing that is preventing you from going forward with this.

arv · 2017-05-06T23:12:33Z

__tests__/bin-readme.js

+  });
+}
+
+var UPDATE = !!process.env.UPDATE;


Do we still need this?

We (I) can change this in a follow up if it works now. I don't want to make this PR any larger.

👍 removed

arv · 2017-05-06T23:21:25Z

__tests__/lib/merge_config.js


-test('bad config', function(t) {
+test('bad config', function(done) {


FYI, Jest also supports using async functions.

arv · 2017-05-06T23:25:34Z

package.json

    "doc": "./bin/documentation.js build lib/index.js -f md --access=public > docs/NODE_API.md",
    "changelog": "standard-changelog -i CHANGELOG.md -w",
    "self-lint": "node ./bin/documentation.js lint",
-    "test": "eslint lib && are-we-flow-yet lib && flow check && npm run self-lint && npm run test-tap",
-    "test-tap": "tap -t 120 --coverage --nyc-arg=--cache test/*.js test/lib test/streams"
+    "test": "eslint src && are-we-flow-yet src && flow check && npm run self-lint && jest --runInBand"


Do we really need --runInBand? That slows things down a bit

--runInBand makes tests way more reliable and faster on CircleCI, but slower locally. I'd love to find a way to only enable it in the CircleCI environment, but haven't found one yet.

arv · 2017-05-06T23:28:20Z

src/serve/server.js

@@ -7,7 +7,7 @@ var http = require('http'),
  mime = require('mime'),
  pify = require('pify'),
  EventEmitter = require('events').EventEmitter,
-  liveReload = require('tiny-lr'),
+  // liveReload = require('tiny-lr'),


arv · 2017-05-06T23:29:53Z

src/serve/server.js

      this._http = http.createServer(this.handler.bind(this));

      return Promise.all([
-        pify(this._lr.listen.bind(this._lr))(35729),
+        // this._lr && pify(this._lr.listen.bind(this._lr))(35729),


arv · 2017-05-06T23:30:15Z

src/serve/server.js

@@ -28,17 +28,19 @@ declare type ServerFile = {
 */
 class Server extends EventEmitter {
  /* :: _lr: Object; */


Can this use non comment syntax now?

arv · 2017-05-06T23:35:19Z

.eslintrc

@@ -20,7 +20,7 @@
    "no-throw-literal": 2,
    "no-self-compare": 2,
    "no-void": 2,
-    "no-unused-vars": 2,
+    "no-unused-vars": 1,


eslint was, for some reason, incorrectly flagging variables as unused, but trying it today.. it works! so back to 2.

arv · 2017-05-06T23:35:55Z

__tests__/bin.js

@@ -1,4 +1,4 @@
-'use strict';
+/* global jasmine */


arv · 2017-05-06T23:36:27Z

declarations/comment.js

@@ -45,6 +42,11 @@ type SourceFile = {
  file: string
 };

+// type InputsConfig = {


👍 Removed

coveralls · 2017-05-08T20:30:03Z

Changes Unknown when pulling 6ef60e0 on native into ** on master**.

We're switching to Flow annotations - not Flow comments. This gives documentation.js the ability to self-document without JSDoc types and improves our compatibility with tools like prettier. Fixes #729. Fixes #709

tmcw · 2017-05-09T00:46:02Z

Thanks for the review, Erik!

coveralls · 2017-05-09T16:07:45Z

Changes Unknown when pulling ad8dc3e on native into ** on master**.

coveralls · 2017-05-09T16:11:49Z

Changes Unknown when pulling 3a91eec on native into ** on master**.

coveralls · 2017-05-09T19:05:48Z

Changes Unknown when pulling 6adabd5 on native into ** on master**.

coveralls · 2017-05-09T19:46:15Z

Changes Unknown when pulling 9b1c41b on native into ** on master**.

tmcw changed the title ~~Native Flow comments~~ Native Flow, use Jest May 1, 2017

tmcw force-pushed the native branch from 9cae7a8 to 1d85a02 Compare May 2, 2017 15:53

tmcw requested review from arv and jfirebaugh May 2, 2017 15:54

arv approved these changes May 6, 2017

View reviewed changes

build: Use Flow syntax without comments.

6adabd5

We're switching to Flow annotations - not Flow comments. This gives documentation.js the ability to self-document without JSDoc types and improves our compatibility with tools like prettier. Fixes #729. Fixes #709

tmcw force-pushed the native branch from 3a91eec to 6adabd5 Compare May 9, 2017 00:24

Report in a post task

9b1c41b

tmcw merged commit 11d9045 into master May 9, 2017

tmcw mentioned this pull request Jun 8, 2017

github links, custom commit/tags support. #801

Closed

tmcw deleted the native branch April 11, 2018 17:39


		test('bad config', function(t) {
		test('bad config', function(done) {

Native Flow, use Jest #767

Native Flow, use Jest #767

Uh oh!

Conversation

tmcw commented May 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmcw commented May 2, 2017

Uh oh!

arv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented May 8, 2017

Uh oh!

tmcw commented May 9, 2017

Uh oh!

coveralls commented May 9, 2017

Uh oh!

coveralls commented May 9, 2017

Uh oh!

coveralls commented May 9, 2017

Uh oh!

coveralls commented May 9, 2017

Uh oh!

Uh oh!

tmcw commented May 1, 2017 •

edited

Loading