Codecov Report

Merging #2732 into master will decrease coverage by 0.02%. The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2732      +/-   ##
==========================================
- Coverage   45.77%   45.74%   -0.03%     
==========================================
  Files         361      361              
  Lines       14637    14645       +8     
  Branches     1927     1930       +3     
==========================================
  Hits         6700     6700              
- Misses       7706     7714       +8     
  Partials      231      231

Impacted Files	Coverage Δ
browser/src/neovim/NeovimProcessSpawner.ts	`21.27% <ø> (ø)`	:arrow_up:
browser/src/neovim/NeovimInstance.ts	`5.42% <0%> (-0.06%)`	:arrow_down:
browser/src/Editor/NeovimEditor/NeovimEditor.tsx	`8.56% <0%> (-0.07%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 50d89d1...76d18c7. Read the comment docs.

TalAmuyal commented 5 years ago

@bryphe, I'm trying to get this one to pass. (cc @CrossR ) Up until now, it passed twice for Travis & failed twice for AppVeyor. The AppVeyor failures seems to be consistent - same error for x86 & x64 in both of the tries. On the other hand, it doesn't fail on Travis.

Tried to reproduce on my Windows machine (I have both Win & Mac) but after ~2 hours I still can't pass the yarn install phase. It fails when building bs-platform with:

oni\node_modules\bs-platform\lib\ocaml\block.cmj
fs.js:141
    throw new ERR_INVALID_CALLBACK();
    ^

TypeError [ERR_INVALID_CALLBACK]: Callback must be a function
    at makeCallback (fs.js:141:11)
    at Object.rename (fs.js:594:14)
    at renameAsync (oni\node_modules\bs-platform\scripts\build_util.js:35:5)
    at oni\node_modules\bs-platform\scripts\build_util.js:95:6
    at Array.forEach (<anonymous>)
    at Object.install (oni\node_modules\bs-platform\scripts\build_util.js:83:11)
    at Object.<anonymous> (oni\node_modules\bs-platform\scripts\install.js:169:20)

A) Is the AppVeyor an indication for a Windows-specific error, or is it just flakier? B) Do you know how to handle this build error on my machine?

CrossR commented 5 years ago

I think the Windows CI is flakier, though it would fail before that (usually). I can give it a try later today on my machine.

For Windows, I usually just run through the stuff on https://github.com/onivim/oni/wiki/Development#windows, and that lets me build it. The most awkward step is the Windows Build Tools, but once I get past that, it should be fine.

TalAmuyal commented 5 years ago

Indeed npm install --global --production windows-build-tools helped getting through, but then I got to the current error :(

My guess is a version incompatibility between bs-platform & my local node installation (though I tried both node-v10.15.3-x64 & node-v11.11.0-x64 with no success).

bryphe commented 5 years ago

Thanks @CrossR & @TalAmuyal for investigating this!

If the bs-platform dependency is problematic, I think we can remove it. It was a first step towards getting first-class reason support, but we never got it 100% - so I'm OK pulling that dependency and test out (we'll do better in Oni2 by integrating vscode-reasonml directly).

bryphe commented 5 years ago

FYI I merged in #2735 - hopefully that helps unblock the investigation

TalAmuyal commented 5 years ago

Thanks, @bryphe ! It helped so it is now installed and running. The unit-tests pass locally, ~but the integration tests fail both on this branch and on master with the following error: (node:12044) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 blur listeners added. Use emitter.setMaxListeners() to increase limit, does it look familiar?~

I tried:

yarn install
yarn run build
yarn run test:unit:browser
yarn run test:integration

And it all passes locally, any ideas on how to reproduce?

onivim / oni

CrossR: Support nvim 0.3.2 #2732

Codecov Report