Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Test running functionality #386

Closed
Blond11516 opened this issue Oct 13, 2020 · 8 comments · Fixed by #389
Closed

Feature: Test running functionality #386

Blond11516 opened this issue Oct 13, 2020 · 8 comments · Fixed by #389
Labels
enhancement New feature or request

Comments

@Blond11516
Copy link
Contributor

Hi! I'm not sure if this issue should be opened here or on the elixir-ls repository. Please let me know if this is not the right place and I'll gladly move it.

I've recently been playing around with adding test running functionality to the extension. My first step is to have the extension provide code lenses to automatically run tests on click (like Jest Runner does).

I've got a simple POC which works on the following levels:

  • A new function in ElixirSense returns all of a file's metadata (I'm guessing this is probably not required, but it was quick and good enough for a POC)
  • ElixirLS uses that metadata to identify all calls to the functions "test" and "describe" in files for which the path contains "test". Code lenses are returned for each of these calls, as well as for the first line of the file (to run all tests).
  • VSCode ElixirLS provides a command to run tests based on a provided filter in a terminal, which is invoked upon clicking the code lens.

Before going any further with this I wanted to check for interest for such a feature and gather comments on a good implementation.

@axelson axelson transferred this issue from elixir-lsp/vscode-elixir-ls Oct 13, 2020
@lukaszsamson
Copy link
Collaborator

@Blond11516 sounds great. Please go on with the PRs

@axelson
Copy link
Member

axelson commented Oct 13, 2020

I've moved this to the elixir-ls repository since I think it fits better here.

I think this would be a great enhancement! And I definitely like that ElixirLS would be returning all the potentials locations that could be tested.

Some thoughts:

  • Ideally we could support other testing frameworks than pure exunit, for example https://github.com/whatyouhide/stream_data
  • It would be great if we could execute the tests on the server-side but I'm not sure we could show the results to the user in a helpful way, so it's probably to leave that as future work (if ever)
    • But the benefit would be to get the test-running support to work for any LSP-aware editor instead of only VSCode ElixirLS

@lukaszsamson/@msaraiva do you have any thoughts on the ElixirSense/finding all the tests for a given file?

@lukaszsamson
Copy link
Collaborator

It would be great if we could execute the tests on the server-side

I'd avoid executing client code in ls process

do you have any thoughts on the ElixirSense/finding all the tests for a given file?

Maybe full metadata is not necessary and we could do a simple test extraction similarly to what DocumentSymbolsProvider does.

@Blond11516
Copy link
Contributor Author

It would be great if we could execute the tests on the server-side

I've seen it done (see the Java Test Runner as an example) but you do lose access to the terminal and you have to come up with some other way of displaying test results. So if we wanted to do it, it would require quite a bit more work.

@lukaszsamson
Copy link
Collaborator

It would be great If we could support running a single test with debugger also. IIRC there were some feature requests for that.

@Blond11516
Copy link
Contributor Author

Ideally we could support other testing frameworks than pure exunit, for example whatyouhide/stream_data

I've been looking into this for the past few days and the only way I've found to do this is to manually add checks for the libraries we'd like to support. I've been playing around with Elixir Sense's expand_full function to see if there's something common to all test macros that we could use, but I couldn't get it to expand test, which I'm assuming is normal, but I don't understand ElixirSense (or macro expansion) enough to say why.

It would be great If we could support running a single test with debugger also

Yeah that was on my todo list too. From what I've seen from other extensions this should be fairly easy to do once the test running functionality is done.

I've also been looking for a more robust way to identify runnable tests. By directly playing with ElixirSense's parser and metadata I can fairly easily check whether a module imports ExUnit.Case (and other related modules), which I think would be a fairly good way to identify if a file should be considered as "test file" (although not so clean). From there, as stated previously, I haven't been able to find a more robust or generic way of checking for test calls.

I've opened a draft PR so you can see where I'm at right now: #389

@Blond11516
Copy link
Contributor Author

Thanks a lot @axelson and @lukaszsamson for your help on this. This felt a little big and scary at first but your feedback helped me a ton along the way!

I'm really motivated to improve on this, so you can expect to hear from me again in the future ;)

@axelson
Copy link
Member

axelson commented Dec 4, 2020

Congrats on getting in! We're glad to have a contribution from you and we look forward to more in the future! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants