Merge pull request #36 from eobrain/flaky

Handle race condition (file deleted after directory scan) that was causing flaky test.

src/fs_util.js

@@ -2,7 +2,7 @@ const externalRequire = require
 const fs = externalRequire('fs')
 const path = externalRequire('path')
 const tmp = externalRequire('tmp')
-// const {pp} = require('../../passprint')
+// const { pp } = require('../../passprint')
 
 /** @returns timestamp if it is a file, or zero if it does not exist or is a directory. */
 const timestamp = path =>
@@ -19,13 +19,17 @@ const walkDir = (dir, callback) => {
       return
     }
     const dirPath = path.join(dir, f)
-    if (fs.statSync(dirPath).isDirectory()) {
-      walkDir(dirPath, callback)
-    } else {
-      const childPath = path.join(dir, f)
-      if (!fs.statSync(childPath).isDirectory()) {
-        callback(childPath)
+    try {
+      if (fs.statSync(dirPath).isDirectory()) {
+        walkDir(dirPath, callback)
+      } else {
+        const childPath = path.join(dir, f)
+        if (!fs.statSync(childPath).isDirectory()) {
+          callback(childPath)
+        }
       }
+    } catch (e) {
+      // Expected occasional race condition: f was deleted after readdir call
     }
   })
 }

src/index.js

@@ -6,35 +6,39 @@ const { timestamp } = require('./fs_util.js')
 const Variables = require('./variables.js')
 const Tasks = require('./tasks.js')
 const ago = require('./ago.js')
-// const {pp} = require('../../passprint')
+// const { pp } = require('passprint')
 
 // const trace = x => console.log('trace:', x) || x
 
 /**
  * @returns [errorCode, stdoutString, stderrString]
  */
 module.exports = async (bajelfile) => {
-  const { tConsole, tStdout, tStderr } = StrConsole()
   const options = getopts(process.argv.slice(2), {
-    boolean: ['n', 'p', 'd', 'h'],
+    boolean: ['d', 'h', 'n', 'p', 's', 't', 'T'],
     alias: {
-      n: ['just-print', 'dry-run', 'recon'],
+      d: ['debug'],
       h: ['help'],
+      n: ['dry-run'],
+      p: ['print'],
+      s: ['silent'],
       t: ['targets'],
       T: ['targets-expanded'],
-      d: ['debug'],
       stopEarly: true
     }
   })
+  const { tConsole, tStdout, tStderr } = StrConsole(!options.silent)
   if (options.help) {
     tConsole.log(`
-       usage: bajel[-n][-p][-h][target]
-       -n  dry run
-       -p  print out the expanded build file
-       -d  debug
-       -t  print out all explicit targets before % expansion
-       -T  print out all targets after % expansion
-       -h  this help
+      Usage: bajel [options] [target]
+      Options:
+       -d, --debug            Explain how decisions are made.
+       -h  --help             Show this help.
+       -n, --dry-run          Don't execute the recipes.
+       -s, --silent           Don't echo anything to stdout or stderr.
+       -p  --print            Print the expanded build file
+       -T  --targets-expanded Print out all targets after % expansion.
+       -t, --targets          Print out all explicit targets before % expansion
        `)
     return [0, tStdout(), tStderr()]
   }
@@ -92,7 +96,12 @@ module.exports = async (bajelfile) => {
 
   try {
     // Phase 3: Executions
-    const { code, updatedTime, recipeHappened, result } = await tasks.recurse([], start, dryRun, options.debug)
+    const {
+      code,
+      updatedTime,
+      recipeHappened,
+      result
+    } = await tasks.recurse([], start, dryRun, options.debug)
 
     if (code !== 0) {
       tConsole.error(`bajel: recipe for target '${start}' failed\nbajel: *** [error] Error ${code}`)
@@ -110,7 +119,7 @@ module.exports = async (bajelfile) => {
     }
     return [0, tStdout(), tStderr(), result]
   } catch (e) {
-    console.error(e)
+    // console.error(e)
     tConsole.error(e.toString())
     return [1, tStdout(), tStderr(), e.toString()]
   }

src/teeconsole.js

@@ -4,24 +4,28 @@ const { Writable } = externalRequire('stream')
 const { Console } = externalRequire('console')
 
 /**
- * @param {!Object} wrapped
- * @return {!Object}
+ * @returns {!Object}
  */
-const TeeStreamToString = wrapped => {
-  let string = ''
-  const stream = Writable()
-  stream._write = (chunk, enc, next) => {
-    wrapped._write(chunk, enc, next)
-    string += chunk.toString()
+module.exports = (enableWrapped = true) => {
+  /**
+   * @param {!Object} wrapped
+   * @return {!Object}
+   */
+  const TeeStreamToString = wrapped => {
+    let string = ''
+    const stream = Writable()
+    stream._write = (chunk, enc, next) => {
+      if (enableWrapped) {
+        wrapped._write(chunk, enc, next)
+      } else {
+        next()
+      }
+      string += chunk.toString()
+    }
+    const toString = () => string
+    return { stream, toString }
   }
-  const toString = () => string
-  return { stream, toString }
-}
 
-/**
- * @returns {!Object}
- */
-module.exports = () => {
   const stdout = TeeStreamToString(process.stdout)
   const stderr = TeeStreamToString(process.stderr)
   const tConsole = new Console(stdout.stream, stderr.stream)

test/_test_helper.js

@@ -6,6 +6,8 @@ const os = require('os')
 const path = require('path')
 // const { pp } = require('passprint')
 
+process.argv.push('-s')
+
 module.exports.buildFileTree = async (spec) => {
   const folder = await mkdtemp(path.join('.', 'test-'))
   for (const filename in spec) {

test/abc_test.js

@@ -62,7 +62,7 @@ test('abc existing', async t => {
     t.deepEqual(stdout + stderr.toString(),
       expected(folder)
     )
-    const abcab = await fs.readFileSync(`${folder}/abcab`, 'utf8')
+    const abcab = fs.readFileSync(`${folder}/abcab`, 'utf8')
     t.deepEqual(abcab, 'aaabbbcccaaabbb')
     t.deepEqual(0, code)
   } finally {
@@ -84,7 +84,7 @@ test('abc existing var', async t => {
     t.deepEqual(stdout + stderr.toString(),
       expected(folder)
     )
-    const abcab = await fs.readFileSync(`${folder}/abcab`, 'utf8')
+    const abcab = fs.readFileSync(`${folder}/abcab`, 'utf8')
     t.deepEqual(abcab, 'aaabbbcccaaabbb')
     t.deepEqual(0, code)
   } finally {
@@ -108,7 +108,7 @@ test('abc generated', async t => {
     t.regex(out, /^echo "Aaa" > .*\/a$/m)
     t.regex(out, /^echo "Bbb" > .*\/b$/m)
     t.regex(out, /^echo "Ccc" > .*\/c$/m)
-    const abcab = await fs.readFileSync(`${folder}/abcab`, 'utf8')
+    const abcab = fs.readFileSync(`${folder}/abcab`, 'utf8')
     t.deepEqual(abcab, 'Aaa\nBbb\nCcc\nAaa\nBbb\n')
     t.deepEqual(0, code)
   } finally {
@@ -135,7 +135,7 @@ test('abc generated var', async t => {
     t.regex(out, /^echo "Aaa" > .*\/a$/m)
     t.regex(out, /^echo "Bbb" > .*\/b$/m)
     t.regex(out, /^echo "Ccc" > .*\/c$/m)
-    const abcab = await fs.readFileSync(`${folder}/abcab`, 'utf8')
+    const abcab = fs.readFileSync(`${folder}/abcab`, 'utf8')
     t.deepEqual(abcab, 'Aaa\nBbb\nCcc\nAaa\nBbb\n')
     t.deepEqual(0, code)
   } finally {

test/bad_test.js

@@ -1,5 +1,6 @@
 const test = require('ava')
 const build = require('../src/index.js')
+require('./_test_helper.js')
 
 const collect = ([code, stdout, stderr]) => ({ code, stdout, stderr })
 

test/call_test.js

@@ -1,6 +1,7 @@
 const test = require('ava')
 const build = require('../src/index.js')
 const fs = require('fs')
+require('./_test_helper.js')
 // const { pp } = require('passprint')
 
 test.beforeEach(t => {

test/cli_test.js

@@ -1,5 +1,6 @@
 const test = require('ava')
 const build = require('../src/index.js')
+require('./_test_helper.js')
 
 const exec = `
   cd $@

test/console_test.js

@@ -2,6 +2,7 @@ const test = require('ava')
 const build = require('../src/index.js')
 const fs = require('fs')
 const { buildFileTree, writeTmpFile } = require('./_test_helper.js')
+// const { pp } = require('passprint')
 
 test('happy path', async t => {
   const [code, stdout, stderr] = await build(
@@ -43,15 +44,7 @@ test.serial('help text', async t => {
       }
     )
 
-    t.deepEqual(stdout, `
-       usage: bajel[-n][-p][-h][target]
-       -n  dry run
-       -p  print out the expanded build file
-       -d  debug
-       -t  print out all explicit targets before % expansion
-       -T  print out all targets after % expansion
-       -h  this help
-       \n`)
+    t.snapshot(stdout)
     t.deepEqual(stderr, '')
     t.deepEqual(code, 0)
   } finally {

test/file_test.js

@@ -1,5 +1,6 @@
 const test = require('ava')
 const build = require('../src/index.js')
+require('./_test_helper.js')
 
 test('directories are ignored when deps', async t => {
   // Using a target that is the same as an existing directory

test/snapshots/console_test.js.md

@@ -0,0 +1,22 @@
+# Snapshot report for `test/console_test.js`
+
+The actual snapshot is saved in `console_test.js.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## help text
+
+> Snapshot 1
+
+    `␊
+          Usage: bajel [options] [target]␊
+          Options:␊
+           -d, --debug            Explain how decisions are made.␊
+           -h  --help             Show this help.␊
+           -n, --dry-run          Don't execute the recipes.␊
+           -s, --silent           Don't echo anything to stdout or stderr.␊
+           -p  --print            Print the expanded build file␊
+           -T  --targets-expanded Print out all targets after % expansion.␊
+           -t, --targets          Print out all explicit targets before % expansion␊
+           ␊
+    `

test/task_test.js

@@ -1,6 +1,12 @@
 const test = require('ava')
 const Task = require('../src/task.js')
 
+const fakeConsole = {
+  log: () => {},
+  warn: (...args) => { throw new Error(`Unexpected warn(${args.join})`) },
+  error: (...args) => { throw new Error(`Unexpected error(${args.join})`) }
+}
+
 test('toString', t => {
   const task = new Task('aTarget', {
     deps: ['aDep', 'anotherDep'],
@@ -17,7 +23,7 @@ test('call', t => {
     deps: [],
     call: () => 'aResult'
   })
-  const { callHappened, result } = task.doCall(false, console, [])
+  const { callHappened, result } = task.doCall(false, fakeConsole, [])
   t.deepEqual(result, 'aResult')
   t.truthy(callHappened)
 })