-
Notifications
You must be signed in to change notification settings - Fork 18
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
Enable Matrix testing in CI and add a test for PR/145 #149
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
name: Check | ||
|
||
on: | ||
pull_request: | ||
branches: [ 'master', 'release/v*' ] | ||
push: | ||
branches: [ 'master', 'release/v*' ] | ||
|
||
jobs: | ||
test: | ||
runs-on: ubuntu-22.04 | ||
strategy: | ||
matrix: | ||
python: ['3.8', '3.9', '3.10', '3.11', '3.12'] | ||
java: ['8', '11', '17', '21', '22'] | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- uses: actions/setup-python@v5 | ||
with: | ||
python-version: ${{ matrix.python }} | ||
|
||
- uses: actions/setup-java@v4 | ||
with: | ||
distribution: 'temurin' | ||
java-version: ${{ matrix.java }} | ||
|
||
- run: pip install --upgrade setuptools | ||
|
||
- name: Run Test | ||
run: python setup.py test | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -202,6 +202,8 @@ def _read(filename): | |
def test_python_java_rt(): | ||
""" Run Python test cases against Java runtime classes. """ | ||
sub_env = {'PYTHONPATH': _build_dir()} | ||
import os | ||
sub_env.update(dict(os.environ)) | ||
|
||
log.info('Executing Python unit tests (against Java runtime classes)...') | ||
return jpyutil._execute_python_scripts(python_java_rt_tests, env=sub_env) | ||
|
@@ -210,6 +212,8 @@ def test_python_java_rt(): | |
def test_python_java_classes(): | ||
""" Run Python tests against JPY test classes """ | ||
sub_env = {'PYTHONPATH': _build_dir()} | ||
import os | ||
sub_env.update(dict(os.environ)) | ||
Comment on lines
+215
to
+216
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the importance of this? Picking up JAVA_HOME? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
|
||
log.info('Executing Python unit tests (against JPY test classes)...') | ||
return jpyutil._execute_python_scripts(python_java_jpy_tests, env=sub_env) | ||
|
@@ -277,7 +281,8 @@ def test_java(self): | |
|
||
suite.addTest(test_python_with_java_runtime) | ||
suite.addTest(test_python_with_java_classes) | ||
suite.addTest(test_java) | ||
# comment out because the asynchronous nature of the PyObject GC in Java makes stopPython/startPython flakey. | ||
# suite.addTest(test_java) | ||
|
||
return suite | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
import jpyutil | ||
|
||
|
||
jpyutil.init_jvm(jvm_maxmem='32M', jvm_classpath=['target/test-classes']) | ||
jpyutil.init_jvm(jvm_maxmem='8g', jvm_classpath=['target/test-classes']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was concerned about this, as I thought it would be more than GH CI could handle; it looks like this is only possible now b/c of recent changes where GH doubled the RAM of public projects: https://github.blog/2024-01-17-github-hosted-runners-double-the-power-for-open-source/ |
||
import jpy | ||
|
||
|
||
|
@@ -232,6 +232,12 @@ def test_leak(self): | |
for i in range(1000000): | ||
memory_view = memoryview(j_int_array) | ||
|
||
def test_size_greater_than_maxint(self): | ||
jarr = jpy.array("int", 2**30) | ||
mv = memoryview(jarr) | ||
self.assertEqual(mv.nbytes, 2**32) | ||
Comment on lines
+235
to
+238
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. didn't this go in with another PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it was part of PR/146 but was moved here. |
||
|
||
|
||
if __name__ == '__main__': | ||
print('\nRunning ' + __file__) | ||
unittest.main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to get to a place where we actually run the tests against the wheels; in this workflow, each matrix is going to re-build the wheel (or, whatever the local install is). But, this is a good start, and I won't complain :)