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

[WIP] Run tests in a single process #1645

Closed
wants to merge 2 commits into from
Closed

Conversation

vadimdemedes
Copy link
Contributor

@vadimdemedes vadimdemedes commented Jan 21, 2018

Fixes #1332.

Note, that there's no sandboxing like in Jest. This PR runs tests in the same AVA process, similar to Mocha. Feel free to try it out in your own projects, propose better solutions/code and generally tell us what you think.

TODO:

  • Fix existing tests.
  • Add new tests.
  • Discuss how to ship it (name of the flag / make it default / experimental flag / etc).
  • Capture console.log() and send its args to lib/fork for proper output, like in fork mode. I'd rather support Make t.log() behave like console.log() #1635 instead.
  • Detect which dependencies belong to other test files in dependency tracking (lib/process-adapter).
  • Fix @std/esm support (doesn't work, because lib/main doesn't have access to module.parent.parent, which is undefined).

@novemberborn
Copy link
Member

What do you think about still running the tests in a separate worker process, but reusing that process?

@vadimdemedes
Copy link
Contributor Author

Interesting idea, I think it would help with correct console.log() handling.

@novemberborn
Copy link
Member

It also protects the main process from any shenanigans the tests may be up to. We'd still get process concurrency.

@sindresorhus
Copy link
Member

What do you think about still running the tests in a separate worker process, but reusing that process?

I think reusing could be useful for multi-process too, but this PR should focus on only the same process. The use-cases are things that need to be run in the same process as the AVA CLI, for example, debugging or Electron. So I'd rather have multi-process mode reuse processes and let this one be without any process forking. If users want reuse of a single process, they can just set concurrency to 1 in multi-process mode.

@novemberborn
Copy link
Member

The use-cases are things that need to be run in the same process as the AVA CLI, for example, debugging or Electron.

Interesting, we can start with top-level process then.

I think rather than mimicking the rather low-level fork(), we should abstract the "pool" and how it communicates with the API and run-status.

@novemberborn
Copy link
Member

I think rather than mimicking the rather low-level fork(), we should abstract the "pool" and how it communicates with the API and run-status.

I've been doing a fair bit of refactoring in this area, and have some ideas on how to make run-status better too. That should make this feature easier to implement.

@vadimdemedes
Copy link
Contributor Author

I think rather than mimicking the rather low-level fork(), we should abstract the "pool" and how it communicates with the API and run-status.

That doesn't sound like an easier way to implement this right now to me :) This PR isn't that complex, is it? What improvements specifically do you have in mind?

@novemberborn
Copy link
Member

What improvements specifically do you have in mind?

Getting rid of the various events that are emitted and forwarded and rewritten and reemitted, as well as the stats counting and test tracking we do all over the place.

@vadimdemedes
Copy link
Contributor Author

But this PR doesn't change events, it just switches process.on and process.send with adapters. Refactoring that you have in mind could be done afterwards with the same outcome imo.

@novemberborn
Copy link
Member

I'd really like this to be a first class implementation, rather than a mimicking of how fork and test-worker interact. Cause I've been looking a lot at that code lately and I still can't load it all into my head.

I have been doing and will continue to do refactoring to clear up the existing code, and hopefully that makes it relatively straight-forward to do that first class implementation. It's definitely in the back of my mind.

@vadimdemedes
Copy link
Contributor Author

I'd really like this to be a first class implementation, rather than a mimicking of how fork and test-worker interact. Cause I've been looking a lot at that code lately and I still can't load it all into my head.

Definitely agree with this. Ok then, we can put this PR on hold for now, I'm super busy lately anyway.

@novemberborn
Copy link
Member

Hey @vadimdemedes, this is quite outdated by now. Going to close if you don't mind 😄

@vadimdemedes vadimdemedes deleted the single-process branch May 30, 2018 14:45
@vadimdemedes
Copy link
Contributor Author

@novemberborn Of course ;)

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.

3 participants