Skip to content

Conversation

GuiMacielPereira
Copy link
Contributor

Description of work

Originally the issue was to add tests to the view of gsas2, but I did not see a case for it when most of the test cases would have just been mock.assert_called(). Instead I added tests for the call_G2sc.py file, which passes mantid inputs into a GSAS2 script.

Summary of work

Added unit tests to functions on call_G2sc.py file.

Fixes #35624 .

Further detail of work

To test:

Code review and do at least one GSAS refinement on the Eng Diff interface.


Reviewer

Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.

Code Review

  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards?
  • Are the unit tests small and test the class in isolation?
  • If there is GUI work does it follow the GUI standards?
  • If there are changes in the release notes then do they describe the changes appropriately?
  • Do the release notes conform to the release notes guide?

Functional Tests

  • Do changes function as described? Add comments below that describe the tests performed?
  • Do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

Gatekeeper

If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.

Set the basic structure for the unit tests of call_G2sc.py.
Had to wrap code in call_G2sc.py in a main() function so can import functions
from file without triggering the entire routine.
Decided to create this function because if add_histograms exists then it
makes sense for add_phases to exist as well
Added unit test for functions that write to directory.
Set up class and tear down to clean temporary directory.
@GuiMacielPereira GuiMacielPereira added Maintenance Unassigned issues to be addressed in the next maintenance period. ISIS: Diffraction Issue and pull requests relating to Diffraction at ISIS labels Oct 1, 2024
@GuiMacielPereira GuiMacielPereira added this to the Release 6.12 milestone Oct 1, 2024
Copy link
Contributor

@RichardWaiteSTFC RichardWaiteSTFC left a comment

Choose a reason for hiding this comment

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

Looks great, good job getting your head around call_G2sc!
Tested GSAS-II is called OK from interfaces, just a couple of small changes requested but overall tests looks very comprehensive - good job!

Handled creation of temporary file with tempfile library
Put repeated line for setting up project inside setUp function
Copy link
Contributor

@RichardWaiteSTFC RichardWaiteSTFC left a comment

Choose a reason for hiding this comment

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

Great thanks for the minor changes - sorry it took me so long to re-review!

@SilkeSchomann SilkeSchomann self-assigned this Nov 26, 2024
@SilkeSchomann SilkeSchomann merged commit 9ad096d into main Nov 26, 2024
10 checks passed
@SilkeSchomann SilkeSchomann deleted the add-gsas2-view-tests branch November 26, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ISIS: Diffraction Issue and pull requests relating to Diffraction at ISIS Maintenance Unassigned issues to be addressed in the next maintenance period.
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

Add tests for GSAS2 view in EngDiff UI
3 participants