@stefan-guggisberg Mocha 3.0.0 changed the behavior for async tests (mochajs/mocha#2407), you cannot return a Promise and call done() at the same time. This causes two of the tests to fail. The solution seems to be remove the return statements here:

https://github.com/stefan-guggisberg/nodegit/blob/bump_node-pre-gyp/test/tests/filter.js#L107

and here:

https://github.com/stefan-guggisberg/nodegit/blob/bump_node-pre-gyp/test/tests/note.js#L61

Not sure what is causing the third test failure.

@trieloff thanks. i've fixed those tests in a6a4aab.

now running npm test on the branch gives the exact same result as on the latest release (v0.22.2):

  381 passing (4m)
  4 pending
  3 failing

  1) Clone
       can clone with ssh:
     Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/stefan/projects/nodegit-fork/test/tests/clone.js)


  2) Clone
       can clone with git:
     Error: Error receiving socket data: Operation timed out


  3) Remote
       can reject fetching from private repository without valid credentials:

      AssertionError [ERR_ASSERTION]: Should not be able to find repository.
      + expected - actual

      -callback failed to initialize SSH credentials
      +ERROR: Repository not found.

the same tests are failing on master, they're not broken by this PR.

This is redundant of my pull request #1519 which I have referenced in #1499

Maybe its redundant @Croydon but is there a reason we can't separate these changes out, to break down the size of that monster a bit? Like would that cause undue conflicts in your branch if we were to merge this then that?

CC @tbranyen @implausible

It's fine with me when this gets merged first. I just noted that because it sounds like duplication of already done work.

I definitely feel like we should probably target these branches instead of having one mega PR, as well. It's a bit intimidating to look at your PR @Croydon.

@implausible Yes, I totally agree. I did start my pull request a long time ago and it kept growing as I never got Node 10 to actually work.

Please go ahead and merge this.

Rerunning the test that failed. Looked like a weird failure to occur as everything passed except the deploy docs noop step.

@implausible thanks! any chance this gets published on npm anytime soon?

I am hoping to get the node 10 branch wrapped up before we publish the next version, but yes, hopefully soon.