Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add tests for pseudonymisation #214

Merged
merged 15 commits into from
Jan 11, 2024

Conversation

stefpiatek
Copy link
Contributor

@stefpiatek stefpiatek commented Jan 11, 2024

Addressed

  • pulled in main, quite a few changes here but if you pull in main yourself then I think those will go away?
  • Broke out some of the helper methods into other files so its a bit easier to have concise test files
  • Using environmental variables instead of config file! Now I understand why it was complaining about not having the yaml file, all of the services will be passed environmental variables which are read in by config so that makes it easier and tests can be run from anywhere
  • Had a quick go at writing some tests for the pipeline, I think they're sufficient for our changes

Next steps assuming you're happy with these

I think this will need a couple of things for it to work:

  • Add required enviornmental variables using something like os.environ, conftest.py is a classic place to do this because its run before the test. Then the env variables exist during testing with dummy values so we don't throw errors
  • monkeypatch the requests.get method to return an object which has the statuscode and content fields
    • status code can always be 200
    • for a fake hash the tests are expecting adding dashes between each character and returning it encoded into bytes using utf-8

@stefpiatek stefpiatek requested a review from ruaridhg January 11, 2024 15:14
…age_pseudonymisation

# Conflicts:
#	pixl_dcmd/src/pixl_dcmd/_database.py
#	pixl_dcmd/src/pixl_dcmd/main.py
#	pixl_dcmd/src/pixl_dcmd/tests/conftest.py
#	pixl_dcmd/src/pixl_dcmd/tests/test_datetime.py
Copy link
Contributor

@ruaridhg ruaridhg left a comment

Choose a reason for hiding this comment

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

Looks good, will add in the env vars now

@ruaridhg ruaridhg merged commit 34120fe into rmg/img_pseudoanon Jan 11, 2024
5 of 8 checks passed
@ruaridhg ruaridhg deleted the stef/image_pseudonymisation branch January 11, 2024 16:52
ruaridhg added a commit that referenced this pull request Jan 12, 2024
* Updated hash and added to db

* Query PIXL db within pixl_dcmd to check dataset exported

* Updated hash_value, started tests

* Remove unused requirements

* Split out deid and datetime helpers

* Use env variables to connect to PIXL db

* Update inputs for tag tests to run

* Negative test for image already exported

* Negative test for image already exported

* Use == instead of is for PIXL query

* Install core in CI

* Check editable install uses dir

* Rollback python version for core

---------

Co-authored-by: ruaridhg <ruaridhg@users.noreply.github.com>
ruaridhg added a commit that referenced this pull request Jan 12, 2024
* Updated hash and added to db

* Query PIXL db within pixl_dcmd to check dataset exported

* Updated hash_value, started tests

* dummy env vars for hasher and mock requests library

* Add tests for pseudonymisation  (#214)

* Updated hash and added to db

* Query PIXL db within pixl_dcmd to check dataset exported

* Updated hash_value, started tests

* Remove unused requirements

* Split out deid and datetime helpers

* Use env variables to connect to PIXL db

* Update inputs for tag tests to run

* Negative test for image already exported

* Negative test for image already exported

* Use == instead of is for PIXL query

* Install core in CI

* Check editable install uses dir

* Rollback python version for core

---------

Co-authored-by: ruaridhg <ruaridhg@users.noreply.github.com>

* ENV vars set, skeleton mock requests

* Mock requests added to conftest

* Remove unnecessary comments

* Update fixture to be more generic

* Update workflow to install pixl_core first

* Updated pydicom to 2.4.4

* Updated pydicom to 2.4.4

* Reading in testdata_file differently

* Reading in testdata_file like originally

* Workflow install pydicom-data

* Add pydicom-data to requirements

* Added PR suggestions

* Moved tests back to org location

* Revert dir errors

* Fix system tests

* Revert workflow

* Try pip install pixl_core

* import annotations to fix linting

* -e workflow

* install pixl_core in orthanc-anon Dockerfile

---------

Co-authored-by: ruaridhg <ruaridhg@users.noreply.github.com>
Co-authored-by: Stef Piatek <s.piatek@ucl.ac.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants