It should be a switch, like --builddir. The way node-gyp works, it can then also be configured by setting npm_config_builddir in the environment. See #916 for an example.

Planning to merge? Would be great to allow it for tensorflow.

@peterbraden Can you add a regression test to test/test-addon.js?

added a very basic test.

@peterbraden Can you rebase and squash? Thanks.

Done

Your test doesn't work, for two reasons: require cache saving "hello_world" and hello.js using the bindings module to load from the existing build directory. So here's a patch to fix your test:

diff --git a/test/test-addon.js b/test/test-addon.js
index 4b8bc5e..2816cd1 100644
--- a/test/test-addon.js
+++ b/test/test-addon.js
@@ -7,6 +7,20 @@ var exec = childProcess.exec
 var path = require('path')
 var addonPath = path.resolve(__dirname, 'node_modules', 'hello_world')
 var nodeGyp = path.resolve(__dirname, '..', 'bin', 'node-gyp.js')
+var rimraf = require('rimraf')
+
+function cleanup (dir) {
+  return function teardown (t) {
+    Object.keys(require.cache)
+      .forEach(function (d) {
+        if (d.indexOf(addonPath) == 0)
+          delete require.cache[d]
+      })
+    rimraf(dir, t.end)
+  }
+}
+
+test('build simple addon - setup', cleanup(path.join(addonPath, 'build')))

 test('build simple addon', function (t) {
   t.plan(3)
@@ -29,6 +43,10 @@ test('build simple addon', function (t) {
   proc.stderr.setEncoding('utf-8')
 })

+test('build simple addon - teardown', cleanup(path.join(addonPath, 'build')))
+
+test('build with different build dir - setup', cleanup(path.join(addonPath, 'foo')))
+
 test('build with different build dir', function(t) {
   var cmd = [nodeGyp, 'rebuild', '-C',
               addonPath, '--loglevel=verbose', '--build-dir=foo']
@@ -39,7 +57,7 @@ test('build with different build dir', function(t) {
     t.strictEqual(lastLine, 'gyp info ok', 'should end in ok')

     try {
-      var binding = require('hello_world')
+      var binding = require(path.join(addonPath, 'foo/Release/hello.node'))
       t.strictEqual(binding.hello(), 'world')

       // Cleanup
@@ -52,3 +70,5 @@ test('build with different build dir', function(t) {
   proc.stdout.setEncoding('utf-8')
   proc.stderr.setEncoding('utf-8')
 })
+
+test('build with different build dir - teardown', cleanup(path.join(addonPath, 'foo')))

ping @peterbraden

Sorry, been busy with a new job. Done now, and squashed and rebased.

Thanks!

Updated, squashed

CI failed on windows, @bnoordhuis could you take a look?

Test failures on Windows are because the newly introduced cleanup() function in the tests is trying to remove a currently loaded .node file. Even if the module is deleted from require.cache the .node is not unloaded (Is it even possible to unload a required .node in Node.js?).

(Is it even possible to unload a required .node in Node.js?)

No.

I don't think clearing entries from require.cache is needed, because the second require() call that was introduced to the test is requiring hello.node from a different location (path.join(addonPath, 'foo/Release/hello.node')) than hello.node that is required from require('hello_world') (via the bindings module) so would end up as a separate entry in the cache.

If the fear is contamination of the second test from the first, then perhaps the new test could be in a separate file? Or does tape run all tests in the same process?

@peterbraden Are you still interested in pursuing this? From #919 (comment) it sounds like it's almost there, it just needs one or two small fix-ups.

If the fear is contamination of the second test from the first, then perhaps the new test could be in a separate file? Or does tape run all tests in the same process?

I think tape uses a process-per-file model so that sounds like a good idea.

Sure, let me try when I have some free time, maybe this weekend.

I found out the hard way today that tape runs all tests in the same process. I've filed #1123.

Has this feature been merged into master? I really need it.

@peterbraden I'd still be interested in this patch if you want to resurrect it :-)

if @peterbraden or someone else wants to pick this up, please either rebase this or take the code and open a new PR against the current master
test failures because of the way tape works should be fixed once we migrate to tap which I hope we can achieve soon.

I don't have time for this any more, hopefully someone else can take this on.

It's with slightly sad eyes that I'm closing this out. I just closed out #263 after no one stepped forward. Thanks anyway, @peterbraden!

I'm interested in helping to get this issue over the line, is the "tap/tape" thing mentioned above still an issue? What remains to be done here.

https://github.com/nodejs/tap2junit