Skip to content

Add decorator to run a test in a subprocess. - #8239

Merged
sklam merged 3 commits into
numba:mainfrom
stuartarchibald:wip/subproc_tests
Jul 20, 2022
Merged

sklam merged 3 commits into
numba:mainfrom
stuartarchibald:wip/subproc_tests

Conversation

@stuartarchibald

Copy link
Copy Markdown
Contributor

This adds a decorator method to Numba's TestCase class which
can be used to specify that a test case should be run in a
subprocess. This patch further cleans up the test suite to use
this decorator where it's reasonable to do so.

This adds a decorator method to Numba's `TestCase` class which
can be used to specify that a test case should be run in a
subprocess. This patch further cleans up the test suite to use
this decorator where it's reasonable to do so.
@stuartarchibald
stuartarchibald marked this pull request as ready for review July 11, 2022 13:56
@stuartarchibald
stuartarchibald requested a review from sklam as a code owner July 11, 2022 13:56
@sklam sklam self-assigned this Jul 12, 2022
@esc esc added this to the Numba 0.57 RC milestone Jul 12, 2022
@esc
esc requested a review from apmasell July 12, 2022 14:34
apmasell
apmasell previously approved these changes Jul 12, 2022
Comment thread numba/tests/support.py Outdated
raise ValueError("Too many args.")
def wrapper(func):
def inner(self, *args, **kwargs):
if os.environ.get("SUBPROC_TEST", None) is None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be "SUBPROC_TEST" in os.environ

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing. I think it's the inverse condition "SUBPROC_TEST" not in os.environ that's needed, but querying the environment dictionary directly makes sense, added in 15fddea.

@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 3 - Ready for Review labels Jul 13, 2022
apmasell
apmasell previously approved these changes Jul 13, 2022

@sklam sklam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two nitpicks

Comment thread numba/tests/support.py Outdated
self.assertNotIn('FAIL', status.stderr)
self.assertNotIn('ERROR', status.stderr)

def run_test_in_subprocess(*maybefunc, timeout=60, envvars=None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking... why make maybefunc vararg?

suggestion:

diff --git a/numba/tests/support.py b/numba/tests/support.py
index 0fe2b32ee..14a8a27b2 100644
--- a/numba/tests/support.py
+++ b/numba/tests/support.py
@@ -586,12 +586,12 @@ class TestCase(unittest.TestCase):
         self.assertNotIn('FAIL', status.stderr)
         self.assertNotIn('ERROR', status.stderr)

-    def run_test_in_subprocess(*maybefunc, timeout=60, envvars=None):
+    def run_test_in_subprocess(maybefunc=None, *, timeout=60, envvars=None):
         """Runs the decorated test in a subprocess via invoking numba's test
         runner. kwargs timeout and envvars are passed through to
         subprocess_test_runner."""
-        if len(maybefunc) > 1:
-            raise ValueError("Too many args.")
+        # if len(maybefunc) > 1:
+        #     raise ValueError("Too many args.")
         def wrapper(func):
             def inner(self, *args, **kwargs):
                 if "SUBPROC_TEST" not in os.environ:
@@ -609,9 +609,8 @@ class TestCase(unittest.TestCase):
                     func(self)
             return inner

-        if maybefunc and isinstance(maybefunc[0], pytypes.FunctionType):
-            func = maybefunc[0]
-            return wrapper(func)
+        if isinstance(maybefunc, pytypes.FunctionType):
+            return wrapper(maybefunc)
         else:
             return wrapper

Comment thread numba/tests/support.py
test_name=func.__name__,
timeout=timeout,
envvars=envvars,)
else:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit weird to allow SUBPROC_TEST=0 running this branch

@sklam sklam added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jul 13, 2022

@sklam sklam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

@sklam sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on author Waiting for author to respond to review labels Jul 20, 2022
@sklam
sklam merged commit 1492ac7 into numba:main Jul 20, 2022
@esc esc modified the milestones: Numba 0.57 RC, Numba 0.56.1 Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge Effort - medium Medium size effort needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants