Skip to content

Conversation

@jandom
Copy link
Collaborator

@jandom jandom commented Jan 19, 2026

Summary

#90 introduced pytest-cov and pytest-xdist but these were placed under the 'dev' instead of the 'test' dependency group.

Changes

  • Move the deps to the correct group
  • Add pytest-benchmark, because we'll need that later anyway for perf measurements

Related Issues

Testing

Other Notes

@jandom jandom requested a review from jnwei January 19, 2026 14:12
@jandom jandom self-assigned this Jan 19, 2026
@jandom jandom added the safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. label Jan 19, 2026
@jandom
Copy link
Collaborator Author

jandom commented Jan 19, 2026

Before the fix locally

docker run --rm -it docker.io/library/openfold-docker:test

==========
== CUDA ==
==========

CUDA Version 12.1.1

Container image Copyright (c) 2016-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.

This container image and its contents are governed by the NVIDIA Deep Learning Container License.
By pulling and using the container, you accept the terms and conditions of this license:
https://developer.nvidia.com/ngc/nvidia-deep-learning-container-license

A copy of this license is made available in this container at /NGC-DL-CONTAINER-LICENSE for your convenience.

WARNING: The NVIDIA Driver was not detected.  GPU functionality will not be available.
   Use the NVIDIA Container Toolkit to start this container with GPU support; see
   https://docs.nvidia.com/datacenter/cloud-native/ .

(openfold3) root@9d0451d61f47:/opt/openfold3# pytest openfold3/tests -vvv -n auto --cov=openfold3 --cov-report=xml
[2026-01-19 14:17:36,197] [WARNING] [real_accelerator.py:209:get_accelerator] Setting accelerator to CPU. If you have GPU or other accelerator, we were unable to detect it.
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: -n --cov=openfold3 --cov-report=xml
  inifile: /opt/openfold3/pyproject.toml
  rootdir: /opt/openfold3

After the fix locally

docker run --rm -it docker.io/library/openfold-docker:test

==========
== CUDA ==
==========

CUDA Version 12.1.1

Container image Copyright (c) 2016-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.

This container image and its contents are governed by the NVIDIA Deep Learning Container License.
By pulling and using the container, you accept the terms and conditions of this license:
https://developer.nvidia.com/ngc/nvidia-deep-learning-container-license

A copy of this license is made available in this container at /NGC-DL-CONTAINER-LICENSE for your convenience.

WARNING: The NVIDIA Driver was not detected.  GPU functionality will not be available.
   Use the NVIDIA Container Toolkit to start this container with GPU support; see
   https://docs.nvidia.com/datacenter/cloud-native/ .

(openfold3) root@ec7e9a71c0b6:/opt/openfold3# pytest openfold3/tests -vvv -n auto --cov=openfold3 --cov-report=xml
/opt/conda/envs/openfold3/lib/python3.12/site-packages/pytest_benchmark/logger.py:44: PytestBenchmarkWarning: Benchmarks are automatically disabled because xdist plugin is active.Benchmarks cannot be performed reliably in a parallelized environment.
  warner(PytestBenchmarkWarning(text))
==================================================== test session starts ====================================================
platform linux -- Python 3.12.12, pytest-9.0.2, pluggy-1.6.0 -- /opt/conda/envs/openfold3/bin/python3.12
cachedir: .pytest_cache
benchmark: 5.2.3 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /opt/openfold3
configfile: pyproject.toml
plugins: benchmark-5.2.3, xdist-3.8.0, cov-7.0.0
initialized: 4/4 workers

@jandom jandom added safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. and removed safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. labels Jan 19, 2026
@jandom jandom added safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. and removed safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. labels Jan 19, 2026
Copy link
Contributor

@jnwei jnwei left a comment

Choose a reason for hiding this comment

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

Overall looks good! Thanks for the quick fix

I went to check the action log associated with the most recent commit. It look likes no code coverage artifact was generated for this run.

Could you double check the path of the coverage report and see that it matches with the argument provided to the upload action?

@jandom jandom added safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. and removed safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. labels Jan 20, 2026
@jandom
Copy link
Collaborator Author

jandom commented Jan 20, 2026

Agreed, that is a bit fishy – also the term coverage report didn't print anything (odd)

@jandom jandom requested a review from jnwei January 20, 2026 13:02
@jandom
Copy link
Collaborator Author

jandom commented Jan 21, 2026

@jnwei this looks good – the manually triggered GH action has the correct artifact

https://github.com/aqlaboratory/openfold-3/actions/runs/21168437686#artifacts

Copy link
Contributor

@jnwei jnwei left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for adding this

@jandom jandom merged commit 8b3a25d into main Jan 21, 2026
8 checks passed
@jandom jandom deleted the jandom/2026-01/fix/pytest-cov-pytest-xdist branch January 21, 2026 08:41
@jandom
Copy link
Collaborator Author

jandom commented Jan 21, 2026

Very happy to – @sdvillal slight duplication with your PR because we added 'pytest-benchmark' but I don't think this will cause a huge amount of pain (you've probably dealt with most conflicts like this already)

@sdvillal
Copy link

I already merged this into the branch, and no worries about conflicts - we do not know when will we merge the big PR and we cannot stop the world until then :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants