CI Server Updates

[CrossR]

Updated ensureProcessNotRunning to run on both Nvim and Oni processes.

May help with the CI server issues on Mac (or at least killing the processes, won't change whatever bug/weirdness is causing it).

EDIT:

• Additionally, I added a skip to travis for MD only files.
• I also updated the CheckBinariesForBuild script to check for nvim binaries on Windows.
• Skip screenshots for PRs (Its failed a few times and seems unnecessary to me for a PR)
• Use Neovim 0.3.0 on Linux.
• Add mac OS 10.11 build (currently fails)

[CrossR]

Guess it would be too easy for the Mac build to fail and let me see that the code changes are working...

It at least didn't break the builds!

Might do a few other tiny changes in this branch, to see if I can get the code changes to trigger.

[codecov]

Codecov Report

Merging #2346 into master will increase coverage by 0.64%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #2346      +/-   ##
=========================================
+ Coverage   37.55%   38.2%   +0.64%     
=========================================
  Files         299     300       +1     
  Lines       12420   12518      +98     
  Branches     1640    1649       +9     
=========================================
+ Hits         4664    4782     +118     
+ Misses       7507    7481      -26     
- Partials      249     255       +6

Impacted Files	Coverage Δ
browser/src/Platform.ts	30% <0%> (-3.34%)	⬇️
browser/src/UI/components/CommandLine.tsx	30.43% <0%> (-2.9%)	⬇️
browser/src/Services/Sidebar/index.ts	36.84% <0%> (-2.05%)	⬇️
browser/src/Editor/NeovimEditor/NeovimSurface.tsx	63.63% <0%> (-1.59%)	⬇️
browser/src/Services/Sidebar/SidebarStore.ts	11.42% <0%> (-1.21%)	⬇️
browser/src/UI/components/ExternalMenus.tsx	57.14% <0%> (-1.2%)	⬇️
...ervices/Configuration/FileConfigurationProvider.ts	22.89% <0%> (-1.11%)	⬇️
browser/src/UI/components/Tabs.tsx	31.06% <0%> (-0.62%)	⬇️
...rowser/src/Services/WindowManager/WindowManager.ts	67.27% <0%> (-0.32%)	⬇️
browser/src/Services/Learning/Tutorial/Notes.tsx	52.45% <0%> (-0.22%)	⬇️
... and 19 more

---

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 82faff6...b303447. Read the comment docs.

[CrossR]

@bryphe, I've updated the check in CheckBinariesForBuild to include Windows, which I think is the intended result. There was occasionally errors where the cache files would be restored fine, but Windows was only restoring the ripgrep binaries, so it would fail to launch nvim.

If I'm missing something and that isn't the intended behaviour, I can restore it.

[CrossR]

I've also moved the Demo screenshot out of PR builds. Since it was labelled demo I assume thats fine (and I don't see much benefit in us building a screenshot for PRs), as it was failing occasionally and stopping the build.

If its actually performing something useful for testing, I can add it back.

[CrossR]

Finally got the Mac build to call the changed code, and it looks like the process stopping works fine, but now that Oni is being killed it causes an issue with the following code (I'm assuming it is this code as the other promise this races against is just a sleep)

const promise1 = this._app.stop().then(
                    () => {
                        log("_app.stop promise completed!")
                        didStop = true
                    },
                    err => {
                        // tslint:disable-next-line
                        console.error(err)
                    },
                )

- Force close complete
- Calling _app.stop
- Racing with 15s timer...
{ Error: Request timed out after the element was still found on the page.
    at windowHandles() - application.js:245:17
  details: undefined,
  message: 'chrome not reachable',
  type: 'RuntimeError',
  seleniumStack: 
   { type: 'SelectorTimeoutError',
     message: 'Request timed out after the element was still found on the page.',
     orgStatusMessage: 'chrome not reachable\n  (Session info: content shell=)\n  (Driver info: chromedriver=2.29 (8e8216e581c512667203931f81c1a1ead47222e5),platform=Mac OS X 10.12.6 x86_64)' } }
- Race complete. didStop: false

What is interesting is that if you look at the final try to quit out (https://travis-ci.org/onivim/oni/jobs/395803384#L3831) we can see that it fails one last time, and moves to the next test, then in that next test, the promise seems to complete:

[379.069] - Calling oni.start
isCiBuild: true
Using executable path: /Users/travis/build/onivim/oni/dist/mac/Oni.app/Contents/MacOS/Oni
Using executable args: 
Start options: {"env":{"ONI_CONFIG_FILE":"/var/folders/bb/n7t3rs157850byt_jfdcq9k80000gn/T/config_1529756087313.js"}}
Oni starting...
_app.stop promise completed!

Not fully sure how that promise completed that much further into the future, when the next test had started.

[CrossR]

@bryphe do you have any input on what a suitable timeout for the CI tests is?

Currently we have two values I think, which causes issues like in https://travis-ci.org/onivim/oni/jobs/395911164#L3702.

runInProcTest.ts runs the tests with a 300000ms timeout but the actual node call defined in the package.json uses a 120000ms timeout. Ideally it would never be reached....but its probably best to have the in code one be the shorter one, so a retry can be performed.

[bryphe]

Thanks for all your help investigating here @CrossR ! Really appreciate it 💯 Sorry I've been slow to reply.

I've also moved the Demo screenshot out of PR builds. Since it was labelled demo I assume thats fine (and I don't see much benefit in us building a screenshot for PRs), as it was failing occasionally and stopping the build.

That's reasonable! I was hoping it'd be more stable, but yes, let's move it out - don't want that to hurt stability of the builds. I think @badosu logged an issue tracking this, too.

Not fully sure how that promise completed that much further into the future, when the next test had started.

Hmm, I wonder if Spectron is doing something extra to close or something that causes the promise to resolve?

runInProcTest.ts runs the tests with a 300000ms timeout but the actual node call defined in the package.json uses a 120000ms timeout. Ideally it would never be reached....but its probably best to have the in code one be the shorter one, so a retry can be performed.

Regarding the timeouts - I think we only hit the timeout in the case where there is a hanging process (I could be wrong, though). I'd be okay adjusting these, though. We could bump the code one down to 120000 to match.

[bryphe]

Thanks for fixing this 😄

[bryphe]

Nice catch!

[bryphe]

And ya, I hadn't seen this error before on OSX before, either:

- Racing with 15s timer...
{ Error: Request timed out after the element was still found on the page.
    at windowHandles() - application.js:245:17
  details: undefined,
  message: 'chrome not reachable',
  type: 'RuntimeError',
  seleniumStack: 
   { type: 'SelectorTimeoutError',
     message: 'Request timed out after the element was still found on the page.',
     orgStatusMessage: 'chrome not reachable\n  (Session info: content shell=)\n  (Driver info: chromedriver=2.29 (8e8216e581c512667203931f81c1a1ead47222e5),platform=Mac OS X 10.12.6 x86_64)' } }
- Race complete. didStop: false

But I think it's due to the fact that we kill the Oni process while the chromedriver process is still doing something. The spectron framework we use for testing uses chromedriver to connect to Oni - and whenever we make a call to our automation it goes through chromedriver's protocol. I wonder what happens if we try stopping the chromedriver process as well?

[CrossR]

Not sure how much more I can do on this....

I've added the killing of chromedriver, but haven't managed to get an error since to actually test it. I also added the macOS 10.11 builds as optional for now.

As far as I can tell, the main outstanding issue is that the AppVeyor builds sometimes just don't have the cached binaries since the release isn't downloaded, nor the cache restored. If I'm remembering correctly, the releases sometimes aren't downloaded due to GitHub API rate limiting? But we evidently don't get that on the Linux builds, which also download things.

I could be missing something, but what is stopping us adding a curl/Invoke-WebRequest to the release downloader to get the things we need? Or at least to add optional "if CI use curl else use github-releases". Just missing what makes the Linux downloads and the Mac/Windows downloads different.

The other solution is the way Veonim does it, which is basically how we had it before. The entirety of the binaries is stored in the repo, just in a seperate repo. Then they publish individual npm modules for each platform. Lot less clean...but also doesn't have the issues we are having on Windows CI. https://github.com/veonim/neovim for reference. Failing that, its storing it all somewhere, which I think we'd rather avoid.

[bryphe]

Thanks for all your help with this, @CrossR ! 💯

I could be missing something, but what is stopping us adding a curl/Invoke-WebRequest to the release downloader to get the things we need? Or at least to add optional "if CI use curl else use github-releases". Just missing what makes the Linux downloads and the Mac/Windows downloads different.

Sounds good. I thought that the curl / Invoke-WebRequest might be throttled in the same way, but that might not be case - definitely worth a shot 👍 Can't be worse.

The other solution is the way Veonim does it, which is basically how we had it before. The entirety of the binaries is stored in the repo, just in a seperate repo.

Yes, given all the issues we've had, I'm OK going back to that approach. It's not ideal, but it would definitely be more reliable. We could just stash the binaries in a bin folder in oni-neovim-binaries and oni-ripgrep-binaries and add them as a submodule. It completely gets us out of the caching shenanigans - so I'm cool with moving back to that approach! I wouldn't miss all the cache issues, that's for sure 😄

Thanks again @CrossR - digging into these CI issues is a heroic endeavor!