Skip to content
This repository was archived by the owner on Sep 21, 2019. It is now read-only.

Integration testing setup with server install / launcher tests #312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ches
Copy link
Contributor

@ches ches commented Jul 17, 2016

(Note: this will certainly fail the build until I can update the Docker image with a Neovim installation)

This has a few rough edges which I'll point out inline, but it's ready to poke around with and discuss.

I have added a test setup that integrates against a real Neovim instance, with initial tests of the launcher functionality as a proof of concept. @jvican mentioned this before (#136 (comment)) and I think it's a great idea—Gherkin BDD is really optimized for "full-stack" tests (acceptance, end-to-end, whatever). This isn't 100% "black box" since using the Neovim Python client is RPC versus actually driving a UI, but it's close enough for the depth of coverage we get with such an easy setup, and it can run completely headless for CI. Of course this won't cover the VimL code that's specific to standard Vim—as @jvican noted—but that's a thin layer, and as far as we can trust Neovim's compatibility it comes pretty close since the tests exercise things by executing commands (e.g. :EnInstall), etc.

Things like vimrunner that can script Vim execution with +client-server seem unlikely to be as robust, and I think it's safe to say we would never want to maintain a CI setup that has X and a virtual framebuffer config, so this is probably the best we're reasonably going to do.

One caveat to try to resolve: running the tests sometimes leaves ENSIME server processes running, especially in the headless/embedded mode. I'm not sure why yet.

Probably obvious, but you need Neovim installed with nvim on your path, and sbt.

I made an asciicast, because hey, it's kind of fun to see 😄 Running on a fairly wimpy MacBook Air so the sbt runs are not the fastest.

asciicast

return None

def eval(self, what):
return "/tmp"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were unused at this point, and I think they're most likely to belong in unit tests instead of integration. Stay tuned for some thoughts/work I have toward that.

@jvican
Copy link
Contributor

jvican commented Jan 1, 2017

Hey @ches, the work on this is awesome! What's the status of this PR? Is there something that could be done by an ensime-vim contributor? Do we only need to update the Docker image? It seems that @jqno could do some progress on adding tests for the autocompletion feature.

@fommil
Copy link
Contributor

fommil commented Jan 1, 2017

Is it hard to add neovim to the base ensime image? ensime-docker v2.x

@ches
Copy link
Contributor Author

ches commented Jan 2, 2017

Is it hard to add neovim to the base ensime image?

It shouldn't be, I just never got around to it in part since I don't think anyone else tried this out until now.

ensime-docker v2.x

…but that may (or may not?) be a rub. Last I remember there was some sticky situation about rebuilding the branch of the Docker image that has the server pinned at v1.x. I don't think I ever got a complete understanding of the scheme of how the images are built/tagged from that thread (e.g. manual build-and-push vs. automated builds IIRC), which is also probably another reason I didn't mess with the Docker image here yet…

@fommil
Copy link
Contributor

fommil commented Jan 2, 2017

I think the tldr is: send it to the v2.x branch of the docker image.

@ches
Copy link
Contributor Author

ches commented Jan 2, 2017

What's the status of this PR? Is there something that could be done by an ensime-vim contributor?

@jvican It probably needs a rebase, but it should be a pretty easy one. There was still some fragility in this too (some notes in my hidden comments above, if they make any sense).

Mainly it's just that we never got into any discussion about this PR. I whipped up the branch on a whim when I wanted to play with the Neovim embedded possibility and had some time. I still think an integration testing harness could be useful in the long run, though this initial case (launcher) might be less interesting for the future after #354. If we can get it to be stable though it still serves as an example of the framework that others can follow for other functionality (completion sounds like it could be a good one).

@ches
Copy link
Contributor Author

ches commented Jan 2, 2017

I think the tldr is: send it to the v2.x branch of the docker image.

K, I'll give it a try when I can give it a few minutes' attention. Our builds are currently running against ensime/ensime:v1.x-cache but there might be nothing that breaks on v2.x anyway…

@fommil
Copy link
Contributor

fommil commented Jan 2, 2017

I strongly recommend upgrading to the v2.x-cache images.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants