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

Add custom cargo runners support. #5017

Merged
merged 3 commits into from
Jun 26, 2020
Merged

Conversation

vsrs
Copy link
Contributor

@vsrs vsrs commented Jun 24, 2020

This PR adds an option to delegate actual cargo commands building to another extension. For example, to use a different manager like cross.

https://github.com/vsrs/cross-rust-analyzer is an example of such extension. I'll publish it after the rust-analyzer release with this functionality.

Fixes #4902

@vsrs
Copy link
Contributor Author

vsrs commented Jun 24, 2020

I decided to dwell on a command approach for the moment, for

  1. looks like it works perfectly
  2. unfortunately, I don't have enough time :(

@vsrs
Copy link
Contributor Author

vsrs commented Jun 24, 2020

@matklad looks like there is a floating error in the tests. I did not change the code but the first run failed while the second succeeded.

@matklad
Copy link
Member

matklad commented Jun 24, 2020

Yup, that's due to #5008. We can just revert it, but ti would be cool to debug this, as this shouldn't be happening, as far as my understanding goes.

For heavy tests, we create files in the tmp directory on disk, then crate analyzer instance. The analyzer reads files from disk and reports when it is done. After that, we proceed with a test.

Apparantly, there are some additional vfs events happening after we report project as loaded. But those events should not affect the state of rust-analyzer, and should not cancel in-flight requests.

We used to "open" all the documents, exactly to prevent file system events from affecting the state, but this should no longer be necessary.

@vsrs
Copy link
Contributor Author

vsrs commented Jun 24, 2020

I tried several times to run heavy tests locally on mac, no "luck" they work. Perhaps it makes sense to rollback #5008 to keep CI working.

matklad added a commit to matklad/rust-analyzer that referenced this pull request Jun 24, 2020
This should rid us of the intermittent test failure

rust-lang#5017 (comment)
bors bot added a commit that referenced this pull request Jun 24, 2020
5024: Simplify r=matklad a=matklad



bors r+
🤖

5026: Disable file watching when running slow tests r=matklad a=matklad

This should rid us of the intermittent test failure

#5017 (comment)



bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@matklad
Copy link
Member

matklad commented Jun 26, 2020

Ok, I assume this is fine to experiment, but, long term, the dependency between extensions is clearly going into the wrong direction here, we need to introduce something akin IntelliJ's extension points.

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 26, 2020

@bors bors bot merged commit e07826b into rust-lang:master Jun 26, 2020
@vsrs vsrs deleted the custom_cargo_runners branch June 27, 2020 09:40
bors bot added a commit that referenced this pull request Jun 30, 2020
5101: Add expect -- a light-weight alternative to insta r=matklad a=matklad

This PR implements a small snapshot-testing library. Snapshot updating is done by setting an env var, or by using editor feature (which runs  a test with env-var set). 

Here's workflow for updating a failing test:

![expect](https://user-images.githubusercontent.com/1711539/85926956-28afa080-b8a3-11ea-9260-c6d0d8914d0b.gif)

Here's workflow for adding a new test:

![expect-fresh](https://user-images.githubusercontent.com/1711539/85926961-306f4500-b8a3-11ea-9369-f2373e327a3f.gif)

Note that colorized diffs are not implemented in this PR, but should be easy to add (we already use them in test_utils). 

Main differences from insta (which is essential for rust-analyzer development, thanks @mitsuhiko!):
* self-updating tests, no need for a separate tool
* fewer features (only inline snapshots, no redactions)
* fewer deps (no yaml, no persistence)
* tighter integration with editor
* first-class snapshot object, which can be used to write test functions (as opposed to testing macros)
* trivial to tweak for rust-analyzer needs, by virtue of being a workspace member. 

I think eventually we should converge to a single snapshot testing library, but I am not sure that `expect` is exactly right, so I suggest rolling with both insta and expect for some time (if folks agree that expect might be better in the first place!). 

# Editor Integration Implementation 

The thing I am most excited about is the ability to update a specific snapshot from the editor. I want this to be available to other snapshot-testing libraries (cc @mitsuhiko, @aaronabramov), so I want to document how this works. 

The ideal UI here would be a code action (:bulb:). Unfortunately, it seems like it is impossible to implement without some kind of persistence (if you save test failures into some kind of a database, like insta does, than you can read the database from the editor plugin). Note that it is possible to highlight error by outputing error message in rustc's format. Unfortunately, one can't use the same trick to implement a quick fix. 

For this reason, expect makes use of another rust-analyzer feature -- ability to run a single test at the cursor position. This does need some expect-specific code in rust-analyzer unfortunately. Specifically, if rust-analyzer notices that the cursor is on `expect!` macro, it adds a special flag to runnable's JSON. However, given #5017 it is possible to approximate this well-enough without rust-analyzer integration. Specifically, an extension can register a special runner which checks (using regexes) if rust-anlyzer runnable covers text with specific macro invocation and do special magic in that case. 

closes #3835 


Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
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.

Allow setting a command override for Run similar to Check
2 participants