feat(server): wait for frameworks to load via q.all() before starting server #1053
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…odules This module is nice, because it allows tests to mock modules which are required() by karma, such as frameworks.
… server The API is to simply return a promise from a factory function, or use a promise as a value. The server will wait for the collection of frameworks to resolve before carrying on starting the server. This is useful for when setup requires some asynchronous startup. Currently, any Promise api which is interoperable with q may be used. In addition to this feature, this CL also adds a set of callbacks to the server, via an object for the 3rd parameter. This is useful for test purposes, and enables the test case to run faster, by not waiting for the server to shut down via the second parameter. Closes karma-runner#851
|
This sounds like a good feature. I need some more time to review it though. |
|
Did you have enough time to review @vojtajina? I kid, but it would be nice to look into this again. |
|
I don't think @vojtajina is going to be reviewing this --- and I can't actually even remember writing this. What was this for again? |
|
@caitp Your original issue was asking for async plugin support, and the discussion ended with a workaround and this PR. |
|
@caitp @PxlBuzzard is this still a feature that is needed? I'm going to close this PR for now, as doesn't merge cleanly anymore. If you want to update it feel free to reopen it and I will review it in a more timely fashion. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The API is to simply return a promise from a factory function, or use a promise as a value. The server will wait for the collection of frameworks to resolve before carrying on starting the server.
This is useful for when setup requires some asynchronous startup.
Currently, any Promise api which is interoperable with q may be used.
In addition to this feature, this CL also adds a set of callbacks to the server, via an object for the 3rd parameter. This is useful for test purposes, and enables the test case to run faster, by not waiting for the server to shut down via the second parameter.
There are some design decisions here which you might not like, such as using
mockeryrather than creating test fixtures outside of the test, or adding an object of callbacks to the server opener to help with testing it (it might be nice to have for other reasons, like bonus logging points or something). And of course, the main changes might be restructured in different ways too. So let me know if there are any issues with it @vojtajina, I suspect you were thinking the CL would be a bit smaller and change fewer things, but testing it seemed like a good thing to do.Closes #851