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 VSI crash when selecting Scale or Cut Filter when "builtin" node is selected #15349

Merged
merged 1 commit into from
Feb 17, 2016

Conversation

AntonPiccardoSelg
Copy link
Contributor

Fixes #11947

Issue

The pipeline browser in the VSI has an initial node called builtin (not sure why it is selectable at all). If this node is selected and subsequently Cut or Scale is pressed, Mantid used to crash.

The cause of the crash is related to us querrying for an Active Source when there is none (basically, because the builtin node is neither a source or a filter). The issue arises here, when we pass the Active Source (which is nullptr) to thect ObjectBuilders createFilter method

Solution

In the filter callbacks we need to hedge for empty Active Sources and not do anything if we encounter such a situation.

For Tester:

Please find an Ubunutu installer here: //olympic/babylon5/Scratch/Anton/VSI/11947

Release Notes

  • Release note changes can be found here

Implemented Tests

  • Note that this is a VSI GUI bugfix and currently not being unit tested

Functional Testing

  1. Load a sample workspace into the VSI
  2. Select the builin node in the pipeline browser (the icon at the beginning of the pipeline).
    • You should see that the properties browser changes its display
  3. Now that it is selected. Press Scale
    • Confirm that nothing happens, especially that Mantid does not crash
  4. Press Cut
    • Confirm that nothing happens, especially that Mantid does not crash

@mantid-builder
Copy link
Collaborator

Thanks for the changes. Here's a few things that we'll use to start the review process:

Main Reviewer

Please comment on the following:

Pull Request
  • Does the title summarize the changes, i.e. it's not the automatically generated one?
  • Is an issue link included/required, e.g. "Fixes #XXXX"? If there is an issue link do the changes look sensible and consistent with issue description?
  • Are there adequate testing instructions?
  • A link to the updated release notes has been provided or no release note updates were necessary?
Code Review
  • Is the code of an acceptable quality?
    • Does the code conform to the coding standards? Is it well structured with small focussed classes/methods/functions?
    • Are there unit/system tests in place? Are the unit tests small and test the a class in isolation?
Functional Tests
  • Do changes function as described? Add comments below that describe the tests performed?
    • How do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant documentation been added/updated?
    • Is user-facing documentation written in a user-friendly manner?
    • Has developer documentation been updated if required?

  • Does everything look good? Comment with the ship it emoji but don't merge.

Second Reviewer (@gatekeepers)

  • Have adequate tests, both success & failure cases, been performed?,
  • Has the code been reviewed?,
  • Ready to go? Hit Merge!

@AntonPiccardoSelg AntonPiccardoSelg added Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Vates Component: GUI labels Feb 17, 2016
@AntonPiccardoSelg AntonPiccardoSelg added this to the Release 3.7 milestone Feb 17, 2016
@matthew-d-jones matthew-d-jones self-assigned this Feb 17, 2016
@matthew-d-jones
Copy link
Contributor

Test carried out as described in the PR description with a simple workspace created with CreateMDWorkspace.
No documentation update required as this is a bug fix.
:shipit:

peterfpeterson added a commit that referenced this pull request Feb 17, 2016
…cking_scale_cut

Fix VSI crash when selecting Scale or Cut Filter when "builtin" node is selected
@peterfpeterson peterfpeterson merged commit dc59834 into master Feb 17, 2016
@stuartcampbell stuartcampbell deleted the 11947_fix_crash_when_clicking_scale_cut branch February 23, 2016 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Vates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VSI: crash when clicking Scale/Cut
4 participants