Apply pytest unit testing by Sichao25 · Pull Request #358 · SCOREC/pcms

Sichao25 · 2026-08-14T01:49:46Z

Close #278 . I put the tests in a separate folder called pytests to locate tests easier. Let me know if it is preferred to keep both python and c++ tests in one folder.

jacobmerson · 2026-08-14T02:10:17Z

@Fuad-HH do you know what's standard with other packages for the python tests? I.e., are they typically in /test, /test/python, etc.?

jacobmerson

I know we briefly discussed test coverage. Is there a convenient way for doing test coverage on python? Going through this PR, I'm seeing a bunch of stuff missing.

Note: that would be a separate PR, and unless it's trivial, not prioritized over other things.

Also, not to be fixed in this PR, but I realized I forgot to fix the python API to the "field"/"function" naming scheme we introduced in #336 , see #359

jacobmerson · 2026-08-14T02:12:13Z

+    happens exactly once per test run.
+    """
+    lib = omega_h.OmegaHLibrary()
+    yield lib


I'm not too familiar with pytest, so pardon the naive question. Is this some sort of python way of creating the library and keeping it in scope for the whole test suite?

I think using a fixture with scope="session" is one way to share the same library instance across all tests. No sure if it is the most standard way. If I understand correctly, it doesn't matter whether we use yield or return in the current code. However, yield seems like a better practice because it leaves room for cleanup or finalization in the future. Technically, we could perform some finalization after the yield, which may or may not help with the existing library lifecycle issue, although I haven't figured out the solution yet.

jacobmerson · 2026-08-14T02:12:55Z

+    Session-scoped; the world is obtained from the session-scoped library
+    and shared by all tests that need mesh-building capabilities.
+    """
+    return omega_h_lib.world()


Why does this session fixture return, but the lib one yields?

jacobmerson · 2026-08-14T02:14:41Z

@@ -0,0 +1,10 @@
+[pytest]


I'm not 100% sure, but should this be included in the pyproject.toml

I think that's correct.

Fuad-HH · 2026-08-14T03:35:37Z

@Fuad-HH do you know what's standard with other packages for the python tests? I.e., are they typically in /test, /test/python, etc.?

For most projects I have seen where python bindings are used, the python tests are collected in a specific directory rather than being in the test directory. I think test/python is better.

jacobmerson · 2026-08-14T04:14:06Z

I agree with Fuad about the location. It makes more sense to me to have them in test/python

Fuad-HH

I have added some small suggestions.

Fuad-HH · 2026-08-14T14:58:13Z

+python_files = "test_*.py"
+python_classes = "Test*"
+python_functions = "test_*"
+addopts = "-v --tb=short --strict-markers -p no:cacheprovider"


Should we hardcode these options here? If -v , --tb, and -p no:cacheprovider are needed for CI/CD, they can be easily added there in the pytest run. --strict-markers looks good to me.

In CI/CD, the following can be used:

pytest -v --tb=short -p no:cacheprovider

I think it depends on what default options we want for the pytest command. Not all of them are specifically meant for CI. I feel --strict-markers looks good in any case. -p no:cacheprovider is specific to CI, so we could remove it here. I don't have a preference on the other options. Do you find it easier to test without verbose and shorter traceback?

I mean we can leave it upto the user and they are free to use whichever option they want when running pytest.

Sounds good. Just removed that.

jacobmerson · 2026-08-14T20:43:20Z

@Sichao25 can you also add this to the self-hosted runner? I think it would be good if the python tests run in GPU configuration since Fuad already found some bugs in the python API related to GPUs (and we will need to add support for PyTorch device tensors for our LEADS collaboration).

Sichao25 · 2026-08-14T20:59:54Z

@jacobmerson Sure. That may also require the update of omega_h dependency with -DOmega_h_USE_pybind11=on.

jacobmerson · 2026-08-14T21:16:33Z

Rebuilding all the AMPERE80 deps.

jacobmerson · 2026-08-20T05:44:13Z

@Sichao25 pcms build linking python is working. I suspect you need to add some things to the LD_LIBRARY_PATH. I further suspect the following path may be needed: /users/mersoj2/laces-software/build/AMPERE80/omega_h/install/lib64/python./site-packages

Sichao25 · 2026-08-20T17:57:09Z

@jacobmerson Thanks, the issue is fixed by correcting the PYTHONPATH. But I'm curious why the path was python. instead of python3.10. That might indicate the code isn't catching the version number correctly.

jacobmerson · 2026-08-20T20:15:24Z

@Sichao25 do you think it's worth fixing the yield vs return difference before merging? I still don't fully understand what's preferred.

Sichao25 · 2026-08-20T20:31:52Z

We can use return for now. If we eventually need yield, we can add it later. My understanding is that if we eventually need to perform some cleanup for the library, adding it after yield is the recommended approach.

use pytest

efc70c0

Sichao25 temporarily deployed to asics August 14, 2026 01:49 — with GitHub Actions Inactive

jacobmerson reviewed Aug 14, 2026

View reviewed changes

remove omega_h tests

889c714

Sichao25 temporarily deployed to asics August 14, 2026 03:40 — with GitHub Actions Inactive

move pytests to test/python

e9f0f33

Sichao25 temporarily deployed to asics August 14, 2026 05:22 — with GitHub Actions Inactive

Fuad-HH reviewed Aug 14, 2026

View reviewed changes

require deep copy expected array comparison

33346fb

Sichao25 requested a deployment to asics August 14, 2026 19:36 — with GitHub Actions Waiting

remove pytest options to CI

3c0e7b1

Sichao25 temporarily deployed to asics August 14, 2026 20:42 — with GitHub Actions Inactive

add pytest to self hosted

640ed82

Sichao25 had a problem deploying to asics August 15, 2026 01:25 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 17, 2026 01:32 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 17, 2026 02:03 — with GitHub Actions Failure

Merge branch 'develop' into yus/ci

04512d0

Sichao25 had a problem deploying to asics August 17, 2026 16:27 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 17, 2026 18:27 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 17, 2026 18:54 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 17, 2026 19:13 — with GitHub Actions Failure

Sichao25 force-pushed the yus/ci branch from 3ad0546 to 04512d0 Compare August 17, 2026 21:33

Sichao25 had a problem deploying to asics August 17, 2026 21:33 — with GitHub Actions Failure

use built pybind11

1cdcc42

Sichao25 had a problem deploying to asics August 17, 2026 23:56 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 20, 2026 05:15 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 20, 2026 16:02 — with GitHub Actions Failure

Sichao25 had a problem deploying to asics August 20, 2026 16:44 — with GitHub Actions Failure

Sichao25 temporarily deployed to asics August 20, 2026 16:46 — with GitHub Actions Inactive

correct PYTHONPATH

7d2211f

Sichao25 force-pushed the yus/ci branch from 5fbb7d3 to 7d2211f Compare August 20, 2026 17:31

Sichao25 temporarily deployed to asics August 20, 2026 17:31 — with GitHub Actions Inactive

jacobmerson mentioned this pull request Aug 20, 2026

versioning for pythonpath on scorec systems #365

Open

use return instead of yield

868c072

Sichao25 deployed to asics August 20, 2026 20:34 — with GitHub Actions Active

jacobmerson approved these changes Aug 20, 2026

View reviewed changes

jacobmerson merged commit 0f2b31d into SCOREC:develop Aug 20, 2026
8 checks passed

Uh oh!

Conversation

Sichao25 commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacobmerson commented Aug 14, 2026

Uh oh!

jacobmerson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sichao25 Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fuad-HH commented Aug 14, 2026

Uh oh!

jacobmerson commented Aug 14, 2026

Uh oh!

Fuad-HH left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacobmerson commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sichao25 commented Aug 14, 2026

Uh oh!

jacobmerson commented Aug 14, 2026

Uh oh!

jacobmerson commented Aug 20, 2026

Uh oh!

Sichao25 commented Aug 20, 2026

Uh oh!

jacobmerson commented Aug 20, 2026

Uh oh!

Sichao25 commented Aug 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sichao25 commented Aug 14, 2026 •

edited

Loading

Sichao25 Aug 14, 2026 •

edited

Loading

jacobmerson commented Aug 14, 2026 •

edited

Loading