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

Fix issue with port conflicts when running automated DICOM networking tests in a CI infrastructure #63

Closed
wants to merge 3 commits into from

Conversation

jogerh
Copy link

@jogerh jogerh commented Jun 28, 2022

When running DICOM networking tests in a CI infrastructure, port conflicts can be a serious obstacle because only one test can run at a time. This prevents efficient utilization of CI hardware.

This change attempts to make DcmSCP more CI friendly by enabling automatic port assignment. This functionality is enabled by configuring the DcmSCP to listen to port 0. The underlying socket library will then assign an unused port to the DcmSCP when opening a listen port. The main part of this change is about propagating this automatically assigned port up to the API level.

This should prevent port conflicts when running multiple tests in parallel on the same OS instance.

100029962 added 3 commits June 28, 2022 22:31
…assigned to it.

This change updates the network settings accordingly.
Helper function scu_sends_echo would always fail in this test. Fixed by taking into account that negotiateAssociation should fail in this test.
…instead of using hard coded or randomized ports.

This should allow running multiple tests concurrently.

Since we need to obtain the assigned port before using it with any SCU, we need to call DcmSCP::openListenPort from the main thread. Therefore, we unfortunately can't test the DcmSCP through DcmSCP::Listen() function.
@michaelonken
Copy link
Member

Integrated with commit d86bea (visible on master within the next days). Thanks for the patch!

michaelonken added a commit that referenced this pull request Jul 13, 2022
This change updates the underlying network settings and API
accordingly.

Thanks to GitHub user "jogerh" (Joger Hansegard) for the proposed patch.
See also PR #63 at #63.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants