Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Run integration tests with pytest #763

Merged
merged 57 commits into from
Jun 25, 2024
Merged

Run integration tests with pytest #763

merged 57 commits into from
Jun 25, 2024

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Jun 4, 2024

Closes none.

Changes proposed in this pull request

  • Translate integration tests (currently in .circleci/*.sh into Python functions (i.e., pytests).
  • Separate out test data downloadingsteps from the test steps in the CircleCI config. This involves caching the test data.
  • Start retaining artifacts for all of the integration tests.
  • Check outputs (just the list of generated files) for all of the integration tests.
  • Start tracking coverage with the integration tests.

@tsalo tsalo added the refactor Changes to the codebase that don't impact workflow inputs or outputs. label Jun 4, 2024
@tsalo
Copy link
Member Author

tsalo commented Jun 5, 2024

For posterity: I was seeing "Can't combine line data with arc data". The solution appears to be to add --cov-config=pyproject.toml to pytest calls.

Full traceback:

Plugin: _cov, Hook: pytest_runtestloop
DataError: Can't combine line data with arc data
For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning
  config.hook.pytest_runtestloop(session=session)

INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/_pytest/main.py", line 285, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/_pytest/main.py", line 339, in _main
INTERNALERROR>     config.hook.pytest_runtestloop(session=session)
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/pluggy/_hooks.py", line 513, in __call__
INTERNALERROR>     return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/pluggy/_manager.py", line 120, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/pluggy/_callers.py", line 156, in _multicall
INTERNALERROR>     teardown[0].send(outcome)
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/pytest_cov/plugin.py", line 339, in pytest_runtestloop
INTERNALERROR>     self.cov_controller.finish()
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/pytest_cov/engine.py", line 46, in ensure_topdir_wrapper
INTERNALERROR>     return meth(self, *args, **kwargs)
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/pytest_cov/engine.py", line 260, in finish
INTERNALERROR>     self.cov.combine()
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/coverage/control.py", line 837, in combine
INTERNALERROR>     combine_parallel_data(
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/coverage/data.py", line 185, in combine_parallel_data
INTERNALERROR>     data.update(new_data, map_path=map_path)
INTERNALERROR>   File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/coverage/sqldata.py", line 668, in update
INTERNALERROR>     raise DataError("Can't combine line data with arc data")
INTERNALERROR> coverage.exceptions.DataError: Can't combine line data with arc data

@tsalo
Copy link
Member Author

tsalo commented Jun 10, 2024

Recon_PYAFQExternalTrk has one file (qsirecon-PYAFQ/sub-ABCD/dwi/sub-ABCD_acq-10per000_space-T1w_desc-preproc_dwimap/viz_core_bundles/sub-ABCD_acq-10per000_space-T1w_desc-preproc_dwi_space-RASMM_model-probCSD_algo-AFQ_desc-ARCLviz_dwi.html) that sometimes gets created and sometimes doesn't.

@tsalo tsalo marked this pull request as ready for review June 10, 2024 21:51
@tsalo tsalo requested a review from mattcieslak June 10, 2024 22:04
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Copy link
Collaborator

@mattcieslak mattcieslak left a comment

Choose a reason for hiding this comment

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

Only minor comments

.circleci/config.yml Show resolved Hide resolved
# Install qsiprep wheel
COPY --from=wheelstage /src/qsiprep/dist/*.whl .
RUN pip install --no-cache-dir $( ls *.whl )
# Install qsiprep
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't this add an extra copy of the source code into the container?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. I needed to be able to overwrite the source code when mounting the local version, but the old approach didn't seem to have a copy in the Docker image.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't the overwritten code live in the site-packages directory?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's been a little while since I made that change, so my memory is a little fuzzy, but I believe I tried the regular mount point (/usr/local/miniconda/lib/python3.10/site-packages/qsiprep) and it didn't work with the old way. Def worth revisiting in case I made a mistake there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh wait, I think I remember what the issue was! I didn't know how to install with the test dependencies using the old way.

qsiprep/cli/workflow.py Show resolved Hide resolved
@mattcieslak mattcieslak merged commit 87f0206 into PennLINC:master Jun 25, 2024
32 checks passed
@tsalo tsalo deleted the pytest branch June 25, 2024 17:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
refactor Changes to the codebase that don't impact workflow inputs or outputs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants