Fixing flaky tests #70

hannahbaumann · 2026-01-19T13:41:28Z

Fixes #43 and #67

This PR:

Closes netcdfs both in the code and in the test
Adds pooch caching back in
Updates the test; the new simulation_skipped dataset is from CDK2, which has two chains. Since the multichain fix is not in yet, this test currently has large RMSDs
Removes flaky argument in tests
Uses the skipped test dataset in more tests for not having to use the bigger one everywhere.

…e runners for the matrix

codecov · 2026-01-19T15:36:21Z

Codecov Report

❌ Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.94%. Comparing base (5260e3b) to head (67dbd04).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/openfe_analysis/rmsd.py	97.14%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
- Coverage   88.16%   87.94%   -0.23%     
==========================================
  Files           7        7              
  Lines         338      340       +2     
==========================================
+ Hits          298      299       +1     
- Misses         40       41       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

.github/workflows/ci.yaml

for more information, see https://pre-commit.ci

.github/workflows/ci.yaml

atravitz · 2026-01-29T22:22:36Z

@hannahbaumann you'll want to build pooch from main (like we are here in openfe) if you're seeing issues with fetching data.

src/openfe_analysis/tests/test_transformations.py

src/openfe_analysis/tests/conftest.py

IAlibay

Couple of things, still looking into that positions thing.

.github/workflows/ci.yaml

IAlibay · 2026-02-02T14:07:19Z

src/openfe_analysis/tests/conftest.py

-POOCH_CACHE = pooch.os_cache("openfe_analysis")
+ZENODO_DOI = "doi:10.5281/zenodo.18378051"
+
+ZENODO_FILES = {


We should probably have an "HAS_INTERNET" check on most of our tests.
Maybe todo in a separate issue?

Raised an issue!

IAlibay · 2026-02-02T14:23:01Z

src/openfe_analysis/tests/test_reader.py



 def test_universe_creation(simulation_nc, hybrid_system_pdb):
-    with pytest.warns(UserWarning, match="This is an older NetCDF file that"):


Do we still have a test somewhere to warn for the old netcdf file types?

If not, can we mock one?

The old one was the one with >500MB, I added a new test test_determine_position_indices_warns_for_old_nc that mocks this and gives a warning.

IAlibay · 2026-02-02T14:27:08Z

src/openfe_analysis/tests/test_reader.py

-    assert_allclose(u.dimensions, [82.191055, 82.191055, 82.191055, 90.0, 90.0, 90.0])
+    assert_allclose(u.dimensions, [78.141495, 78.141495, 78.141495, 60.0, 60.0, 90.0])

+    u.trajectory.close()


Are these actually necessary now? __del__ should call self.close (it's a reader thing), so they should get garbage collected when they go out of scope, if they don't then we might still be encountering an issue with closing the files.

I'm not seeing failures without this, I had put it since I wasn't sure if things could be unstable when lots of test run in parallel, but we could remove it for now and if we see problems later again, investigate further?

Since it's only in tests, I would vote for removing and seeing how we fare - it's a good canary.

Added it again, see below.

src/openfe_analysis/tests/test_transformations.py

Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>

.github/workflows/ci.yaml

…ailures

hannahbaumann · 2026-02-03T08:43:48Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

hannahbaumann · 2026-02-03T08:47:44Z

@IAlibay : After I removed the different u.trajectory.close(), the tests were flaky again with workers crashing, sometimes for macos, sometimes for ubuntu. I added the closing back in and now tests are stable again. I haven't seen the crashes locally.

…eEnergy/openfe_analysis into closing_netcdfs_properly

IAlibay · 2026-02-03T09:22:44Z

@IAlibay : After I removed the different u.trajectory.close(), the tests were flaky again with workers crashing, sometimes for macos, sometimes for ubuntu. I added the closing back in and now tests are stable again. I haven't seen the crashes locally.

That means things aren't being deleted properly.. I'm not sure we can fix it in this PR, can you open an issue and we can revisit later?

hannahbaumann · 2026-02-03T10:09:49Z

@IAlibay , I opened an issue here #84 !

hannahbaumann added 8 commits January 16, 2026 15:57

Move some tests to use skipped smaller data

b528ca2

Test out zenodo dealings

a477bc1

Try to improbe speed

ad84082

Try removing locking

8ba8087

Run downloads before the testing to have a single download for all th…

ead7951

…e runners for the matrix

add import pooch

f898a35

Test out more

c675a5c

Ensure datasets get closed

88e456d

hannahbaumann added 9 commits January 19, 2026 16:40

Move to per test download again

73a8e4d

Remove commented out lines

43aaca2

Test out adding an extra slash

c165525

Switch to all version doi

5f17770

Download url directly

c28286e

Small fix

197b6ba

Change url

b45390a

Add missing s

1d70936

Switch to api url

20084c3

IAlibay mentioned this pull request Jan 21, 2026

Datasets not being closed properly #71

Open

Update tests for new results

a9a8780

jameseastwood assigned hannahbaumann Jan 26, 2026

hannahbaumann added 8 commits January 26, 2026 17:30

Update conftest

c34c97c

Update to v2

0161673

Update tests

9b6ca69

Update rmsd test, currently large rmsd till rmsd fix comes in

1d5c849

Make last test pass

f4e88e2

Switch to zenodo fetch

bd0c8ee

remove lines

ba4c912

Update tests with large errors multichain failure

157c02f

hannahbaumann commented Jan 28, 2026

View reviewed changes

.github/workflows/ci.yaml Outdated Show resolved Hide resolved

hannahbaumann linked an issue Jan 28, 2026 that may be closed by this pull request

improve file clean up #67

Open

hannahbaumann mentioned this pull request Jan 28, 2026

Tests are flaky - unclosed NetCDF datasets? #43

Open

hannahbaumann linked an issue Jan 28, 2026 that may be closed by this pull request

Tests are flaky - unclosed NetCDF datasets? #43

Open

jameseastwood assigned atravitz and IAlibay and unassigned hannahbaumann Jan 28, 2026

[pre-commit.ci] auto fixes from pre-commit.com hooks

1b2c488

for more information, see https://pre-commit.ci

atravitz reviewed Jan 29, 2026

View reviewed changes

.github/workflows/ci.yaml Show resolved Hide resolved

atravitz reviewed Jan 29, 2026

View reviewed changes

src/openfe_analysis/tests/test_transformations.py Show resolved Hide resolved

src/openfe_analysis/tests/conftest.py Outdated Show resolved Hide resolved

Apply review suggestions

2a1d130

IAlibay requested changes Feb 2, 2026

View reviewed changes

Update .github/workflows/ci.yaml

209620a

Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>

IAlibay reviewed Feb 2, 2026

View reviewed changes

.github/workflows/ci.yaml Outdated Show resolved Hide resolved

hannahbaumann and others added 6 commits February 3, 2026 09:27

Revert ci.yaml change

c86faa7

Remove close on MDtraj universes

5c12459

Revert closing for test_transformations to see if that fixes sudden f…

8d02b78

…ailures

Small fix

d0c8d89

Add traj.close to see if that fixes flakyness again

a90ec9d

Merge branch 'main' into closing_netcdfs_properly

b487f21

[pre-commit.ci] auto fixes from pre-commit.com hooks

73d2cec

for more information, see https://pre-commit.ci

hannahbaumann added 2 commits February 3, 2026 09:58

Mock old netcdf file gives warning

2db8863

Merge branch 'closing_netcdfs_properly' of https://github.com/OpenFre…

67dbd04

…eEnergy/openfe_analysis into closing_netcdfs_properly

hannahbaumann requested review from IAlibay and atravitz February 3, 2026 10:10

atravitz mentioned this pull request Feb 3, 2026

[WIP] fix pooch data cacheing #61

Closed



		def test_universe_creation(simulation_nc, hybrid_system_pdb):
		with pytest.warns(UserWarning, match="This is an older NetCDF file that"):

Fixing flaky tests #70

Are you sure you want to change the base?

Fixing flaky tests #70

Uh oh!

Conversation

hannahbaumann commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

atravitz commented Jan 29, 2026

Uh oh!

Uh oh!

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hannahbaumann commented Feb 3, 2026

Uh oh!

hannahbaumann commented Feb 3, 2026

Uh oh!

IAlibay commented Feb 3, 2026

Uh oh!

hannahbaumann commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hannahbaumann commented Jan 19, 2026 •

edited

Loading

codecov bot commented Jan 19, 2026 •

edited

Loading