Skip to content

Migrate to vscode-test #315#317

Merged
andycraig merged 13 commits intoREditorSupport:masterfrom
andycraig:migrate-vscode-test
May 9, 2020
Merged

Migrate to vscode-test #315#317
andycraig merged 13 commits intoREditorSupport:masterfrom
andycraig:migrate-vscode-test

Conversation

@andycraig
Copy link
Copy Markdown
Collaborator

Closes #315

What problem did you solve?

Unit tests were not working. This PR migrates unit tests to vscode-test. Now unit tests work again.

BUT, they only work from the command line. I could not get them to work from within VS Code. So, I have removed the 'Run Tests' option within VS Code for now. I would like to restore it at some point, but this PR at least gives us a way of running tests (which should also work in Continuous Integration).

How can I check this pull request?

In terminal, in vscode-R directory:

npm run test

The first time, it will take a while because it needs to download VS Code.

Eventually, observe output ending in:


    ✓ Selecting multi-line bracket with pipe
    ✓ Selecting shorter RStudio comparison

  22 passing (143ms)

Exit code:   0
Done

If this doesn't work, it may be necessary to run npm install first.

@andycraig
Copy link
Copy Markdown
Collaborator Author

lintr CI test is failing but during the installation phase. Maybe some package repo is offline. I will try running it again later.

@renkun-ken
Copy link
Copy Markdown
Member

Thanks for the nice work! This works for me now.

I'm wondering if this only works under desktop environment?

Another thing is that it'll create a folder .vscode-test that is not ignored? Should we ignore this folder?

@andycraig
Copy link
Copy Markdown
Collaborator Author

Thanks for testing again! Good point about .vscode-test - I've added it to .gitignore.

I'm wondering if this only works under desktop environment?

Do you mean, whether it would work in a container for CI? I think it would, although there would be some more dependencies.

@renkun-ken
Copy link
Copy Markdown
Member

I make some changes to main.yaml for github actions according to https://github.com/renkun-ken/vscode-R/tree/migrate-vscode-test, and it works nicely now: https://github.com/renkun-ken/vscode-R/runs/658838903?check_suite_focus=true.

I'm not sure if there's a good way to incorporate my changes to main.yaml? Maybe I send a PR from my fork to your fork? Not sure about this.

@renkun-ken
Copy link
Copy Markdown
Member

Just saw #318. I'll file a separate PR for this once this PR is merged.

@andycraig
Copy link
Copy Markdown
Collaborator Author

Nice! Looks like I created an issue just as you solved it 😄

This PR is set to allow edits from maintainers, so you may be able to push directly to this branch. Or, doing a separate PR after is fine too.

@renkun-ken
Copy link
Copy Markdown
Member

Just pushed to your branch and the tests look good.

LGTM now.

@andycraig
Copy link
Copy Markdown
Collaborator Author

Great! Thank you for adding the commits and reviewing.

The lintr installation is still failing but this PR doesn't change any R files anyway, so I'm going to go ahead and merge.

@andycraig andycraig merged commit 98c8e70 into REditorSupport:master May 9, 2020
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.

Migrate tests to vscode-test

2 participants