reuse more kernels in kernel tests #4149
Conversation
|
on my machine, these changes take |
|
confirming the improvement - on my machine, this takes the 38 tests that run on master which take 34 seconds to run, to doing 655 tests which take 17 seconds to run. Just going to look over the code now before merging |
|
Now rebased on #4165. |
| @@ -171,7 +171,7 @@ def signal_kernel(self, kernel_id, signum): | |||
| self.log.info("Signaled Kernel %s with %s" % (kernel_id, signum)) | |||
|
|
|||
| @kernel_method | |||
| def restart_kernel(self, kernel_id): | |||
| def restart_kernel(self, kernel_id, now=False): | |||
There was a problem hiding this comment.
Is this change just for API consistency with another part? Should the extra parameter be described in the docstring?
There was a problem hiding this comment.
it is for API consistency - all MKM methods should be identical to KM methods, adding a leading kernel_id arg.
context managers and tools for starting kernels, and sharing a *single* global kernel for many tests. reduce the number of kernels started for IPython.kernel tests, which should save some time.
| kc = km.client() | ||
| kc.start_channels() | ||
|
|
||
| msg_id = kc.kernel_info() |
There was a problem hiding this comment.
This variable is never used - is that deliberate?
There was a problem hiding this comment.
yes, there's no need to use it. I could validate the subsequent call with it, but I don't think it's necessary.
|
Looks good. |
|
OK, I'm happy with that. Should @ivanov or anyone else look over this? |
|
I just noticed that the test suite in |
|
yay! |
reuse more kernels in kernel tests
reuse more kernels in kernel tests
move scattered kernel-test utilities to kernel.tests.utils
context managers and tools for starting kernels, and sharing a single global kernel for many tests.
reduce the number of kernels started for IPython.kernel tests, which should save some time.