Fix watch regression (#519)

* Fix broken tests

I switched to a deep equal because it provides better test output vs the boolean comparison. Let me know if you want to keep it the same as before.

* Use the correct initVersion

Use the configured default init-version config from npm, instead of assuming the default. This validates the current behavior.

* Remove unreachable code

# Conflicts:
#	lib/watch/index.js

* Increase timeout on slow test

I think this will make the test pass more reliably

* Revert "skip broken tests" and re-enable other disabled tests

This reverts commit c4cde16 with modifications.

Make watch tests pass again

Looking into why these events are missed.

Enable a passing test for CI

* Disable another test on windows

* Fix depreciation warning

* Increase test timeout

Windows is having spurious timeouts.

Co-authored-by: Andy Pickler <[email protected]>

Author: bcomnes

lib/watch/index.js

@@ -48,9 +48,6 @@ const watch = function (args, ignoreWatch, verboseWatch) {
     })
 
     _child.on('exit', function (code, signal) {
-      if (!code) {
-        stop()
-      }
       if (childs.length === 0 && !allStop) {
         childs.push(run('restart'))
       }

package.json

@@ -9,7 +9,7 @@
   "scripts": {
     "lint": "standard",
     "lint:fix": "standard --fix",
-    "unit": "tap \"test/**/*.test.{js,ts}\" \"templates/**/*.test.{js,ts}\"",
+    "unit": "tap \"test/**/*.test.{js,ts}\" \"templates/**/*.test.{js,ts}\" --timeout 120",
     "pretest": "xcopy /e /k /i . \"..\\node_modules\\fastify-cli\" || rsync -r --exclude=node_modules ./ node_modules/fastify-cli || echo 'this is fine'",
     "test": "npm run lint && npm run unit && npm run test:typescript",
     "test:typescript": "tsd templates/plugin && tsc --project templates/app-ts/tsconfig.json && del-cli templates/app-ts/dist"

test/generate-plugin.test.js

@@ -17,10 +17,11 @@ const { generate, pluginTemplate } = require('../generate-plugin')
 const workdir = path.join(__dirname, 'workdir')
 const templateDir = path.join(__dirname, '..', 'templates', 'plugin')
 const cliPkg = require('../package')
-const { exec } = require('child_process')
+const { exec, execSync } = require('child_process')
 const minimatch = require('minimatch')
 const strip = require('strip-ansi')
 const expected = {}
+const initVersion = execSync('npm get init-version').toString().trim()
 
 ;(function (cb) {
   const files = []
@@ -121,7 +122,7 @@ function define (t) {
         t.equal(pkg.main, 'index.js')
         t.equal(pkg.types, 'index.d.ts')
         // we are not checking author because it depends on global npm configs
-        t.equal(pkg.version, '1.0.0')
+        t.equal(pkg.version, initVersion)
         t.equal(pkg.description, '')
         t.ok(pkg.license === 'MIT')
         t.equal(pkg.scripts.lint, 'standard && npm run lint:typescript')

test/generate-typescript.test.js

@@ -17,10 +17,11 @@ const { generate, typescriptTemplate } = require('../generate')
 const workdir = path.join(__dirname, 'workdir')
 const appTemplateDir = path.join(__dirname, '..', 'templates', 'app-ts')
 const cliPkg = require('../package')
-const { exec } = require('child_process')
+const { exec, execSync } = require('child_process')
 const minimatch = require('minimatch')
 const strip = require('strip-ansi')
 const expected = {}
+const initVersion = execSync('npm get init-version').toString().trim()
 
 ;(function (cb) {
   const files = []
@@ -120,7 +121,7 @@ function define (t) {
         const pkg = JSON.parse(data)
         t.equal(pkg.name, 'workdir')
         // we are not checking author because it depends on global npm configs
-        t.equal(pkg.version, '1.0.0')
+        t.equal(pkg.version, initVersion)
         t.equal(pkg.description, 'This project was bootstrapped with Fastify-CLI.')
         // by default this will be ISC but since we have a MIT licensed pkg file in upper dir, npm will set the license to MIT in this case
         // so for local tests we need to accept MIT as well

test/generate.test.js

@@ -20,12 +20,13 @@ const { generate, javascriptTemplate } = require('../generate')
 const workdir = path.join(__dirname, 'workdir')
 const appTemplateDir = path.join(__dirname, '..', 'templates', 'app')
 const cliPkg = require('../package')
-const { exec } = require('child_process')
+const { exec, execSync } = require('child_process')
 const pExec = promisify(exec)
 const pUnlink = promisify(unlink)
 const minimatch = require('minimatch')
 const strip = require('strip-ansi')
 const expected = {}
+const initVersion = execSync('npm get init-version').toString().trim()
 
 ;(function (cb) {
   const files = []
@@ -154,7 +155,7 @@ function define (t) {
         const pkg = JSON.parse(data)
         t.equal(pkg.name, pkgName)
         // we are not checking author because it depends on global npm configs
-        t.equal(pkg.version, '1.0.0')
+        t.equal(pkg.version, initVersion)
         t.equal(pkg.description, 'This project was bootstrapped with Fastify-CLI.')
         // by default this will be ISC but since we have a MIT licensed pkg file in upper dir, npm will set the license to MIT in this case
         // so for local tests we need to accept MIT as well

test/print-routes.test.js

@@ -19,7 +19,7 @@ test('should print routes', async t => {
 
   await fastify.close()
   t.ok(spy.called)
-  t.ok(spy.calledWithMatch('debug', '└── / (GET)\n    / (POST)\n'))
+  t.same(spy.args, [['debug', '└── / (GET)\n    / (HEAD)\n    / (POST)\n']])
 })
 
 test('should print routes via cli', async t => {
@@ -32,7 +32,7 @@ test('should print routes via cli', async t => {
   await command.cli(['./examples/plugin.js'])
 
   t.ok(spy.called)
-  t.ok(spy.calledWithMatch('debug', '└── / (GET)\n    / (POST)\n'))
+  t.same(spy.args, [['debug', '└── / (GET)\n    / (HEAD)\n    / (POST)\n']])
 })
 
 test('should warn on file not found', t => {
@@ -109,5 +109,5 @@ test('should print routes of server with an async/await plugin', async t => {
 
   await fastify.close()
   t.ok(spy.called)
-  t.ok(spy.calledWithMatch('debug', '└── / (GET)\n'))
+  t.same(spy.args, [['debug', '└── / (GET)\n    / (HEAD)\n']])
 })

test/start.test.js

@@ -251,8 +251,10 @@ test('should error with a good timeout value', async t => {
     }
   })
 
+  const port = getPort()
+
   try {
-    const argv = ['-p', '3040', '-T', '100', './test/data/timeout-plugin.js']
+    const argv = ['-p', port, '-T', '100', './test/data/timeout-plugin.js']
     await start.start(argv)
   } catch (err) {
     t.equal(err.code, 'AVV_ERR_READY_TIMEOUT')
@@ -533,13 +535,14 @@ test('should start the server listening on 0.0.0.0 when running in docker', asyn
   t.pass('server closed')
 })
 
-// TODO remove skip
-test('should start the server with watch options that the child process restart when directory changed', { skip: true }, async (t) => {
-  t.plan(4)
+test('should start the server with watch options that the child process restart when directory changed', { skip: process.platform === 'win32' }, async (t) => {
+  t.plan(3)
   const tmpjs = path.resolve(baseFilename + '.js')
 
+  const port = getPort()
+
   await writeFile(tmpjs, 'hello world')
-  const argv = ['-p', '4042', '-w', './examples/plugin.js']
+  const argv = ['-p', port, '-w', './examples/plugin.js']
   const fastifyEmitter = await start.start(argv)
 
   t.teardown(async () => {
@@ -549,9 +552,6 @@ test('should start the server with watch options that the child process restart
     await fastifyEmitter.stop()
   })
 
-  await once(fastifyEmitter, 'start')
-  t.pass('should receive start event')
-
   await once(fastifyEmitter, 'ready')
   t.pass('should receive ready event')
 
@@ -563,9 +563,8 @@ test('should start the server with watch options that the child process restart
   t.pass('should receive restart event')
 })
 
-// TODO remove skip
-test('should start the server with watch and verbose-watch options that the child process restart when directory changed with console message about changes ', { skip: true }, async (t) => {
-  t.plan(5)
+test('should start the server with watch and verbose-watch options that the child process restart when directory changed with console message about changes ', { skip: process.platform === 'win32' }, async (t) => {
+  t.plan(4)
 
   const spy = sinon.spy()
   const watch = proxyquire('../lib/watch', {
@@ -580,8 +579,10 @@ test('should start the server with watch and verbose-watch options that the chil
 
   const tmpjs = path.resolve(baseFilename + '.js')
 
+  const port = getPort()
+
   await writeFile(tmpjs, 'hello world')
-  const argv = ['-p', '4042', '-w', '--verbose-watch', './examples/plugin.js']
+  const argv = ['-p', port, '-w', '--verbose-watch', './examples/plugin.js']
   const fastifyEmitter = await start.start(argv)
 
   t.teardown(async () => {
@@ -591,9 +592,6 @@ test('should start the server with watch and verbose-watch options that the chil
     await fastifyEmitter.stop()
   })
 
-  await once(fastifyEmitter, 'start')
-  t.pass('should receive start event')
-
   await once(fastifyEmitter, 'ready')
   t.pass('should receive ready event')
 
@@ -603,11 +601,10 @@ test('should start the server with watch and verbose-watch options that the chil
   // this might happen more than once but does not matter in this context
   await once(fastifyEmitter, 'restart')
   t.pass('should receive restart event')
-
-  t.ok(spy.calledOnce, 'should print a console message on file update')
+  t.ok(spy.args.length > 0, 'should print a console message on file update')
 })
 
-test('should reload the env on restart when watching', { skip: true }, async (t) => {
+test('should reload the env on restart when watching', { skip: process.platform === 'win32' }, async (t) => {
   const testdir = t.testdir({
     '.env': 'GREETING=world',
     'plugin.js': await readFile(path.join(__dirname, '../examples/plugin-with-env.js'))