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

Added a wait option to be used in combination with background:true #57

Closed
wants to merge 3 commits into from

Conversation

mokkabonna
Copy link

This allows you to run several karma tasks in the same queue without getting multiple "port in use" warning.

https://travis-ci.org/mokkabonna/knockout.bindingHandlers.number#L2335

Also causes possible eventemitter memory leak warnings

https://travis-ci.org/mokkabonna/knockout.bindingHandlers.number#L2174

This would preferrably be implemented without needing it to run in the background. If karma supported being stopping without the whole process exiting then you could implement it like so:

server.start(data, function(code){
    server.stop();
    done(code === 0);
});

You would also get the benefit of getting the output from karma as it comes, not all at once like now with this wait option.

But until then I hope this is something you could want to merge.

If so let me know and I'll add the docs for the wait option as well.

@mokkabonna
Copy link
Author

Now you get the output as it comes.

@mokkabonna
Copy link
Author

Here is a build that uses the changes: https://travis-ci.org/mokkabonna/knockout.bindingHandlers.number/builds/10857212

Notice no warn errors as it spawns and waits each time. So we don't have several karma servers running.

@eugeneware
Copy link

Just what the doctor ordered. Thanks.
👍

@mokkabonna
Copy link
Author

Is this something that could be considered? Been silent here. For instance the default setup for genrator-angular outputs

WARN [karma]: Port 9876 in use
WARN [karma]: Port 9877 in use

etc after each run, since it does that if you run grunt serve and triggers another karma server on watch.

Merged back from master to get the travis config, and it passes now.

@dignifiedquire
Copy link
Member

@mokkabonna sorry for the long silence and thanks for your contribution.
If you are still interested here are a few things,

Thanks

@mokkabonna
Copy link
Author

Not really a priority for me anymore. Anyone else is welcome to continue the work.

@mokkabonna mokkabonna closed this Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants