Fix native zip

[remithomas]

• The native zip action wasn't used, this fixes this issue nativeZip is never called #12 .
• Rewrite the call of the spawn method for native zip

[peacechen]

Thanks @remithomas

I haven't had a chance to test on various platforms (Win, OSX, Linux). Would be happy to merge once someone validates that.

[nfriedly]

We should really set up https://docs.travis-ci.com/user/multi-os/ and https://www.appveyor.com/

[nfriedly]

Thanks again! This is obviously an important fix, but it's causing a couple of test failures that need to be addressed. More comments below.

[nfriedly]

This passes in the exit code as the first argument, but the first argument returned to the user should be an error or null.

Come to think of it.. we should add a zip.on('error', ...); to capture any errors.

Also, lets use the close event rather than exit - because it guarantees that all underlying resources have been closed. https://nodejs.org/dist/latest-v10.x/docs/api/child_process.html#child_process_class_childprocess

So, this is my variation:

function nativeZip(dest, source, done) {
  var args = ["--quiet", "--recurse-paths", dest].concat(source);
  var zip = cp.spawn("zip", args);
  var error = null;
  zip.on("error", function(err) {
    error = err;
  });
  zip.on("close", function(code) {
    if (code !== 0 && !error) {
      error = new Error(
        "Unexpected exit code from native zip command: " + code
      );
    }
    if (error) {
      done(error);
    } else {
      done();
    }
  });
}

That allows some of the tests to pass, but there is still one failure:

  1) bestzip
       When archiving a dummy file
         Valid archive
           should contain valid data after unarchive:
     Uncaught AssertionError: expected [Error: ENOENT: no such file or directory, open '/Users/nathan.friedly/projects/node-bestzip/test/validArchiveExtract/file1.js'] to not exist
      at ReadFileContext.callback (test/bestzip.js:108:35)
      at FSReqWrap.readFileAfterOpen [as oncomplete] (fs.js:420:13)

The wording of that message is a little confusing, but basically the test expected the file to be extracted to ./file1.js but it actually gets extracted to ./Users/nathan.friedly/projects/node-bestzip/test/fixtures/file1.js

Basically, the archive that macOS's native zip creates when passed an absolute path includes that entire file path! So, we need to do something to normalize that so that both node and native produce archives that contain the same file structure from a given input. Perhaps strip the CWD from the beginning of the input strings?

[nfriedly]

Actually, on second thought, I think nodeZip should emulate the native behavior. So, the test is wrong, and the nodeZip implimentation is wrong.. and it'll be a major version bump :/

[remithomas]

You right! we should emulate it !
Yes this is major bump.

We should add this in todo list and close this one ?

[nfriedly]

I was thinking just extend this one - make the test expect the native behavior, and then update the node zip to pass the test.

[nfriedly]

Do you have time available to work on that?

[nfriedly]

yay!