Skip to content

Pytests for each step in the workflow and initial GitHub Actions setup.#1

Open
Sasha-Barisic wants to merge 5 commits into
mainfrom
sasha/setup-ci
Open

Pytests for each step in the workflow and initial GitHub Actions setup.#1
Sasha-Barisic wants to merge 5 commits into
mainfrom
sasha/setup-ci

Conversation

@Sasha-Barisic
Copy link
Copy Markdown

Good morning @pchlap,

Hope you are well!

In this PR:

  • I have added unit tests for each step in the sequential workflow. Some can be refined further once the code is modularised.
  • I have also added the initial GitHub Actions workflow to trigger on each PR.

Discussion

While developing a unit test for the scan_file function in preprocess.py, I tested what happens when a non-DICOM file is provided (even though this is unlikely in practice). I expected the result to be None and the status to be "error", but instead I received result = None, status = "ok", and an error message.

I also tried with a random DICOM file from the test data and encountered the same result - the status was "ok" despite an error being raised during scanning.

Looking at the scan_file function, it seems the except block does not explicitly set the status to "error". Was this intentional - perhaps to keep files in quarantine regardless of scan success?

I have commented out the affected unit tests for now:

  • test_scan_file_unsuccessful (ID007)
  • test_scan_file_successful (ID008)
    in test_preprocess.py.

Also, no urgency to review - just when you have some time.

Thank you and enjoy your weekend!

@Sasha-Barisic Sasha-Barisic requested a review from pchlap May 30, 2025 09:17
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.

1 participant