WIP - feat(lsp): Run tests with file watcher#27762
WIP - feat(lsp): Run tests with file watcher#27762stefnotch wants to merge 10 commits intodenoland:mainfrom
Conversation
|
|
| file_watcher::PrintConfig::new("Test", false), | ||
| file_watcher::WatcherRestartMode::Automatic, | ||
| self.token.clone(), | ||
| move |flags, watcher_communicator, changed_paths| { |
There was a problem hiding this comment.
I'm not entirely sure which files I should pass to the watcher_communicator, and what to do with the changed_paths. I did look at the other places where the file watcher is used, but there the file patterns are part of the CLI arguments.
Meanwhile here, the list of included files are set through the included and excluded tests. Any pointers would be very appreciated.
| let line_number = user_frame.line_number.map(|v| v - 1)? as u32; | ||
| let column_number = | ||
| user_frame.column_number.map(|v| v - 1).unwrap_or(0) as u32; | ||
| Some(TestLocation { |
There was a problem hiding this comment.
This info lets us show an indicator at the assertEquals() that actually failed.
I couldn't really find a better way of figuring out where in the user code the error happened. Is this really it?
| deno_unsync = { workspace = true, features = ["tokio"] } | ||
| faster-hex.workspace = true | ||
| flate2 = { workspace = true, features = ["zlib-ng-compat"] } | ||
| flate2 = { workspace = true, features = ["default"] } |
There was a problem hiding this comment.
This change is one of the changes that I'll get rid of when rebasing and fixing the PR. Currently that's just to not require the CMake dependency when developing locally.
|
|
||
| [dev-dependencies] | ||
| libuv-sys-lite = "=1.48.2" | ||
| libuv-sys-lite = { version = "=1.48.3", features = ["dyn-symbols"] } |
There was a problem hiding this comment.
This change will hopefully not be needed much longer. Hopefully Deno will update its libuv-sys-lite dependency soon :)
|
Looks like Deno has been updated a bit since then. I'll happily update this PR to the latest version if that helps with reviewing it. |
|
Hey @stefnotch, sorry for a slow response. Could you give me some context what this PR is trying to achieve? Seeing changes to so many |
|
Hi! Thank you for looking at this PR. My goal is to add the equivalent of the The Cargo.toml changes are unrelated, they're just because I really struggled with getting Deno to build on my Windows system. I'll remove those, sorry for accidentally including them.
The The biggest other struggle that I have is that re-implementing the entire watch logic means duplicating quite a bit of nontrivial code. See also #29132 |
|
Okay, I started pulling out an unrelated change to #29221 I also finally got Deno to compile on my Windows system. Only involved
Hopefully we'll someday have a world with fewer C++ dependencies. I miss the nice Cargo guarantee of "clone git repository, |
3 participants
This addresses issue denoland/vscode_deno#1132
See linked PR for more context denoland/vscode_deno#1243
This pull request is not ready for merging yet. I opened it so that I can ask questions about my code, and about how to best achieve certain results.
I did my best to implement the following
I did not yet
changed_filesof the file watcherMy questions are left as comments in the code.
Also, this is more of a personal struggle: I initially really struggled to locally build Deno, as Deno requires some dependencies that are nontrivial to install on Windows.
The instructions were incomplete #27535 Upon updating them, nathanwhit figured out how to change libuv-sys-lite to no longer require the LLVM dependency, which was very nice.
The CMake dependency for libz-sys is still there, and it still causes me troubles, so I tweaked it (
zlib-ng-no-cmake-experimental-community-maintained). My hope is that zlib will eventually get replaced by zlib-rs, which just works.