Repository: innolitics/pacsman (language: python)

Issue:
Start Using Orthanc in GitHub Actions
Currently this repo uses `dicomserver.co.uk` for automated remote-based tests. Unfortunately, it looks like things have changed significantly with what query retrieve models or attributes they accept and without a conformance statement, it is very difficult to track down the right set of things to change to get these to work.

> The `integration_test.py::test_remote_patient_search` test is failing with no changes to the codebase

Even if we could fix this test to work with the existing remote site, it seems like a good strategy to switch to using a GitHub actions hosted Orthanc instance to avoid any future flakiness.

I'll be working on this shortly, to unblock a future PR.
Start Using Orthanc in GitHub Actions
Currently this repo uses `dicomserver.co.uk` for automated remote-based tests. Unfortunately, it looks like things have changed significantly with what query retrieve models or attributes they accept and without a conformance statement, it is very difficult to track down the right set of things to change to get these to work.

> The `integration_test.py::test_remote_patient_search` test is failing with no changes to the codebase

Even if we could fix this test to work with the existing remote site, it seems like a good strategy to switch to using a GitHub actions hosted Orthanc instance to avoid any future flakiness.

I'll be working on this shortly, to unblock a future PR.


Discussion / hints:

Fix patch:
diff --git a/README.md b/README.md
index 0a8ca26..468ce2c 100644
--- a/README.md
+++ b/README.md
@@ -17,3 +17,27 @@ files.
 In addition to the supplied backends, you can write your own backend implementing the
 `BaseDicomClient`. This can be a useful interface layer for non-PACS systems such as a
 cloud storage system.
+
+## Development
+Linting is done with `flake8` and testing with `pytest`.
+
+GitHub actions has automatic checks for both linting and tests, using [`tox`](https://tox.wiki/en/latest/) as the runner (see [`./tox.ini`](tox.ini)). To replicate this locally, install `tox`, then run `tox .` in the root of the project.
+
+> If you get an error about "InterpreterNotFound", make sure you have that version of Python installed and in the path (e.g., discoverable with `which python{version}`). Or use `--skip-missing-interpreters` to skip those.
+
+
+### Remote DICOM Testing - Using Orthanc
+Tests marked *remote* rely on a live DICOM server. For GitHub actions, an instance of Orthanc will be used, and you can re-use this service locally as well.
+
+```bash
+docker-compose up -d orthanc
+
+# If this is the first time, and test data files have not yet been loaded
+python3 pacsman/upload_test.py remote
+
+# If you have DCMTK installed, here is a quick test
+echoscu localhost 4242
+
+# Or, with pynetdicom
+python -m pynetdicom echoscu localhost 4242
+```
diff --git a/docker-compose.yml b/docker-compose.yml
new file mode 100644
index 0000000..2f94faa
--- /dev/null
+++ b/docker-compose.yml
@@ -0,0 +1,29 @@
+# For use with GitHub Actions, or for local testing
+# https://book.orthanc-server.com/users/docker-osimis.html
+# Legacy docs: https://osimis.atlassian.net/wiki/spaces/OKB/pages/26738689/How+to+use+osimis+orthanc+Docker+images (some of these settings are still used)
+version: '3'
+services:
+  orthanc:
+    image: osimis/orthanc:22.6.2
+    restart: unless-stopped
+    ports:
+      # For local testing, if you want to poke around the web interface
+      - "8042:8042"
+      # DICOM port
+      # If we ever dockerize the entire repo, then host-binding like this would not be necessary, and we could move to
+      # using `orthanc:4242`
+      - "4242:4242"
+    environment:
+      # This is the default value, just documenting
+      DICOM_AET: "ORTHANC"
+      # Syntax is [AET, Address, Port, optional_Vendor_Patch]
+      DICOM_MODALITIES: |
+        {
+          "testing": [ "TEST", "localhost", 4242 ]
+        }
+      AC_AUTHENTICATION_ENABLED: "false"
+      VERBOSE_ENABLED: "true"
+    volumes:
+      - "orthanc-storage-volume:/var/lib/orthanc/db"
+volumes:
+  orthanc-storage-volume:


Test patch:
diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml
index c0fa76f..b174e6e 100644
--- a/.github/workflows/unit-tests.yml
+++ b/.github/workflows/unit-tests.yml
@@ -15,8 +15,28 @@ jobs:
         uses: actions/setup-python@v2
         with:
           python-version: ${{ matrix.python }}
+      - name: Start Orthanc
+        run: docker-compose -f docker-compose.yml up -d orthanc
       - name: Install Tox and build dependencies
-        run: pip install tox
+        run: |
+          pip install .
+          pip install tox
+      - name: Wait for Orthanc
+        run: |
+          ORTHANC_PORT=4242
+          MAX_ITERATIONS=60
+          ITERATIONS=0
+          until python -m pynetdicom echoscu localhost $ORTHANC_PORT > /dev/null 2>&1 || (($ITERATIONS >= $MAX_ITERATIONS)); do
+              >&2 echo "Waiting on Orthanc to be ready..."
+              ITERATIONS=$(( ITERATIONS + 1 ))
+              echo $ITERATIONS
+              sleep 1
+          done
+
+          # At this point, orthanc should be up - if not, halt
+          python -m pynetdicom echoscu localhost $ORTHANC_PORT
+      - name: Import Test DICOM Files
+        run: python3 pacsman/upload_test.py remote
       - name: Run Tox
         # Run tox using the version of Python in `PATH`
         run: tox . --skip-missing-interpreters
diff --git a/pacsman/integration_test.py b/pacsman/integration_test.py
index e8c3502..a1441ee 100644
--- a/pacsman/integration_test.py
+++ b/pacsman/integration_test.py
@@ -1,5 +1,5 @@
 '''
-Remote tests depend on data from www.dicomserver.co.uk. The data there could change.
+Remote tests depend on a running Orthanc instance (emulating a remote server).
 No data is fetched from the remote server because C-GET is not yet supported in pacsman.
 
 Local tests depend on data in the `test_dicom_dir` directory, which is imported into a local
@@ -14,9 +14,6 @@
 
 An alternative to step 2 above is to run upload_test.py
 
-To explore or debug the remote data interactively, add www.dicomserver.co.uk:11112 as a
-location in Horos with any AETitle.
-
 If horos is running on a different machine set the LOCAL_PACS_URL environment variable to
 the ip of the machine running horos and similarly replace localhost in step
 3 of the instructions above.
@@ -33,7 +30,7 @@
 
 
 def initialize_pynetdicom_client(client_ae, pacs_url, pacs_port, dicom_dir):
-    return PynetDicomClient(client_ae=client_ae, remote_ae='TEST', pacs_url=pacs_url, pacs_port=pacs_port,
+    return PynetDicomClient(client_ae=client_ae, remote_ae='ORTHANC', pacs_url=pacs_url, pacs_port=pacs_port,
                             dicom_dir=dicom_dir)
 
 
@@ -70,8 +67,8 @@ def remote_client(request):
     logger.setLevel(logging.DEBUG)
     pynetdicom_logger = logging.getLogger('pynetdicom3')
     pynetdicom_logger.setLevel(logging.DEBUG)
-    return request.param(client_ae='TEST', pacs_url='www.dicomserver.co.uk',
-                         pacs_port=11112, dicom_dir='.')
+    return request.param(client_ae='TEST', pacs_url='localhost',
+                         pacs_port=4242, dicom_dir='.')
 
 
 @pytest.mark.integration
diff --git a/pacsman/upload_test.py b/pacsman/upload_test.py
index fdb5d3e..1e1f078 100644
--- a/pacsman/upload_test.py
+++ b/pacsman/upload_test.py
@@ -1,24 +1,32 @@
 import os
+import argparse
 
-from pacsman import DcmtkDicomClient, PynetDicomClient
+from pacsman import PynetDicomClient
 from pacsman import dicom_file_iterator
 
 TEST_DATA_DIRECTORY = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'test_dicom_data')
 LOCAL_PACS_URL = os.environ.get('LOCAL_PACS_URL', 'localhost')
 
 
-def main():
+def main(destination='both'):
     print(f'uploading test data from {TEST_DATA_DIRECTORY}')
 
-    # "TEST" must be registered AE in the Horos listener
-    client = DcmtkDicomClient(client_ae='TEST', pacs_url=LOCAL_PACS_URL, pacs_port=11112,
-                              dicom_dir='.', remote_ae='HOROS-LOCAL')
-    client.send_datasets(dicom_file_iterator(TEST_DATA_DIRECTORY))
+    if destination == 'both' or destination == 'remote':
+        print('Uploading files to remote destination')
+        client = PynetDicomClient(client_ae='TEST', pacs_url=LOCAL_PACS_URL, pacs_port=4242,
+                                  dicom_dir='.', remote_ae='ORTHANC')
+        client.send_datasets(dicom_file_iterator(TEST_DATA_DIRECTORY))
 
-    client = PynetDicomClient(client_ae='TEST', pacs_url=LOCAL_PACS_URL, pacs_port=11112,
-                              dicom_dir='.', remote_ae='HOROS-LOCAL')
-    client.send_datasets(dicom_file_iterator(TEST_DATA_DIRECTORY))
+    if destination == 'both' or destination == 'local':
+        print('Uploading files to local destination')
+        # "TEST" must be registered AE in the Horos listener
+        client = PynetDicomClient(client_ae='TEST', pacs_url=LOCAL_PACS_URL, pacs_port=11112,
+                                  dicom_dir='.', remote_ae='HOROS-LOCAL')
+        client.send_datasets(dicom_file_iterator(TEST_DATA_DIRECTORY))
 
 
 if __name__ == '__main__':
-    main()
+    parser = argparse.ArgumentParser()
+    parser.add_argument('destination', choices=['both', 'remote', 'local'], default='both', nargs='?')
+    args = parser.parse_args()
+    main(destination=args.destination)
diff --git a/pytest.ini b/pytest.ini
new file mode 100644
index 0000000..c069a87
--- /dev/null
+++ b/pytest.ini
@@ -0,0 +1,5 @@
+[pytest]
+markers =
+    remote: mark a test as depending on remote data (using Orthanc)
+    local: mark a test as depending on local data (`test_dicom_dir`), imported into a local Horos instance
+    integration: mark a test as being an integration test