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

Add a proccess module to the common library #22303

Closed
conectado opened this issue Aug 13, 2018 · 3 comments
Closed

Add a proccess module to the common library #22303

conectado opened this issue Aug 13, 2018 · 3 comments
Labels
test Issues and PRs related to the tests.

Comments

@conectado
Copy link
Contributor

Describe the solution you'd like
I've been reading the Testing for internet and I've seen the function killChildren repeatedly defined over many files, such as test/internet/test-dgram-multicast-set-interface-lo.js and test/internet/test-dgram-broadcast-multi-process.js, and other similar patterns

Describe alternatives you've considered
I was wondering if it was a good idea to add a process module inside the common library so to not repeat code inside the tests. Otherwise we could add a Utils library.

Thanks!

@Trott
Copy link
Member

Trott commented Aug 14, 2018

Tests are the one place where it doesn't always make sense to keep could DRY. (See https://stackoverflow.com/q/6453235/436641 for example.) And this function is tiny. Abstracting it out does not gain us much. It appears in three tests.

I wouldn't stop it if someone wanted to do it, but I wouldn't bother.

@Trott
Copy link
Member

Trott commented Aug 14, 2018

(Actually, I probably would stop it. The test common module is a monolith that needs fewer things in it, not more.)

@maclover7 maclover7 added the test Issues and PRs related to the tests. label Aug 14, 2018
@conectado
Copy link
Contributor Author

Makes sense, I will close the issue. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

3 participants