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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add test support #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fahrradflucht
Copy link
Contributor

This PR adds support for test execution in an output pane. The commands provided are:

  • Elixir: Test Project
  • Elixir: Run Current Test File
  • Elixir: Run Test at Cursor
  • Elixir: Re-run last test

I think what they do is pretty self explanatory. Sadly currently the output panes have no syntax highlighting.

Please don't feel forced to pull this into the extension if you think this isn't in the scope of the project. In addition to that I would love feedback if you think the way the tests are integrated is handy or not.

If this high-level checks are out of the way please review. 馃槂

There aren't any tests for my code which is sad but I didn't came up with a good way to test this. I have to think about this some more.

NOTE: I updated the extension to newer versions of the vscode library and typescript. Let me know if this is a problem.

Sidenote: I used tslint with some basic rules for my new code but I neither commited the tslint.json not did update the old code. Let me know if you think we should adopt it in some form for the whole project.

Since `vscode` `1.0.0` and Code (the editor itself) `1.6.1` extensions generated
by the Yeoman generator use typescript `2.0.3`.

This commit migrates the extension following the instructions provided here:
https://code.visualstudio.com/updates/v1_6#_extension-authoring
@timmhirsens
Copy link
Owner

@fahrradflucht This looks nice and I would like to pull this in. But have you tested this on Windows? As you can see in elixirServer, launching processes from within node (used?) to require some extra handling for windows.

@fahrradflucht
Copy link
Contributor Author

fahrradflucht commented Mar 5, 2017

No I have not. And sadly I don't have the possibility to do it easily.

FWIW IIRC the problem on windows is primarily if you have spaces in the command elements. This should be no problem here.


Edit 1:
I looked at what you do in Elixir server. Here you replace / with \ which I think isn't necessary here either because I get the file path from vscode which already should have a Windows and not a Unix style file path...


Edit 2:
This by no means should come across like I don't want to add those checks but I think we shouldn't bloat the code with this if it isn't necessary.

@timmhirsens
Copy link
Owner

I will check this on windows once I find the time (and nerve ;) ) to boot into it.

I would like to add your tslint configuration as well. Style looks good to me 馃憤 Could you add that to the PR?

@fahrradflucht
Copy link
Contributor Author

Hmpf I tested it on a Windows 8.1 32bit virtual machine and it is broken 馃槥 .
I have no energy left tonight to debug this but I will do it on the weekend. Then I will also add the tslint configuration.

@timmhirsens
Copy link
Owner

@fahrradflucht: Got a chance to work on this yet?

@fahrradflucht
Copy link
Contributor Author

Sadly no. Can't motivate myself to fix these Windows issues. But I think after this: http://code.visualstudio.com/updates/v1_11#_tasks we should rethink the feature design from the ground up anyway.

@bcardarella
Copy link

@fr1zle would it be possible to get this into the extension without Windows support and have that be a TODO?

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

Successfully merging this pull request may close these issues.

None yet

3 participants