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 nexus save #2405

Merged
merged 5 commits into from
Nov 19, 2024
Merged

Fix nexus save #2405

merged 5 commits into from
Nov 19, 2024

Conversation

samtygier-stfc
Copy link
Collaborator

@samtygier-stfc samtygier-stfc commented Nov 15, 2024

Issue

Closes #2404

Description

Async tasks need to return something, because the None looks like a failed task.
Add a test so that this would have been caught.
Make the test_save_images able to catch the equivalent issue
Make load and save fails raise an exception rather than just open a dialog, so that problems are easier for the tests to catch. This means the test_failed_attempt_to_load_shows_error and test_failed_attempt_to_save_shows_error tests now test for an exception.

Testing

A new test and updates to others

Acceptance Criteria

Check that you can save a nexus file without errors.
Remove the return True from do_nexus_saving() and check that you still get an error dialog and the the system tests (make test-system) catch the problem.

Documentation

Release notes

@samtygier-stfc samtygier-stfc marked this pull request as ready for review November 15, 2024 18:12
@coveralls
Copy link

Coverage Status

coverage: 74.222% (-0.005%) from 74.227%
when pulling 9dd000d on 2402-fix-nexus-save
into e027cbb on main.

@JackEAllen JackEAllen self-requested a review November 19, 2024 09:07
Copy link
Collaborator

@JackEAllen JackEAllen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for resolving this, happy to approve. The changes to do_nexus_saving() and the addition of a new system test look good. I have tested saving as nexus and also followed your instructions to check the system test passes and fails as expected under the right conditions.

@JackEAllen JackEAllen added this pull request to the merge queue Nov 19, 2024
Merged via the queue into main with commit 45a56d1 Nov 19, 2024
8 checks passed
@JackEAllen JackEAllen deleted the 2402-fix-nexus-save branch November 19, 2024 15:01
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.

Saving to nexus: ERROR: Failed to save data. Error: None
3 participants