Add aicsimageio test #77
Conversation
|
example test log here: https://github.com/tlambert03/ome-types/runs/1817769610?check_suite_focus=true |
There was a problem hiding this comment.
This is awesome. I learned some stuff here and it's great to see integration tests between the two libs like this.
I may make a PR back to this library once we have the OmeTiffWriter back in the main to include it's tests here as I expect there will be more specific ome-types API usage in that function than in the reader.
| def download_test_resources() -> None: | ||
| root = Path(__file__).parent.parent.parent | ||
| resources = (root / "aicsimageio" / "aicsimageio" / "tests" / "resources").resolve() | ||
| resources.mkdir(exist_ok=True) | ||
| package = Package.browse( | ||
| "aicsimageio/test_resources", "s3://aics-modeling-packages-test-resources" | ||
| ) | ||
| package["resources"].fetch(resources) |
There was a problem hiding this comment.
Glad you saw that we have a weird test resources setup!
There was a problem hiding this comment.
i like it actually. nice way to access lots of extra-repo data!
There was a problem hiding this comment.
I am wondering if I should place the test resources hash in a text file that you can then read and use here because the latest test resources may not match up with the specific ones for the main branch. Versioned s3 buckets and all.
There was a problem hiding this comment.
yeah, I wasn't sure about that part... you can see I just removed the hash here, assuming it would go out of date at some point. if you can think of a good trick for me to use that'd be great
There was a problem hiding this comment.
Yep. I will get a patch out for it tonight.
| - uses: actions/cache@v2 | ||
| id: cache | ||
| with: | ||
| path: aicsimageio/aicsimageio/tests/resources | ||
| key: ${{ runner.os }}-${{ hashFiles('.github/scripts/download_aics_test_data.py') }} |
There was a problem hiding this comment.
Woah. TIL about this one. This will have to be added to our workflows!
|
Also I didn't even know about the |
|
my favorite flag! the |
There was a problem hiding this comment.
Honestly, this feels a bit weird to me in the sense that it's not this project's responsibility to test aicsimageio. Future changes to aicsimageio's tests could then break ome-types's tests. But I suppose I have nothing against it if you want to adopt that risk, and this seems like a nice quick way to get there.
|
hah. ok!
(just want to point out here, I'm not directly using aicsimagio in any of my tests ... I'm literally just cloning your repo and running a subset of your own tests) |
My mistake on that :). All looks good to me! |
since aicsimageio is probably one of the main users of this library, it'd be nice to know if I'm breaking anything as I iterate. This adds a github action that runs some of the aicsimageio/main branch tests whenever merging to master.
@JacksonMaxfield and @toloudis, would be good to get your input on this. At the moment, I'm only running
pytest aicsimageio/tests/readers/test_ome_tiff_reader.py -v -k "not REMOTE"... but if you can point me to other tests that might directly or indirectly useome-typesfunctionality i can add them