Pytest integrated by tomc271 · Pull Request #3331 · boutproject/BOUT-dev

tomc271 · 2026-03-17T10:43:55Z

No description provided.

…ted_or_mms_test

matching either to a file named `test_foo.py`.

Native configuration is x86_64-pc-linux-gnu === tests === Schedule of variations: unix Running target unix Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target. Using /usr/share/dejagnu/config/unix.exp as generic interface file for target. === Summary === to in CMakeLists.txt

…ctest

…renaming

…ipts

so that the `input` directory is found.

so that the `test` directory is found.

so that the `data` directory is found.

Fails due to missing file `grid.fci.nc`.

Change directory to exe location prior to attempting to run it.

dschwoerer

It seems one of the MMS tests is missing. That certainly needs fixing.

I find it also increadibly hard to review, as in some case the rename from runtest to test_* is not rendered correctly on github. In some cases this seems to be done such that git itself is not aware of the the rename, e.g. test-fci-mpi/runtest.

This makes it even harder to figure out what was changed, as you clearly did not write the whole test new.

I have reviewed test-fci-mpi/runtest more carefully, and it is doing only part what is in next. It is skipping the iteration over the different test cases:
https://github.com/boutproject/BOUT-dev/blob/next/tests/integrated/test-fci-mpi/runtest#L49

Also, it would only output the errors after the assert:

BOUT-dev/tests/integrated/test-fci-mpi/test_fci_mpi.py

Line 72 in 7f920ac

assert success, "\nSome tests failed:"

So this is never printed.

dschwoerer · 2026-06-15T09:31:51Z

+# Requires: netcdf
+# Requires: not metric_3d
+# Cores: 4
+


I doubt the current test system still parses and process them, so the can be removed?

dschwoerer · 2026-06-15T09:58:02Z

I forgot - the new pytest is much faster, which is nice, but I am wondering this comes from skipping tests.

(accidentally re-added when merging in `next`).

…grid_laplace to prevent spurious test failures due to unused options.

…ntegrated

ZedThree

There's a few things that haven't been ported exactly right, and there's still quite a few patterns that I would like to update, but they could be done in a separate PR so we can get the bulk of the changes in sooner. Maybe just leave all the "could be..." for now, and concentrate on fixing the other comments

I noticed some particular patterns that we could improve upon in a second PR:

Could have a helper function to simplify this pattern:

s, out = launch_safe(...)
with open("run.log", "w") as f:
   f.write(out)

We have quite a few tests that do something vaguely like this:

# some setup
launch(cmd, ...)

success = True

for v in vars:
   r = collect(v)
   success &= r < tol

assert success

where we run an executable, and want to check all the variables before failing, rather than just failing on the first bad one.
This pattern is maybe best transformed into a fixture, and then the test is parameterised over the variables:

@fixture(scope="module")
def fixture():
  # some setup
  launch(cmd, ...)

@parametrize(variables)
def test(variables):
   assert_allclose(r, expected)

then we only actually run the code once, but can parallelise over the variables. I think there's a way to parameterise fixtures too, so this could also run once e.g. per nproc.

ZedThree · 2026-06-24T10:47:47Z

+          # Install the core requirements into the same venv
+          uv pip install -r requirements.txt


Orthogonal PR -> this file should be merged into pyprojects.toml

ZedThree · 2026-06-24T10:49:56Z

+    if fe.getLocation() == fc.getLocation():
+        print("The loaded field should not be at CELL_CENTRE")
+        fail = 1


This could be

Suggested change

if fe.getLocation() == fc.getLocation():

print("The loaded field should not be at CELL_CENTRE")

fail = 1

assert fe.getLocation() != fc.getLocation(), "The loaded field should not be at CELL_CENTRE"

ZedThree · 2026-06-24T10:51:02Z

+    if compare(fc, f) > 1e-3:
+        print("Something is wrong. Maybe interpolation is broken")
+        fail = 1


These could also be

Suggested change

if compare(fc, f) > 1e-3:

print("Something is wrong. Maybe interpolation is broken")

fail = 1

assert compare(fc, f) <= 1e-3, "Maybe interpolation is broken"

ZedThree · 2026-06-24T12:11:12Z

+# Requires: netcdf
+# Cores: 4


We can delete all of these # Requires and # Cores comments now too

ZedThree · 2026-06-24T14:12:14Z

+    for data_dir, stop_file in zip(data_dirs, stop_files):
+        for check, expected in zip(check_values, expected_steps):


Could be parameterised

ZedThree · 2026-06-24T14:14:10Z

Not sure this has been converted?

ZedThree · 2026-06-24T14:14:57Z

+pytest = [
+     "pytest~=8.4",
+     "pytest-xdist~=3.8",
+]


Can we add the other test dependencies here too? numpy, scipy, zoidberg, etc? And perhaps rename:

Suggested change

pytest = [

"pytest~=8.4",

"pytest-xdist~=3.8",

]

test = [

"pytest~=8.4",

"pytest-xdist~=3.8",

]

ZedThree · 2026-06-24T14:19:30Z

+    return False
+
+
+@pytest.mark.parametrize("shift_type, variable", list(zip(shift_types, variables)))


I think this is wrong? This runs the following cases:

"shifted" + ("ddy", "ddy_check")

"shiftedinterp" + ("ddy2", "ddy_check")

but we want to run once for each of the shift_types, and check all of variables for each run

dschwoerer · 2026-06-26T08:54:09Z

I think this should be merged quickly, as it it seems like a great improvement, as it makes the tests faster.

I am however worried that there are regressions. @ZedThree did you compare test by test, to make sure this (still) matches what is in next? It is a bit difficult to get a feeling of the state of this, with so little comments from @tomc271

I would be willing to review at least part of this, but only if you think this is close to being merged, so the review does not get outdated.

ZedThree · 2026-06-26T10:10:40Z

@dschwoerer Yeah, I went through one by one, comparing the runtest to the test_*.py. I think I only caught one actual regression (test_yupdown). It's probably wise to have a second set of eyes check things too.

@tomc271 Can you comment when you've fixed test_yupdown, and we'll merge then. Everything else can wait for a second PR

ZedThree reviewed Mar 18, 2026

View reviewed changes

tomchapman added 29 commits April 14, 2026 12:12

Use pytest rather than runtest as the command set in bout_add_integra…

a102ffe

…ted_or_mms_test

Rename runtest file to test-foo.py for each integrated test

a55dda0

Rename runtest file to test-foo.py for each MMS test

053f62a

Convert runtest file to use pytest for test_drift_instability

db34143

Handle test files for tests named either test-foo or foo

0ed0750

matching either to a file named `test_foo.py`.

Handle test files with hyphens or prefixed with

fdd93e0

Update symlink to renamed runtest file

f0b4405

Rename more runtest files

4dd41cc

Rename more runtest files

f9e2374

Use PATH-based pytest executable

d00629c

Fix more renamings

393ac3e

Change custom_target check-integrated-tests to use pytest instead of …

4deaf22

…ctest

Some runtest files are shell scripts not python scripts, so revert …

82ff7ca

…renaming

Add python tests to run those runtest files that are not python scr…

681c2fb

…ipts

Convert test_cyclic.py to a valid pytest file

01cc49c

Convert several more test files to work with pytest

09114fe

Change directory before calling boutpp.init()

be04639

so that the `input` directory is found.

Change directory before calling boutpp.init()

abe75f2

so that the `test` directory is found.

Change directory before calling boutpp.Field3D.fromCollect()

52436a8

so that the `data` directory is found.

Modify more test files for running via pytest

f7378bc

MPI oversubscribe for communications test

5a959a2

Change directory before removing files in data directory

e977ed6

Modify test_communications to run via pytest

9afb0b2

Modify test_delp2 to run via pytest

18b9c90

Modify test_fci_boundary to run via pytest

c55ab8f

Fails due to missing file `grid.fci.nc`.

Modify test_fieldgroupcomm to run via pytest

125dd80

Change directory to exe location prior to attempting to run it.

Modify test_gyro to run via pytest

ede926e

Change directory to exe location prior to attempting to run it.

Modify test_interpolate to run via pytest

cc47c24

Change directory to exe location prior to attempting to run it.

tomchapman added 5 commits June 11, 2026 13:44

Refactor test_laplacexy_fv for pytest

c193065

Refactor test_laplace for pytest

81f6d27

Refactor test_petsc_laplace for pytest

b43a1c4

Refactor test_laplacexy_short for pytest

a3d299d

Refactor test_laplacexy2_hypre for pytest

7f920ac

tomc271 force-pushed the pytest-integrated branch from 3e91b3a to 7f920ac Compare June 11, 2026 12:48

dschwoerer requested changes Jun 15, 2026

View reviewed changes

tomchapman added 19 commits June 16, 2026 09:17

Fix merge

2f0c1f5

Remove old runtest file

dc38981

(accidentally re-added when merging in `next`).

Integrate splitting of test-multigrid_laplace into two tests

a9e2006

Explicitly specify the required inp files

5704f8b

Set error_on_unused_options option to false when running test_multi…

7123424

…grid_laplace to prevent spurious test failures due to unused options.

Use EXTRA_FILES multi-value argument for the extra input files

1996d27

Apply cmake-format formatting

0642f30

Merge remote-tracking branch 'remotes/boutproject/next' into pytest-i…

4c7e22a

…ntegrated

Refactor test_datafile_facade for pytest

8deac5b

Refactor test_initial for pytest

4e7cb9d

Refactor test_multigrid_laplace for pytest

2c384ef

Refactor test_fieldgroupComm for pytest

70e341d

Refactor test_griddata_yboundary_guards for pytest

9577bbd

Refactor test_laplacexy for pytest

883083a

Refactor test_multigrid_laplace_unsheared for pytest

ec7fdd6

Refactor test_naulin_laplace for pytest

66a2c5c

Refactor test_petsc_laplace_MAST_grid for pytest

c65424b

Apply ruff formatting

1f4c9e1

Refactor test_twistshift for pytest

d0072e8

ZedThree requested changes Jun 24, 2026

View reviewed changes

		# Install the core requirements into the same venv
		uv pip install -r requirements.txt

		for data_dir, stop_file in zip(data_dirs, stop_files):
		for check, expected in zip(check_values, expected_steps):

		return False


		@pytest.mark.parametrize("shift_type, variable", list(zip(shift_types, variables)))

Uh oh!

Conversation

tomc271 commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dschwoerer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschwoerer commented Jun 15, 2026

Uh oh!

ZedThree 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!

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!

Choose a reason for hiding this comment

Uh oh!

dschwoerer commented Jun 26, 2026

Uh oh!

ZedThree commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants