-
Notifications
You must be signed in to change notification settings - Fork 88
Test xarray naming post conversion and enable GUI tests #327
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
tests/test_legacy.py
Outdated
def test_linked_dataset_convertsion_to_xarray(ij, xarr): | ||
ensure_legacy_enabled(ij) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word 'convertsion' in the test function name is misspelled; consider renaming it to 'conversion'.
def test_linked_dataset_convertsion_to_xarray(ij, xarr): | |
ensure_legacy_enabled(ij) | |
def test_linked_dataset_conversion_to_xarray(ij, xarr): |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Do you respond to replies? Good catch on the typo, but the suggestion deletes a line that is necessary.
Legacy tests are always skipped because no framebuffer is available.
Note that I may need to interactive mode for testing conditional on the platform (i.e. macOS does not support interactive mode).
Showing a Dataset and modifying it is not enough. We need to next obtain an ImagePlus via the WindowManager and sync it.
We merged #323, but we don't have a great test for the fix that reproduced the error, partially because running the test (on CI) would require xvfb. @elevans said he'd look into that - this change should be verified (and merged) after that.