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

Split parfors tests #7160

Merged
merged 8 commits into from Jun 29, 2021
Merged

Conversation

stuartarchibald
Copy link
Contributor

Work on splitting up the parfors test suite.

@stuartarchibald stuartarchibald added this to the Numba 0.54 RC milestone Jun 28, 2021
@stuartarchibald stuartarchibald added the Effort - long Long size effort needed label Jun 28, 2021
class TestParforsBase(TestCase):

@needs_subprocess
class TestParforsBase(TestCase, MemoryLeakMixin):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class TestParforsBase(TestCase, MemoryLeakMixin):
class TestParforsBase(MemoryLeakMixin, TestCase):

mixin class goes first; earlier in the MRO

Copy link
Member

@sklam sklam left a comment

Choose a reason for hiding this comment

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

There are two minor comments regarding the mixin location and printing failing tests.

@@ -44,6 +46,83 @@
import cmath
import unittest

# NOTE: Each parfors test class is run in separate subprocess, this to reduce
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# NOTE: Each parfors test class is run in separate subprocess, this to reduce
# NOTE: Each parfors test class is run in separate subprocess, this is to reduce

status = subprocess.run(cmd, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, timeout=self._TIMEOUT,
env=env_copy, universal_newlines=True)
self.assertEqual(status.returncode, 0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.assertEqual(status.returncode, 0)
self.assertEqual(status.returncode, 0, msg=status.stderr)

so that we know what tests are failing.

@stuartarchibald
Copy link
Contributor Author

Thanks for the review @sklam, fixes are in e280409.

@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 3 - Ready for Review labels Jun 29, 2021
numba/tests/test_parfors.py Outdated Show resolved Hide resolved
stuartarchibald and others added 2 commits June 29, 2021 17:37
Co-authored-by: Siu Kwan Lam <1929845+sklam@users.noreply.github.com>
Copy link
Member

@sklam sklam left a comment

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 reviewer Waiting for reviewer to respond to author labels Jun 29, 2021
@sklam sklam merged commit 6ec3a37 into numba:master Jun 29, 2021
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 - long Long size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants