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 request] Add ability to force rerun of all tests #176

Closed
erikns opened this issue Nov 30, 2017 · 16 comments · Fixed by #674
Closed

[Feature request] Add ability to force rerun of all tests #176

erikns opened this issue Nov 30, 2017 · 16 comments · Fixed by #674

Comments

@erikns
Copy link
Contributor

erikns commented Nov 30, 2017

Would be nice to have a feature (a command for instance) to rerun all tests without stopping and restarting the extension.

@orta
Copy link
Member

orta commented Nov 30, 2017

Sounds good, you're welcome to add a command that does this 👍

@seanpoulter
Copy link
Member

seanpoulter commented Dec 5, 2017

We might be able to interact with Jest in watch mode.

@marcinczenko
Copy link
Member

It would be nice to interact with Jest directly and keep in the watch mode without restarting the Jest process. If I understand it correctly it would require to change jest-editor-support as for now I do not see anything there to support that. If in the mean time you would be ok to have it done by just restarting the jest process, I can create pull request for that. Please take a look at my comment under #179.

@erikns
Copy link
Contributor Author

erikns commented Dec 12, 2017

Interacting with Jest in watch mode was what I was thinking, too. Seems we need a custom runner to achieve this, since jest-editor-support does not support it as far as I can tell. I have been fiddling with the idea a little bit, but I'm having issues with the Jest child process not accepting input on stdin from another node process. Could this be because the stream used to write commands doesn't look like a terminal to Jest? Perhaps it is possible with some sort of terminal emulation in between?

@seanpoulter
Copy link
Member

I'm having issues with the Jest child process not accepting input on stdin from another node process. Could this be because the stream used to write commands doesn't look like a terminal to Jest?

Yep. Have a look at jestjs/jest#5017 from a week ago for the cause of the problems and a proposed workaround.

Seems we need a custom runner to achieve this, since jest-editor-support does not support it as far as I can tell.

It doesn't support interacting with watch mode yet. With the stdin challenge probably resolved (but not tested yet), we can dream up the API. So far, I was thinking the API ...

  • should send an event when we can interact with Jest (or is it always?)
  • should be able to expand the "Watch Usage" prompt to detect available "Watch Plugins"
  • should be able to register "Watch Plugin" handlers that handle a given key/prompt. We know the standard set of options, but Jest is rolling out with a config option that lets users specify watchPlugins as modules that export a key, prompt text, and behaviour.

The "Watch Plugin" handlers ...

  • should be able to work through any UI/prompts, e.g.: test pattern
  • should notify when they're complete, e.g.: back to the "Watch Usage" prompt
  • should be abortable and return to the "watch usage" prompt

@erikns
Copy link
Contributor Author

erikns commented Dec 14, 2017

The workaround proposed by @seanpoulter seems to at least make the jest process accept raw input 👍, so we can probably make it work with that. I can probably have a proposal for a custom runner prototype ready for critique (at least for the basic features like capturing output and rerunning tests) ready sometime late next week. Given we agree on a custom runner being the right approach at least until jest-editor-support gets more flexible (?)

@seanpoulter
Copy link
Member

Based on what I've seen about jest-editor-support in our issues and Jest's issues -- notably jestjs/jest#5048 -- it sounds like we can develop the package as long as we're mindful that it's shared with nuclide and maybe others. How do you feel about looking at jest-editor-support?

In other news, @orta's helped me reach out to another dev who's also working on an API to interact with Jest -- I'll keep you posted and/or hopefully they jump in on the thread. 🤞

@marcinczenko
Copy link
Member

@seanpoulter, @connectdotz in #189, I also hoped to create new abstractions to communicate with Jest process, so if there is yet someone else working on Jest API, do you suggest that I stop for a moment?

@seanpoulter
Copy link
Member

seanpoulter commented Dec 15, 2017

Sorry for the ambiguity. I said "working on an API to interact with Jest" which doesn't say it was specific to jest-editor-support. @marcinczenko , you're the only one working on the way vscode-jest interacts with jest-editor-support. Don't let us slow you down.

What I'm encouraging is that we consider jest-editor-support as our own first-party dependency, despite it being in a third-party repo. The package was pulled over to share with the community.

@marcinczenko
Copy link
Member

Ok, clear. I will then do some work over the weekend.

@seanpoulter
Copy link
Member

Here's an update from @Raathigesh on his efforts to interact with Jest:

I was able to make Jest accept input in watch mode using the createProcess custom argument that is passed to the Runner and then spawning a node-pty process instead of a regular node process.

I couldn't find time to generalise this approach and send a PR to jest-editor-support yet. But i'm willing to help in any way possible.

You can take a look at the approach here https://github.com/Raathigesh/majestic/blob/master/src/renderer/util/Process.ts

This is how I interact with the process while it's in watch mode https://github.com/Raathigesh/majestic/blob/d39a9de6d4f900c1ef185d550bb72e5aecd0ac59/src/renderer/stores/Runner.ts#L201-L236

Fyi, The repo is still a work in progress 😃

@connectdotz
Copy link
Collaborator

these issues share a clear common need: restart jest. There are a few approaches to solve this:

  1. the simple way: create a restart method in vscode-jest by utilizing the existing stop/start methods sequentially, knowing both methods are async in nature, as discussed in Cleanup the Jest process management #189. This doesn't require touching jest-editor-support and should only introduce minor code change in vscode-jest.

  2. the hard way: we could work on the jest side to expose a set of API (facebook/jest#5048) or try to interact with the jest process via stdin/custom-runner mentioned above. Both can indeed solve the restart need, but neither is trivial and could introduce other issues simply because they are new.

I don't know if there are other issues need the additional features provided by the more complex solutions; but if our goal is to provide restart as #176 and #179 requested, IMHO, we should probably start with the simplest solution first, only when it doesn't work then look for the more complex ones...

@ymulenll
Copy link

Could we have a simple command like:

      {
        "command": "io.orta.jest.stop && io.orta.jest.start",
        "title": "Jest: Start Rerun"
      }

Here:
https://github.com/jest-community/vscode-jest/blob/master/package.json#L123

I will appreciate it and I think that more people also, I need to stop a start your (very useful) extension 2 or 3 times a day, a rerun command would be more than welcome.

Thanks!

@marcinczenko
Copy link
Member

@ymulenll we should indeed get back to it as this issue is pretty old ;).

Unless someone has time to search for a more direct integration with Jest, I think this is something we could consider, but then done slightly differently. We already do have functionality to restart the process - when snapshots get updated. So the correct way to restart the JestProcess is by doing:

this.jestProcessManager.stopJestProcess(this.jestProcess).then(() => {
  this.startProcess()
})

Then we can provide a command to do just this and we are almost done.
I know it is a bit rough way of achieving the goal - we effectively restart the whole process.
Unfortunately, I do not have too much time to dive into better integration with Jest - I do not even know what is currently possible, so I would go for simple things for the moment. Maybe this is just fine for the user. This functionality should not normally be used too often (I use the extension all day long on a monorepo and I only occasionally have to restart it).

What are your views on it?

Cheers!

@ymulenll
Copy link

ymulenll commented Jul 20, 2018

Hello @marcinczenko, thanks for your answer,

In my case I use the console to update snapshots instead of using always the vscode-jest button, and in those cases the extension does not rerun so I need to stop and start it in order to make it work (and also if I use the extension button to update snapshot it does not rerun the affected tests). There are other cases when I do a git pull from other partners' work to my branch, and the extension does not rerun neither. And other cases that happen to my team and me that I do not remember all.

A simple rerun command would be appreciated for some cases in which we want to run again all the test cases. (I need to restart the (very useful) extension 2 or 3 times a day).

Thanks!

@marcinczenko
Copy link
Member

marcinczenko commented Jul 27, 2018

I will create a pull request soon. Then other maintainers can comment on it. Or maybe you @ymulenll would like to create pull request? If you like, you are more than welcomed!

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

Successfully merging a pull request may close this issue.

6 participants