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

Scalable Test Harness over many DICOMs #84

Open
suyashkumar opened this issue Jun 8, 2020 · 5 comments
Open

Scalable Test Harness over many DICOMs #84

suyashkumar opened this issue Jun 8, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@suyashkumar
Copy link
Owner

We should consider building a more scalable test harness. As the test set of DICOMs grow, we should not be committing these dicom blobs into git. Instead, we should have a set of lightweight sanity checks along with unit tests that run based on information in the repository, and on demand (or before merging into master) we should be able to trigger a test routine where we fetch a (larger) collection of DICOMs from cloud storage and run simple parse-failure checks over all of them.

@suyashkumar suyashkumar changed the title Scalable Test Harness Scalable Test Harness over many DICOMs Jun 8, 2020
@suyashkumar suyashkumar added the enhancement New feature or request label Jun 8, 2020
@wkoszek
Copy link
Contributor

wkoszek commented Jun 20, 2020

@suyashkumar Do you have any examples for writing new DICOMs with your library?

@suyashkumar
Copy link
Owner Author

suyashkumar commented Jun 22, 2020

Nothing yet other than what you can find in the tests here: https://github.com/suyashkumar/dicom/blob/master/write/writer_test.go. Most of the write package code was inherited from upstream fork, and I haven't done much in that area. Would be great to start to improve this portion of the library if you have any requests.

@wkoszek
Copy link
Contributor

wkoszek commented Jun 23, 2020

@suyashkumar I'll discuss with @IChocked We would benefit from this functionality for CI/CD functionality, where I'd really like to generate some DICOMs created from the code.

@suyashkumar
Copy link
Owner Author

Sounds good, one option I've been looking into for "smoke test" level coverage is using GIT LFS with GitHub to store the DICOM binary blobs.

@suyashkumar
Copy link
Owner Author

So, I need to look into GIT LFS some more for hosting these test files. It seems like repos that use GIT LFS may not work well with go get w/modules (even if the LFS assets are not needed to build the dependency and are only used for tests). I've only just taken a cursory glance at golang/go#39720. This needs to be investigated further, but in the meantime to unblock e2e tests I will add some smaller sets of test files directly into the git repo to help provide some regression coverage.

suyashkumar added a commit that referenced this issue Oct 31, 2020
Adding some more testfiles inline in the repository for smoke tests to prevent regressions. Longer term, we can explore a large cloud bucket of testfiles that we can iterate over in a test if we want a more expansive set of files (and larger files).

This is somewhat associated with #84
suyashkumar added a commit that referenced this issue Nov 2, 2020
Adding some more testfiles inline in the repository for smoke tests to prevent regressions. Longer term, we can explore a large cloud bucket of testfiles that we can iterate over in a test if we want a more expansive set of files (and larger files).

This is somewhat associated with #84
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants