Start doing integration testing

[ebrahimebrahim]

We should generally start including integration testing in SlicerOpenLIFU.

Since we have zero of these at the moment, adding the first one is going to be the hardest part. So this issue represents just adding a single integration test of just anything, as long as its some multi-module workflow, to establish a precedent for future tests.

Some questions to be answered:

• Where will the test live? If it covers multiple modules, then is it logical for it to live in OpenLIFUHome for example?
• What kind of example data will the test use? How will it get data if not from db_dvc?
• What is the best way to run the test? Is this something you should be able to run headless, or would that prevent it from messing around with widgets? What if there are many tests -- is there a convenient way to run all of them?

[ebrahimebrahim]

I just remembered that I've had some snippets that I've used in the past to more conveniently test specific things I was working on or reviewing. They may be a helpful reference

• Update SlicerOpenLIFUTransducer  #159 (review)
• Save virtual fit results in scene and sessions p#179 #179 (comment) (expand the "Details")

[arhowe00]

Before I set us in a direction regarding our use of test data in SlicerOpenLIFU, we should weigh the options. Is there a consensus desire to separate testing data for openlifu and SlicerOpenLIFU? This works, but it may lead to hiccups such as version mismatches and forgetting to to double update data if we are not careful.

The current state of our data is that we have 'db_dvc' for human use, and openlifu uses git-controlled data in tests/, which is notably small and works seamlessly with pytest.

To adopt an automated workflow for testing SlicerOpenLIFU, we will need version controlled data, and tracking this in git may not work for us (e.g. if we use a large MRI). However an important question is whether we need our test data to scale. Does the size of the data impact the testing of the extension? If the answer is "no," it may save time and effort to track the data in git. It may be extra wise to reference the same data in tests/ tracked by openlifu. This would technically be possible, since both SlicerOpenLIFU and openlifu use the same data schema, though some features are only intended for use in the extension.

If we must separate testing data, the consistency of testing data with the openlifu guaranteed by the openlifu hash will no longer apply. When updating the openlifu version, the SlicerOpenLIFU data will need to be updated (whether it is git-tracked or stored somewhere scalable). Now, we explore using scalable testing data stores:

Using DVC with SlicerOpenLIFU

Similar to 'db_dvc', we would have version controlled test data for SlicerOpenLIFU. In OpenLIFULib, we would introduce utility functions that use the dvc API to collect this test data, with an API key supplied in an environment variable. When the individual modules are being tested, or just the OpenLIFUHome module (which calls the other tests), these utilities would be called. We could either hardcode a hash corresponding to specific versions of the dvc data, or we could make sure that when updating openlifu and the corresponding dvc test data, that those data updates don't break anything about SlicerOpenLIFU. Therefore, updating the version of openlifu, which would come with a corresponding SlicerOpenLIFU test data update, might imply having to update SlicerOpenLIFU before doing a version bump. This last item would have prevented a few bugs we have seen so far.

Potential workflow improvement: using DVC with pytest

If we decide to version control the testing data for the extension, one way we could improve our workflow is by referencing the same data in pytest for openlifu. Instead of using tests/, we would reference a unified data store whose test data is updated with openlifu updates and contains a comprehensive database. We can still include simple data for most pytest use cases, but we can also include larger data objects.

This would change how our tests are run in CI/CD pipelines. We could use a hook like the following:

- name: Pull data with DVC
  run: dvc pull
- name: Run tests
  run: pytest tests/

which would be efficient in most cases because the data are cached. Then, for CI/CD, we could explore using GitHub Actions Secrets if we don't want the testing data to be public. However, assuming we use a read-only public DVC remote, we could have something like:

steps:
  - name: Set up DVC
    run: pip install 'dvc[gdrive]'  # or another remote

  - name: Pull data from DVC
    run: dvc pull

to fetch the data in the CI. We could then use similar tricks for automating data fetching in SlicerOpenLIFU, but let's consider that depending on what we choose.

---

Deciding on how we reference data depends on how much we care about data volume scalability. We could avoid using dvc altogether for everything testing related, eliminating the need for anything above, only if it is sufficient to test the extension. As stated, the simplest solution would be to update and reference the same git-tracked tests/ in openlifu.

[ebrahimebrahim]

Thank you for putting together all these thoughts!

However an important question is whether we need our test data to scale.

Yes, it needs to scale. I expect that sticking with committing testing data directly into the repo would quickly become too limiting.

I think if I had infinite time en my favorite option is to create a third, common, publicly readable openlifu database tracked by DVC in the OpenLIFU-python repo, with your suggested updates to the OpenLIFU-python CI to pull and test against that data. But this is actually a lot of work I think, and makes things more complicated in OpenLIFU-python. That's undesirable because we want to keep OpenLIFU-python accessible as much as possible so that Openwater can reasonably maintain and develop on it.

I wonder -- can we use the existing db_dvc/ in OpenLIFU-python to serve the role of a testing dataset for SlicerOpenLIFU?
Previously we did not like this idea because example data and testing data are two different things.
But could we just stick some testing data into the same db_dvc/ and then only check out that data when we pull dvc data for testing purposes?

If we go this route, then here are the tasks I see ahead:

• Confirm with Openwater that the data in db_dvc/ can be made public for testing purposes.
• In OpenLIFU-python:
  • Add to the CI the things you are suggesting, using GitHub Actions Secrets, but find a DVC command that only checks out the data (i.e. subjects and sessions) that is actually used in tests
  • Add a pytest that for now just loads a session, does a quick thing with it, checks that the thing worked, and saves the session.
• In SlicerOpenLIFU:
  • We don't have CI and no need to add it yet. Just add a script that (1) checks out only the DVC sessions that are used for testing, referencing the OpenLIFU-python commit hash in the python-requirements.txt to identify the version of DVC data to grab and (2) runs the tests that are in the OpenLIFUHome module.
  • Add an example test or two to the OpenLIFUHome module, preferably one that involves a multi-module workflow where the code that tests the a functionality that is specific to a given module is local to that module.

Let's discuss more this afternoon and see if we agree.

[arhowe00]

I wonder -- can we use the existing db_dvc/ in OpenLIFU-python to serve the role of a testing dataset for SlicerOpenLIFU?
Previously we did not like this idea because example data and testing data are two different things.
But could we just stick some testing data into the same db_dvc/ and then only check out that data when we pull dvc data for testing purposes?

Yes, we could do that, but I think it makes more sense to have a separate dvc location just for the testing data. So the db_dvc/ example data, and then we can make tests/ dvc-tracked and remove the content tracking from git, only keeping the content address. If the testing data is to scale, there will likely be an order of magnitude difference between the testing data and the example data. We could even store the tests/ data in a different place or with a provider of Openwater's choice. Then, new testing data would only be tracked in tests/, which would be used to test both openlifu and the extension.

If we are worried about Openwater not being able to maintain and use tests/, but we also need testing data to scale, we are at a standstill; either we defy that project requirement or add versioning to testing data explain how it works. Since dvc conveniently wraps the data retrieval in an API call and (I think) minimally accomplishes scalable data versioning, it may not be too complicated to maintain.

[ebrahimebrahim]

I would say to separate it from the unit tests which live in tests/ -- those can just be github tracked.

The test database can be another dvc database like db_dvc_testing/ or something like that
specifically for integration tests that are shared between openlifu and SlicerOpenLIFU.
(But tracked using the same DVC remote, it would just be another folder)

[arhowe00]

I would say to separate it from the unit tests which live in tests/ -- those can just be github tracked.

Won't some of the unit tests use the scaled testing data? I think we should put all the testing data in one place or this duplication would make things more difficult to maintain (for both us and Openwater). Can we come up with a solid list of data formats (e.g. MRI) we need to scale and approximately how large that data will be, as well as a rule of thumb for which data or at which size threshold should things be stored separately, if we decide to store things in two locations?

The test database can be another dvc database like db_dvc_testing/ or something like that specifically for integration tests that are shared between openlifu and SlicerOpenLIFU. (But tracked using the same DVC remote, it would just be another folder)

When you say the same remote -- yes -- we can still use gdrive and the same location, but if we do go with this option, I don't think we should add db_dvc_testing/to db_dvc/ (I know this may not be what you are suggesting). We currently only track a single content hash, for db_dvc/, which is quite large, and it would be important to separately track changes in content address between testing data and example data.

[ebrahimebrahim]

Unit tests and integration tests are doing different things, so there is a case for separating the test data here. It wouldn't be duplication, but quite different data that makes sense to use for integration tests.

Let's discuss more tomorrow to hash out some of the details, as well as whether we want to pursue this aggressively yet or not during this milestone.

[ebrahimebrahim]

Result of our discussion: In order to get this kicked off we will vastly simplify it. No shared common test data between SlicerOpenLIFU and OpenLIFU-python. Now it's just the following:

• Start a DVC-tracked testing dataset that lives in the SlicerOpenLIFU repo. It can start as the db_dvc from OpenLIFU-python with only the example_subject and example_protocol. We need a full transducer example for integration testing, so example_transducer isn't good enough. We need to ask openwater if they are cool with making the openlifu_2x400 transducer public.
• Add dvc to the python requirements of SlicerOpenLIFU, so that you can stick a data checkout step inside the tests. Figure out where is the appropriate place to stick that download step.
• Add an example test or two to the OpenLIFUHome module, preferably one that involves a multi-module workflow where the code that tests the a functionality that is specific to a given module is local to that module.